| Message ID | 20220503150735.32723-2-jiangshanlai@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: X86/MMU: Use one-off special shadow page for special roots | expand |
On Tue, May 03, 2022 at 11:07:29PM +0800, Lai Jiangshan wrote: > From: Lai Jiangshan <jiangshan.ljs@antgroup.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@antgroup.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 >
On Sat, May 14, 2022 at 6:53 AM David Matlack <dmatlack@google.com> wrote: > > On Tue, May 03, 2022 at 11:07:29PM +0800, Lai Jiangshan wrote: > > From: Lai Jiangshan <jiangshan.ljs@antgroup.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@antgroup.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. Thank you for the review. All the scenarios are listed in v3. And comments are added to prove the code matches the scenarios and the scenarios match the code (the proof must be in bi-direction). > > 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. "special" is renamed to "local" thanks Lai
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) +{ + 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))