From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0E2A72E0C0 for ; Thu, 15 Jun 2023 16:59:45 +0000 (UTC) Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-bc9483b506fso2155131276.0 for ; Thu, 15 Jun 2023 09:59:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686848385; x=1689440385; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ncA8NUQ/d7RIXyoqKxF4dofBT4rINO6z44852n+jNFs=; b=3+BLFa+Gex3JLa4Zjt1LnKcKFDqyW7CVf7F+H6cOlGpsVINgPgwXyBezn84ilCScQj VEgywmnKY7GLPanOVXUespMiUXuOxLzFqwmDxyyFm/8z1IeyCs1FdebyMF5ykXWTVQ2A Bg9EKrWtul41ll1LOjDjwTKcngzEwIE4oEdWfe7MaFsWgzqDFmUy/H10RvQ+RdMw19f4 bTnyN5swJ/XbJCyti74SLgwiuR94BWZwLL2oAgVBWrbe0rAc6d5feHQ2wSyJ1AzgvxUg vFa/cNJqTGbNSlx8MrOWVgEIWOT+lK1n7ifu70g4+RsV/MQ/oHanzxaWgeajwqAffE3K 6K0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686848385; x=1689440385; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ncA8NUQ/d7RIXyoqKxF4dofBT4rINO6z44852n+jNFs=; b=lVxZOLwJ0uRArQtuk4XDAS3zEBuwgZWt3C/1cGS+/EyUg/BfYWZYiR0N3tYe4hEB0i lrsyiqUUA3xFVa89bA9JTiQaqJvlWIVY5wfRCJdkNsuBXQk/eeDnTlEARlUjr/gP+NM8 Zdw5HjKt3cdLooIEcSf4Gfty8Bbt8nAxeHNtSNjXIjvdQjGjIQAjasnmVXA3b+1d2um7 cZ4tRB7n2TD1HWQJvRYg25temY4jvVCOlVnQGzjvueJEjYRrElh4gtlvgyJLvIYjWlv7 V0FltzZLJmSCSPUhIUpyA81m/GDLkJ+VDPRbnD081Wsdcb6davtxTk13gJ+lQjEKSlWA BBbg== X-Gm-Message-State: AC+VfDzd9DaGNzWdN9kzXqA8SLW9Ftewve1KvcBrv/7Zx6q5lXTqaPSK aMTTpQ1kWDm4PXygwRPpbA1ZwhGMD0Y= X-Google-Smtp-Source: ACHHUZ58VbUpKiHJn4yWrs3hFzFVgLfs2NAcssDwAzKOV57o1C8oPk/BJPOHvBI5DAEtAMIVLjSdWrCDPAY= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:3495:0:b0:bc3:9cd9:6e0e with SMTP id b143-20020a253495000000b00bc39cd96e0emr732274yba.10.1686848384875; Thu, 15 Jun 2023 09:59:44 -0700 (PDT) Date: Thu, 15 Jun 2023 09:59:43 -0700 In-Reply-To: <20230526234435.662652-9-yuzhao@google.com> Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20230526234435.662652-1-yuzhao@google.com> <20230526234435.662652-9-yuzhao@google.com> Message-ID: Subject: Re: [PATCH mm-unstable v2 08/10] kvm/x86: move tdp_mmu_enabled and shadow_accessed_mask From: Sean Christopherson To: Yu Zhao Cc: Andrew Morton , Paolo Bonzini , Alistair Popple , Anup Patel , Ben Gardon , Borislav Petkov , Catalin Marinas , Chao Peng , Christophe Leroy , Dave Hansen , Fabiano Rosas , Gaosheng Cui , Gavin Shan , "H. Peter Anvin" , Ingo Molnar , James Morse , "Jason A. Donenfeld" , Jason Gunthorpe , Jonathan Corbet , Marc Zyngier , Masami Hiramatsu , Michael Ellerman , Michael Larabel , Mike Rapoport , Nicholas Piggin , Oliver Upton , Paul Mackerras , Peter Xu , Steven Rostedt , Suzuki K Poulose , Thomas Gleixner , Thomas Huth , Will Deacon , Zenghui Yu , kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org, linux-trace-kernel@vger.kernel.org, x86@kernel.org, linux-mm@google.com Content-Type: text/plain; charset="us-ascii" On Fri, May 26, 2023, Yu Zhao wrote: > tdp_mmu_enabled and shadow_accessed_mask are needed to implement > kvm_arch_has_test_clear_young(). > > Signed-off-by: Yu Zhao > --- > arch/x86/include/asm/kvm_host.h | 6 ++++++ > arch/x86/kvm/mmu.h | 6 ------ > arch/x86/kvm/mmu/spte.h | 1 - > 3 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index fb9d1f2d6136..753c67072c47 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1772,6 +1772,7 @@ struct kvm_arch_async_pf { > > extern u32 __read_mostly kvm_nr_uret_msrs; > extern u64 __read_mostly host_efer; > +extern u64 __read_mostly shadow_accessed_mask; > extern bool __read_mostly allow_smaller_maxphyaddr; > extern bool __read_mostly enable_apicv; > extern struct kvm_x86_ops kvm_x86_ops; > @@ -1855,6 +1856,11 @@ void kvm_fire_mask_notifiers(struct kvm *kvm, unsigned irqchip, unsigned pin, > bool mask); > > extern bool tdp_enabled; > +#ifdef CONFIG_X86_64 > +extern bool tdp_mmu_enabled; > +#else > +#define tdp_mmu_enabled false > +#endif I would much prefer that these be kept in kvm/mmu.h. And looking at all the arch code, there's no reason to make kvm_arch_has_test_clear_young() a runtime callback, all of the logic is constant relative to when KVM is loaded. So rather than have generic KVM pull from arch code, what if we have arch code push info to generic KVM? We could even avoid #ifdefs if arch code passed in its handler. That might result in an extra indirect branch though, so it might be better to just use a flag? E.g. the x86 conversion would be something like this. --- arch/x86/kvm/mmu/mmu.c | 5 +++++ arch/x86/kvm/mmu/tdp_mmu.c | 2 +- arch/x86/kvm/mmu/tdp_mmu.h | 1 + include/linux/kvm_host.h | 24 ++++-------------------- virt/kvm/kvm_main.c | 14 ++++++++++---- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index c8ebe542c565..84a4a83540f0 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -5809,6 +5809,11 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level, max_huge_page_level = PG_LEVEL_1G; else max_huge_page_level = PG_LEVEL_2M; + + if (tdp_mmu_enabled && kvm_ad_enabled()) + kvm_init_test_clear_young(kvm_tdp_mmu_test_clear_young); + else + kvm_init_test_clear_young(NULL); } EXPORT_SYMBOL_GPL(kvm_configure_mmu); diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index f463d54228f8..e878c88f0e02 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -1308,7 +1308,7 @@ bool kvm_tdp_mmu_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) return kvm_tdp_mmu_handle_gfn(kvm, range, test_age_gfn); } -bool kvm_arch_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range) +bool kvm_tdp_mmu_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range) { struct kvm_mmu_page *root; int offset = ffs(shadow_accessed_mask) - 1; diff --git a/arch/x86/kvm/mmu/tdp_mmu.h b/arch/x86/kvm/mmu/tdp_mmu.h index 0a63b1afabd3..aaa0b75b3896 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.h +++ b/arch/x86/kvm/mmu/tdp_mmu.h @@ -34,6 +34,7 @@ bool kvm_tdp_mmu_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range, bool kvm_tdp_mmu_age_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range); +bool kvm_tdp_mmu_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_wrprot_slot(struct kvm *kvm, const struct kvm_memory_slot *slot, int min_level); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 1714f82a0c47..7a0922cbc36f 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -264,31 +264,15 @@ struct kvm_gfn_range { pte_t pte; bool may_block; }; + +typedef bool (*hva_handler_t)(struct kvm *kvm, struct kvm_gfn_range *range); + bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_should_clear_young(struct kvm_gfn_range *range, gfn_t gfn); -bool kvm_arch_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range); -#endif - -/* - * Architectures that implement kvm_arch_test_clear_young() should override - * kvm_arch_has_test_clear_young(). - * - * kvm_arch_has_test_clear_young() is allowed to return false positive, i.e., it - * can return true if kvm_arch_test_clear_young() is supported but disabled due - * to some runtime constraint. In this case, kvm_arch_test_clear_young() should - * return true; otherwise, it should return false. - * - * For each young KVM PTE, kvm_arch_test_clear_young() should call - * kvm_should_clear_young() to decide whether to clear the accessed bit. - */ -#ifndef kvm_arch_has_test_clear_young -static inline bool kvm_arch_has_test_clear_young(void) -{ - return false; -} +void kvm_init_test_clear_young(hva_handler_t arch_test_clear_young); #endif enum { diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index ef2790469fda..ac83cfb30771 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -530,8 +530,6 @@ static void kvm_mmu_notifier_invalidate_range(struct mmu_notifier *mn, srcu_read_unlock(&kvm->srcu, idx); } -typedef bool (*hva_handler_t)(struct kvm *kvm, struct kvm_gfn_range *range); - typedef void (*on_lock_fn_t)(struct kvm *kvm, unsigned long start, unsigned long end); @@ -859,6 +857,14 @@ bool kvm_should_clear_young(struct kvm_gfn_range *range, gfn_t gfn) return args->clear; } +static hva_handler_t kvm_test_clear_young; + +void kvm_init_test_clear_young(hva_handler_t arch_test_clear_young) +{ + WARN_ON_ONCE(!list_empty(&vm_list)); + kvm_test_clear_young = arch_test_clear_young; +} + static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_struct *mm, unsigned long start, unsigned long end, bool clear, unsigned long *bitmap) @@ -873,7 +879,7 @@ static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_ trace_kvm_age_hva(start, end); - if (kvm_arch_has_test_clear_young()) { + if (kvm_test_clear_young) { struct test_clear_young_args args = { .bitmap = bitmap, .end = end, @@ -882,7 +888,7 @@ static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_ range.args = &args; range.lockless = true; - range.handler = kvm_arch_test_clear_young; + range.handler = kvm_test_clear_young; if (!__kvm_handle_hva_range(kvm, &range)) return args.young ? MMU_NOTIFIER_RANGE_LOCKLESS : 0; base-commit: 39ca80f27cc0d2a37b4e3d07bbf763d4954934d7 -- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A9F19EB64D9 for ; Thu, 15 Jun 2023 20:44:04 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20221208 header.b=nQbZV095; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4QhvQq0Pddz3bsP for ; Fri, 16 Jun 2023 06:44:03 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20221208 header.b=nQbZV095; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=flex--seanjc.bounces.google.com (client-ip=2607:f8b0:4864:20::a49; helo=mail-vk1-xa49.google.com; envelope-from=3geolzaykdf8pb7kg9dlldib.9ljifkrumm9-absifpqp.lwi78p.lod@flex--seanjc.bounces.google.com; receiver=lists.ozlabs.org) Received: from mail-vk1-xa49.google.com (mail-vk1-xa49.google.com [IPv6:2607:f8b0:4864:20::a49]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4QhvPs5pzCz30FW for ; Fri, 16 Jun 2023 06:43:11 +1000 (AEST) Received: by mail-vk1-xa49.google.com with SMTP id 71dfb90a1353d-46e6f186080so763784e0c.3 for ; Thu, 15 Jun 2023 13:43:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686861788; x=1689453788; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ncA8NUQ/d7RIXyoqKxF4dofBT4rINO6z44852n+jNFs=; b=nQbZV095qPTODM38p030xUkUE5YvPu/SZRzBpPfAXb+fLsoa1IGO9yqckVeOQshL6T K+JwXgnKUHxQEx5636G+4fgYCnUVleMvPleLvieSaIvK9v+lIL8nmEZQO8QButhEQLZv lW//NSLqYnHhKpkWpZb3umXstgQFcdd2srXFlCBcWln+720WttnI0XQ9xBtmd1Evi7YL a5oQG5UTyBCNvIUY6C/RgnT59oqEOLyP8z+w1VtbmtIDLYsjgcDSunfFLuBPr1wPUPYU pwLL1WcZBPSf8UWLLktIi6pDH2d+QINc05EvgnluyEVFUlX2ucegVqKnUxlMpj7qFseF q1sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686861788; x=1689453788; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ncA8NUQ/d7RIXyoqKxF4dofBT4rINO6z44852n+jNFs=; b=ZbZRu+jMI+WlFZkrvjrkCPiKlctmOtpz0jSMwjNJxxzYSrKVJlaGYUmonmLXMYCurY VzTxTPIJF/F4CeUEtBZSHap7tAAlwjxBmwnPiugArWxsQYMzVdESUOqC+utnpnomoThp vBsbdwyXXvu+5A+No0GEL1s2tyHrsYdaXLi7kJ7FHuQqtd0HkyaBEFm+IyCIsR6z/lcx cmt8T7M0SDVgenOc4hprxpsLO7YrpShwPaQgsmbPR8lx5z+mS+9a8SbybP/DREHrT96k R/a/R4lGF7nY+D6LH2TWF2xXn2XtoOFny8qx5LwE52/Bjg1a54WJn/1pU23lZynuNL7J SAJg== X-Gm-Message-State: AC+VfDy8T/CydmYKuyISeOgVqxjpJaahbARA4GGrKXS/+p0umkzidDT8 2xR51gkT8NsIlr+HdROdPV7V7B0OY4Q= X-Google-Smtp-Source: ACHHUZ58VbUpKiHJn4yWrs3hFzFVgLfs2NAcssDwAzKOV57o1C8oPk/BJPOHvBI5DAEtAMIVLjSdWrCDPAY= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:3495:0:b0:bc3:9cd9:6e0e with SMTP id b143-20020a253495000000b00bc39cd96e0emr732274yba.10.1686848384875; Thu, 15 Jun 2023 09:59:44 -0700 (PDT) Date: Thu, 15 Jun 2023 09:59:43 -0700 In-Reply-To: <20230526234435.662652-9-yuzhao@google.com> Mime-Version: 1.0 References: <20230526234435.662652-1-yuzhao@google.com> <20230526234435.662652-9-yuzhao@google.com> Message-ID: Subject: Re: [PATCH mm-unstable v2 08/10] kvm/x86: move tdp_mmu_enabled and shadow_accessed_mask From: Sean Christopherson To: Yu Zhao Content-Type: text/plain; charset="us-ascii" X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Jason A. Donenfeld" , x86@kernel.org, Gavin Shan , kvm@vger.kernel.org, linux-doc@vger.kernel.org, Catalin Marinas , Dave Hansen , Peter Xu , linux-mm@kvack.org, Ben Gardon , Chao Peng , Will Deacon , Gaosheng Cui , Marc Zyngier , "H. Peter Anvin" , Jonathan Corbet , Alistair Popple , Jason Gunthorpe , Ingo Molnar , Zenghui Yu , linux-trace-kernel@vger.kernel.org, linux-mm@google.com, Thomas Huth , Suzuki K Poulose , Nicholas Piggin , Borislav Petkov , Steven Rostedt , kvmarm@lists.linux.dev, Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Fabiano Rosas , Michael Larabel , linux-kernel@vger.kernel.org, Oliver Upton , James Morse , Masami Hiramatsu , Anup Patel , Paolo Bonzini , Andrew Morton , linuxppc-dev@lists.ozlabs.org, Mike Rapoport Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Fri, May 26, 2023, Yu Zhao wrote: > tdp_mmu_enabled and shadow_accessed_mask are needed to implement > kvm_arch_has_test_clear_young(). > > Signed-off-by: Yu Zhao > --- > arch/x86/include/asm/kvm_host.h | 6 ++++++ > arch/x86/kvm/mmu.h | 6 ------ > arch/x86/kvm/mmu/spte.h | 1 - > 3 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index fb9d1f2d6136..753c67072c47 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1772,6 +1772,7 @@ struct kvm_arch_async_pf { > > extern u32 __read_mostly kvm_nr_uret_msrs; > extern u64 __read_mostly host_efer; > +extern u64 __read_mostly shadow_accessed_mask; > extern bool __read_mostly allow_smaller_maxphyaddr; > extern bool __read_mostly enable_apicv; > extern struct kvm_x86_ops kvm_x86_ops; > @@ -1855,6 +1856,11 @@ void kvm_fire_mask_notifiers(struct kvm *kvm, unsigned irqchip, unsigned pin, > bool mask); > > extern bool tdp_enabled; > +#ifdef CONFIG_X86_64 > +extern bool tdp_mmu_enabled; > +#else > +#define tdp_mmu_enabled false > +#endif I would much prefer that these be kept in kvm/mmu.h. And looking at all the arch code, there's no reason to make kvm_arch_has_test_clear_young() a runtime callback, all of the logic is constant relative to when KVM is loaded. So rather than have generic KVM pull from arch code, what if we have arch code push info to generic KVM? We could even avoid #ifdefs if arch code passed in its handler. That might result in an extra indirect branch though, so it might be better to just use a flag? E.g. the x86 conversion would be something like this. --- arch/x86/kvm/mmu/mmu.c | 5 +++++ arch/x86/kvm/mmu/tdp_mmu.c | 2 +- arch/x86/kvm/mmu/tdp_mmu.h | 1 + include/linux/kvm_host.h | 24 ++++-------------------- virt/kvm/kvm_main.c | 14 ++++++++++---- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index c8ebe542c565..84a4a83540f0 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -5809,6 +5809,11 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level, max_huge_page_level = PG_LEVEL_1G; else max_huge_page_level = PG_LEVEL_2M; + + if (tdp_mmu_enabled && kvm_ad_enabled()) + kvm_init_test_clear_young(kvm_tdp_mmu_test_clear_young); + else + kvm_init_test_clear_young(NULL); } EXPORT_SYMBOL_GPL(kvm_configure_mmu); diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index f463d54228f8..e878c88f0e02 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -1308,7 +1308,7 @@ bool kvm_tdp_mmu_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) return kvm_tdp_mmu_handle_gfn(kvm, range, test_age_gfn); } -bool kvm_arch_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range) +bool kvm_tdp_mmu_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range) { struct kvm_mmu_page *root; int offset = ffs(shadow_accessed_mask) - 1; diff --git a/arch/x86/kvm/mmu/tdp_mmu.h b/arch/x86/kvm/mmu/tdp_mmu.h index 0a63b1afabd3..aaa0b75b3896 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.h +++ b/arch/x86/kvm/mmu/tdp_mmu.h @@ -34,6 +34,7 @@ bool kvm_tdp_mmu_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range, bool kvm_tdp_mmu_age_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range); +bool kvm_tdp_mmu_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_wrprot_slot(struct kvm *kvm, const struct kvm_memory_slot *slot, int min_level); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 1714f82a0c47..7a0922cbc36f 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -264,31 +264,15 @@ struct kvm_gfn_range { pte_t pte; bool may_block; }; + +typedef bool (*hva_handler_t)(struct kvm *kvm, struct kvm_gfn_range *range); + bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_should_clear_young(struct kvm_gfn_range *range, gfn_t gfn); -bool kvm_arch_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range); -#endif - -/* - * Architectures that implement kvm_arch_test_clear_young() should override - * kvm_arch_has_test_clear_young(). - * - * kvm_arch_has_test_clear_young() is allowed to return false positive, i.e., it - * can return true if kvm_arch_test_clear_young() is supported but disabled due - * to some runtime constraint. In this case, kvm_arch_test_clear_young() should - * return true; otherwise, it should return false. - * - * For each young KVM PTE, kvm_arch_test_clear_young() should call - * kvm_should_clear_young() to decide whether to clear the accessed bit. - */ -#ifndef kvm_arch_has_test_clear_young -static inline bool kvm_arch_has_test_clear_young(void) -{ - return false; -} +void kvm_init_test_clear_young(hva_handler_t arch_test_clear_young); #endif enum { diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index ef2790469fda..ac83cfb30771 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -530,8 +530,6 @@ static void kvm_mmu_notifier_invalidate_range(struct mmu_notifier *mn, srcu_read_unlock(&kvm->srcu, idx); } -typedef bool (*hva_handler_t)(struct kvm *kvm, struct kvm_gfn_range *range); - typedef void (*on_lock_fn_t)(struct kvm *kvm, unsigned long start, unsigned long end); @@ -859,6 +857,14 @@ bool kvm_should_clear_young(struct kvm_gfn_range *range, gfn_t gfn) return args->clear; } +static hva_handler_t kvm_test_clear_young; + +void kvm_init_test_clear_young(hva_handler_t arch_test_clear_young) +{ + WARN_ON_ONCE(!list_empty(&vm_list)); + kvm_test_clear_young = arch_test_clear_young; +} + static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_struct *mm, unsigned long start, unsigned long end, bool clear, unsigned long *bitmap) @@ -873,7 +879,7 @@ static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_ trace_kvm_age_hva(start, end); - if (kvm_arch_has_test_clear_young()) { + if (kvm_test_clear_young) { struct test_clear_young_args args = { .bitmap = bitmap, .end = end, @@ -882,7 +888,7 @@ static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_ range.args = &args; range.lockless = true; - range.handler = kvm_arch_test_clear_young; + range.handler = kvm_test_clear_young; if (!__kvm_handle_hva_range(kvm, &range)) return args.young ? MMU_NOTIFIER_RANGE_LOCKLESS : 0; base-commit: 39ca80f27cc0d2a37b4e3d07bbf763d4954934d7 -- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AF3D2EB64D9 for ; Thu, 15 Jun 2023 17:00:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=60iEZxX4h8ZcwzBEts79A6q9TFgg/d3c4rheopN6Dcc=; b=BFMknBgBE91Ab3m+GQ4pH48UZO Upg253ANKU8c8uI72U7R40pfdVqu/QmBEazmDW7o95SZW/iw2JjTkKd3gkStKKW7gdxNHh+EryxhR NaUIU+yiywq/hwXKOePR3L9/7WYwzBUcdxiQhyn/T2kRppXVRky9jNY/g//WmgdL6O78pp8qOYZhG 7E2ER+v4WV0dMjqqJweGKSm8R7WrQ+zCkErwaQiB0LpSAJWvWGVR6KXWDQajFe4HHv00GQwMgACFi 1PUFsmWnTfzfWJGDYxDcP6uM5IfmbjD4l+phcalBtd+/S+dkoF1rLr/HRB74P2joyHUxB6pKi4nsN pF2Hj/LA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q9qK7-00FW3X-0D; Thu, 15 Jun 2023 16:59:51 +0000 Received: from mail-yw1-x114a.google.com ([2607:f8b0:4864:20::114a]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q9qK3-00FW2Y-1P for linux-arm-kernel@lists.infradead.org; Thu, 15 Jun 2023 16:59:49 +0000 Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-56942442eb0so24170207b3.1 for ; Thu, 15 Jun 2023 09:59:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686848385; x=1689440385; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ncA8NUQ/d7RIXyoqKxF4dofBT4rINO6z44852n+jNFs=; b=3+BLFa+Gex3JLa4Zjt1LnKcKFDqyW7CVf7F+H6cOlGpsVINgPgwXyBezn84ilCScQj VEgywmnKY7GLPanOVXUespMiUXuOxLzFqwmDxyyFm/8z1IeyCs1FdebyMF5ykXWTVQ2A Bg9EKrWtul41ll1LOjDjwTKcngzEwIE4oEdWfe7MaFsWgzqDFmUy/H10RvQ+RdMw19f4 bTnyN5swJ/XbJCyti74SLgwiuR94BWZwLL2oAgVBWrbe0rAc6d5feHQ2wSyJ1AzgvxUg vFa/cNJqTGbNSlx8MrOWVgEIWOT+lK1n7ifu70g4+RsV/MQ/oHanzxaWgeajwqAffE3K 6K0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686848385; x=1689440385; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ncA8NUQ/d7RIXyoqKxF4dofBT4rINO6z44852n+jNFs=; b=mGzgKthXtNnP8S6HHy5RphyHH7NwPc/XqHLXa0Te/iblIoh3i9eprB41qabaIN32Pt 4iF3jBkuLYZEajXgahbWFCxor6wYAVf2z0f2ORSsN+Qncyw6YCiQl/RsLZKAk8SxKjuO Kz5w8ewjH+oSRKRnwVA97FuNxh+BXOuBQZXTEFbCht3agiJsadAnRZ+Ct029c25oNNWE 5bqXLsi1QSko0kPoLqN2S62MUNkhGSWminXCvsGIMOh11nf5/g+h7UBK+8dw+VWOb/wL mYWG6Wc+cYozueVQNUoYgK69uSLHRapRNPoAVQZScMRAdQtBiNU1mCzxWiNMiE9+L1bD LKpA== X-Gm-Message-State: AC+VfDzLOizJ60THkTudQsZDXZ6icg6EMzTaADIvrrHwHupoLVMgltHn M7XKvslC/CceOpqD510B2o/Rl09sBI4= X-Google-Smtp-Source: ACHHUZ58VbUpKiHJn4yWrs3hFzFVgLfs2NAcssDwAzKOV57o1C8oPk/BJPOHvBI5DAEtAMIVLjSdWrCDPAY= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:3495:0:b0:bc3:9cd9:6e0e with SMTP id b143-20020a253495000000b00bc39cd96e0emr732274yba.10.1686848384875; Thu, 15 Jun 2023 09:59:44 -0700 (PDT) Date: Thu, 15 Jun 2023 09:59:43 -0700 In-Reply-To: <20230526234435.662652-9-yuzhao@google.com> Mime-Version: 1.0 References: <20230526234435.662652-1-yuzhao@google.com> <20230526234435.662652-9-yuzhao@google.com> Message-ID: Subject: Re: [PATCH mm-unstable v2 08/10] kvm/x86: move tdp_mmu_enabled and shadow_accessed_mask From: Sean Christopherson To: Yu Zhao Cc: Andrew Morton , Paolo Bonzini , Alistair Popple , Anup Patel , Ben Gardon , Borislav Petkov , Catalin Marinas , Chao Peng , Christophe Leroy , Dave Hansen , Fabiano Rosas , Gaosheng Cui , Gavin Shan , "H. Peter Anvin" , Ingo Molnar , James Morse , "Jason A. Donenfeld" , Jason Gunthorpe , Jonathan Corbet , Marc Zyngier , Masami Hiramatsu , Michael Ellerman , Michael Larabel , Mike Rapoport , Nicholas Piggin , Oliver Upton , Paul Mackerras , Peter Xu , Steven Rostedt , Suzuki K Poulose , Thomas Gleixner , Thomas Huth , Will Deacon , Zenghui Yu , kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org, linux-trace-kernel@vger.kernel.org, x86@kernel.org, linux-mm@google.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230615_095947_500843_24DFCD51 X-CRM114-Status: GOOD ( 30.65 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, May 26, 2023, Yu Zhao wrote: > tdp_mmu_enabled and shadow_accessed_mask are needed to implement > kvm_arch_has_test_clear_young(). > > Signed-off-by: Yu Zhao > --- > arch/x86/include/asm/kvm_host.h | 6 ++++++ > arch/x86/kvm/mmu.h | 6 ------ > arch/x86/kvm/mmu/spte.h | 1 - > 3 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index fb9d1f2d6136..753c67072c47 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1772,6 +1772,7 @@ struct kvm_arch_async_pf { > > extern u32 __read_mostly kvm_nr_uret_msrs; > extern u64 __read_mostly host_efer; > +extern u64 __read_mostly shadow_accessed_mask; > extern bool __read_mostly allow_smaller_maxphyaddr; > extern bool __read_mostly enable_apicv; > extern struct kvm_x86_ops kvm_x86_ops; > @@ -1855,6 +1856,11 @@ void kvm_fire_mask_notifiers(struct kvm *kvm, unsigned irqchip, unsigned pin, > bool mask); > > extern bool tdp_enabled; > +#ifdef CONFIG_X86_64 > +extern bool tdp_mmu_enabled; > +#else > +#define tdp_mmu_enabled false > +#endif I would much prefer that these be kept in kvm/mmu.h. And looking at all the arch code, there's no reason to make kvm_arch_has_test_clear_young() a runtime callback, all of the logic is constant relative to when KVM is loaded. So rather than have generic KVM pull from arch code, what if we have arch code push info to generic KVM? We could even avoid #ifdefs if arch code passed in its handler. That might result in an extra indirect branch though, so it might be better to just use a flag? E.g. the x86 conversion would be something like this. --- arch/x86/kvm/mmu/mmu.c | 5 +++++ arch/x86/kvm/mmu/tdp_mmu.c | 2 +- arch/x86/kvm/mmu/tdp_mmu.h | 1 + include/linux/kvm_host.h | 24 ++++-------------------- virt/kvm/kvm_main.c | 14 ++++++++++---- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index c8ebe542c565..84a4a83540f0 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -5809,6 +5809,11 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level, max_huge_page_level = PG_LEVEL_1G; else max_huge_page_level = PG_LEVEL_2M; + + if (tdp_mmu_enabled && kvm_ad_enabled()) + kvm_init_test_clear_young(kvm_tdp_mmu_test_clear_young); + else + kvm_init_test_clear_young(NULL); } EXPORT_SYMBOL_GPL(kvm_configure_mmu); diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index f463d54228f8..e878c88f0e02 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -1308,7 +1308,7 @@ bool kvm_tdp_mmu_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) return kvm_tdp_mmu_handle_gfn(kvm, range, test_age_gfn); } -bool kvm_arch_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range) +bool kvm_tdp_mmu_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range) { struct kvm_mmu_page *root; int offset = ffs(shadow_accessed_mask) - 1; diff --git a/arch/x86/kvm/mmu/tdp_mmu.h b/arch/x86/kvm/mmu/tdp_mmu.h index 0a63b1afabd3..aaa0b75b3896 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.h +++ b/arch/x86/kvm/mmu/tdp_mmu.h @@ -34,6 +34,7 @@ bool kvm_tdp_mmu_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range, bool kvm_tdp_mmu_age_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range); +bool kvm_tdp_mmu_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_tdp_mmu_wrprot_slot(struct kvm *kvm, const struct kvm_memory_slot *slot, int min_level); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 1714f82a0c47..7a0922cbc36f 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -264,31 +264,15 @@ struct kvm_gfn_range { pte_t pte; bool may_block; }; + +typedef bool (*hva_handler_t)(struct kvm *kvm, struct kvm_gfn_range *range); + bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_should_clear_young(struct kvm_gfn_range *range, gfn_t gfn); -bool kvm_arch_test_clear_young(struct kvm *kvm, struct kvm_gfn_range *range); -#endif - -/* - * Architectures that implement kvm_arch_test_clear_young() should override - * kvm_arch_has_test_clear_young(). - * - * kvm_arch_has_test_clear_young() is allowed to return false positive, i.e., it - * can return true if kvm_arch_test_clear_young() is supported but disabled due - * to some runtime constraint. In this case, kvm_arch_test_clear_young() should - * return true; otherwise, it should return false. - * - * For each young KVM PTE, kvm_arch_test_clear_young() should call - * kvm_should_clear_young() to decide whether to clear the accessed bit. - */ -#ifndef kvm_arch_has_test_clear_young -static inline bool kvm_arch_has_test_clear_young(void) -{ - return false; -} +void kvm_init_test_clear_young(hva_handler_t arch_test_clear_young); #endif enum { diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index ef2790469fda..ac83cfb30771 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -530,8 +530,6 @@ static void kvm_mmu_notifier_invalidate_range(struct mmu_notifier *mn, srcu_read_unlock(&kvm->srcu, idx); } -typedef bool (*hva_handler_t)(struct kvm *kvm, struct kvm_gfn_range *range); - typedef void (*on_lock_fn_t)(struct kvm *kvm, unsigned long start, unsigned long end); @@ -859,6 +857,14 @@ bool kvm_should_clear_young(struct kvm_gfn_range *range, gfn_t gfn) return args->clear; } +static hva_handler_t kvm_test_clear_young; + +void kvm_init_test_clear_young(hva_handler_t arch_test_clear_young) +{ + WARN_ON_ONCE(!list_empty(&vm_list)); + kvm_test_clear_young = arch_test_clear_young; +} + static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_struct *mm, unsigned long start, unsigned long end, bool clear, unsigned long *bitmap) @@ -873,7 +879,7 @@ static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_ trace_kvm_age_hva(start, end); - if (kvm_arch_has_test_clear_young()) { + if (kvm_test_clear_young) { struct test_clear_young_args args = { .bitmap = bitmap, .end = end, @@ -882,7 +888,7 @@ static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, struct mm_ range.args = &args; range.lockless = true; - range.handler = kvm_arch_test_clear_young; + range.handler = kvm_test_clear_young; if (!__kvm_handle_hva_range(kvm, &range)) return args.young ? MMU_NOTIFIER_RANGE_LOCKLESS : 0; base-commit: 39ca80f27cc0d2a37b4e3d07bbf763d4954934d7 -- _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel