Messages in this thread Patch in this message | | | Date | Tue, 9 Aug 2022 11:26:09 +0800 | From | Yan Zhao <> | Subject | Re: [PATCH v3 5/8] KVM: x86/mmu: Set disallowed_nx_huge_page in TDP MMU before setting SPTE |
| |
On Fri, Aug 05, 2022 at 11:05:10PM +0000, Sean Christopherson wrote: > Set nx_huge_page_disallowed in TDP MMU shadow pages before making the SP > visible to other readers, i.e. before setting its SPTE. This will allow > KVM to query the flag when determining if a shadow page can be replaced > by a NX huge page without violating the rules of the mitigation. > > Note, the shadow/legacy MMU holds mmu_lock for write, so it's impossible > for another CPU to see a shadow page without an up-to-date > nx_huge_page_disallowed, i.e. only the TDP MMU needs the complicated > dance. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > Reviewed-by: David Matlack <dmatlack@google.com> > --- > arch/x86/kvm/mmu/mmu.c | 28 +++++++++++++------- > arch/x86/kvm/mmu/mmu_internal.h | 5 ++-- > arch/x86/kvm/mmu/tdp_mmu.c | 46 +++++++++++++++++++++++---------- > 3 files changed, 53 insertions(+), 26 deletions(-) > <snip>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index 0e94182c87be..34994ca3d45b 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -392,8 +392,19 @@ static void tdp_mmu_unlink_sp(struct kvm *kvm, struct kvm_mmu_page *sp, > lockdep_assert_held_write(&kvm->mmu_lock); > > list_del(&sp->link); > - if (sp->nx_huge_page_disallowed) > - unaccount_nx_huge_page(kvm, sp); > + > + /* > + * Ensure nx_huge_page_disallowed is read after observing the present > + * shadow page. A different vCPU may have _just_ finished installing > + * the shadow page if mmu_lock is held for read. Pairs with the > + * smp_wmb() in kvm_tdp_mmu_map(). > + */ > + smp_rmb(); hi Sean,
I understand this smp_rmb() is intended to prevent the reading of p->nx_huge_page_disallowed from happening before it's set to true in kvm_tdp_mmu_map(). Is this understanding right?
If it's true, then do we also need the smp_rmb() for read of sp->gfn in handle_removed_pt()? (or maybe for other fields in sp in other places?)
Thanks Yan
> + > + if (sp->nx_huge_page_disallowed) { > + sp->nx_huge_page_disallowed = false; > + untrack_possible_nx_huge_page(kvm, sp); > + } > > if (shared) > spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
| |