[Bug 219588] [6.13.0-rc2+]WARNING: CPU: 52 PID: 12253 at arch/x86/kvm/mmu/tdp_mmu.c:1001 tdp_mmu_map_handle_target_level+0x1f0/0x310 [kvm]

[bugzilla-daemon@kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=219588

--- Comment #4 from leiyang@redhat.com ---
Hi (In reply to Sean Christopherson from comment #3)
> On Mon, Dec 16, 2024, bugzilla-daemon@kernel.org wrote
> However, retrying the faulting access instead of overwriting an existing
> SPTE is functionally correct and desirable irrespective of the WARN, and
> fast GUP _can_ legitimately fail with a writable VMA, e.g. if the Accessed
> bit in primary MMU's PTE is toggled and causes a PTE value mismatch.  The
> WARN was also recently added, specifically to track down scenarios where
> KVM is unnecessarily overwrites SPTEs, i.e. treating the fault as spurious
> doesn't regress KVM's bug-finding capabilities in any way.  In short,
> letting the WARN linger because there's a tiny chance it's due to a bug
> elsewhere would be excessively paranoid.
> 
> Fixes: 1a175082b190 ("KVM: x86/mmu: WARN and flush if resolving a TDP MMU
> fault clears MMU-writable")
> Reported-by: leiyang@redhat.com
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219588
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/mmu/mmu.c     | 12 ------------
>  arch/x86/kvm/mmu/spte.h    | 17 +++++++++++++++++
>  arch/x86/kvm/mmu/tdp_mmu.c |  5 +++++
>  3 files changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 22e7ad235123..2401606db260 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3364,18 +3364,6 @@ static bool fast_pf_fix_direct_spte(struct kvm_vcpu
> *vcpu,
>       return true;
>  }
>  
> -static bool is_access_allowed(struct kvm_page_fault *fault, u64 spte)
> -{
> -     if (fault->exec)
> -             return is_executable_pte(spte);
> -
> -     if (fault->write)
> -             return is_writable_pte(spte);
> -
> -     /* Fault was on Read access */
> -     return spte & PT_PRESENT_MASK;
> -}
> -
>  /*
>   * Returns the last level spte pointer of the shadow page walk for the given
>   * gpa, and sets *spte to the spte value. This spte may be non-preset. If no
> diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
> index f332b33bc817..6285c45fa56d 100644
> --- a/arch/x86/kvm/mmu/spte.h
> +++ b/arch/x86/kvm/mmu/spte.h
> @@ -461,6 +461,23 @@ static inline bool is_mmu_writable_spte(u64 spte)
>       return spte & shadow_mmu_writable_mask;
>  }
>  
> +/*
> + * Returns true if the access indiciated by @fault is allowed by the
> existing
> + * SPTE protections.  Note, the caller is responsible for checking that the
> + * SPTE is a shadow-present, leaf SPTE (either before or after).
> + */
> +static inline bool is_access_allowed(struct kvm_page_fault *fault, u64 spte)
> +{
> +     if (fault->exec)
> +             return is_executable_pte(spte);
> +
> +     if (fault->write)
> +             return is_writable_pte(spte);
> +
> +     /* Fault was on Read access */
> +     return spte & PT_PRESENT_MASK;
> +}
> +
>  /*
>   * If the MMU-writable flag is cleared, i.e. the SPTE is write-protected for
>   * write-tracking, remote TLBs must be flushed, even if the SPTE was
> read-only,
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 4508d868f1cd..2f15e0e33903 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -985,6 +985,11 @@ static int tdp_mmu_map_handle_target_level(struct
> kvm_vcpu *vcpu,
>       if (fault->prefetch && is_shadow_present_pte(iter->old_spte))
>               return RET_PF_SPURIOUS;
>  
> +     if (is_shadow_present_pte(iter->old_spte) &&
> +         is_access_allowed(fault, iter->old_spte) &&
> +         is_last_spte(iter->old_spte, iter->level))
> +             return RET_PF_SPURIOUS;
> +
>       if (unlikely(!fault->slot))
>               new_spte = make_mmio_spte(vcpu, iter->gfn, ACC_ALL);
>       else
> 
> base-commit: 3522c419758ee8dca5a0e8753ee0070a22157bc1

Hi Sean

This problem has gone after applied your provide patch.

Thanks
Lei

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.