[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Yn7hUe9nyey/CS3J@google.com>
Date: Fri, 13 May 2022 22:53:05 +0000
From: David Matlack <dmatlack@...gle.com>
To: Lai Jiangshan <jiangshanlai@...il.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
Paolo Bonzini <pbonzini@...hat.com>,
Sean Christopherson <seanjc@...gle.com>,
Lai Jiangshan <jiangshan.ljs@...group.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH V2 1/7] KVM: X86/MMU: Add using_special_root_page()
On Tue, May 03, 2022 at 11:07:29PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@...group.com>
>
> In some case, special roots are used in mmu. It is often using
> to_shadow_page(mmu->root.hpa) to check if special roots are used.
>
> Add using_special_root_page() to directly check if special roots are
> used or needed to be used even mmu->root.hpa is not set.
>
> Prepare for making to_shadow_page(mmu->root.hpa) return non-NULL via
> using special shadow pages.
>
> Signed-off-by: Lai Jiangshan <jiangshan.ljs@...group.com>
> ---
> arch/x86/kvm/mmu/mmu.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 909372762363..7f20796af351 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -1711,6 +1711,14 @@ static void drop_parent_pte(struct kvm_mmu_page *sp,
> mmu_spte_clear_no_track(parent_pte);
> }
>
> +static bool using_special_root_page(struct kvm_mmu *mmu)
Could you enumerate all the scenarios that use special roots and which
roots are the special ones? I think that would help a lot with reviewing
this series and would be useful to encode in a comment, probably above
this function here, for future readers.
Also the term "special" is really vague. Maybe once you enumerate all
the scenarios a common theme will arise and we can pick a better name,
unless you have any ideas off the top of your head.
> +{
> + if (mmu->root_role.direct)
> + return mmu->root_role.level == PT32E_ROOT_LEVEL;
> + else
> + return mmu->cpu_role.base.level <= PT32E_ROOT_LEVEL;
> +}
> +
> static struct kvm_mmu_page *kvm_mmu_alloc_page(struct kvm_vcpu *vcpu, int direct)
> {
> struct kvm_mmu_page *sp;
> @@ -4241,10 +4249,10 @@ static bool fast_pgd_switch(struct kvm *kvm, struct kvm_mmu *mmu,
> {
> /*
> * For now, limit the caching to 64-bit hosts+VMs in order to avoid
> - * having to deal with PDPTEs. We may add support for 32-bit hosts/VMs
> - * later if necessary.
> + * having to deal with PDPTEs. Special roots can not be put into
> + * mmu->prev_roots[].
> */
> - if (VALID_PAGE(mmu->root.hpa) && !to_shadow_page(mmu->root.hpa))
> + if (VALID_PAGE(mmu->root.hpa) && using_special_root_page(mmu))
> kvm_mmu_free_roots(kvm, mmu, KVM_MMU_ROOT_CURRENT);
>
> if (VALID_PAGE(mmu->root.hpa))
> --
> 2.19.1.6.gb485710b
>
Powered by blists - more mailing lists