Re: [PATCH v2 2/4] KVM: x86/mmu: Harden "map private PFN" against unexpected root invalidation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The WARN_ON_ONCE diagnostic to detect unexpected root invalidations evaluates after the request bit is silently cleared by kvm_mmu_reload(), rendering it effectively useless.
--

commit 5bfe54f32e3516c8d2a1f4dc3a29739732e31eb5
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Harden "map private PFN" against unexpected root invalidation

This commit moves the MMU reload inside the tight loop in
kvm_tdp_mmu_map_private_pfn() to better handle unexpected root invalidations.
It also adds a warning to detect unexpected invalidations that break KVM
expectations.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 3220f05387b58..1969c26861e5a 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
[ ... ]
> @@ -5224,10 +5220,22 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
>  		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu))
>  			return -EIO;
>  
> +		r = kvm_mmu_reload(vcpu);
> +		if (r)
> +			return r;
> +
>  		cond_resched();
>  
>  		guard(read_lock)(&kvm->mmu_lock);
>  
> +		/*
> +		 * Because slots_lock is held, it should be impossible for *any*
> +		 * roots to be invalidated after the initial MMU reload.  WARN,
> +		 * but continue on; the above MMU reload will do the right thing
> +		 * if the current root is actually invalid.
> +		 */
> +		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));

[Severity: Medium]
Will this WARN_ON_ONCE ever trigger?

Looking at kvm_mmu_reload(), it calls kvm_check_request() which consumes and
clears the request bit:

static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
{
        if (kvm_check_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu))
                kvm_mmu_free_obsolete_roots(vcpu);
...

Since kvm_mmu_reload() is called unconditionally just above in the same loop,
won't the KVM_REQ_MMU_FREE_OBSOLETE_ROOTS bit always be cleared by the time
execution reaches this check in kvm_tdp_mmu_map_private_pfn()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826164214.756512-1-seanjc@google.com?part=2