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@xxxxxxxxxx>
> Reviewed-by: David Matlack <dmatlack@xxxxxxxxxx>
> ---
> 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);
