Re: [PATCH 3/4] KVM: x86/mmu: Top-up memory caches when retrying "map private PFN"

On Thu, Aug 06, 2026, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index c6cac893cbad8..379f570ef04f8 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -5184,10 +5184,6 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
> >  	if (kvm_gfn_is_write_tracked(kvm, fault.slot, fault.gfn))
> >  		return -EPERM;
> >  
> > -	r = mmu_topup_memory_caches(vcpu, false);
> > -	if (r)
> > -		return r;
> > -
> >  	do {
> >  		if (signal_pending(current))
> >  			return -EINTR;
> 
> [ ... ]
> 
> > @@ -5199,6 +5195,10 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
> >  		if (r)
> >  			return r;
> >  
> > +		r = mmu_topup_memory_caches(vcpu, false);
> > +		if (r)
> > +			return r;
> > +
> >  		cond_resched();
> >  
> >  		guard(read_lock)(&kvm->mmu_lock);
> 
> [Severity: Medium]
> In kvm_tdp_mmu_map_private_pfn(), is the WARN_ON_ONCE() check for
> obsolete roots still effective here?
> 
> Just a few lines down from this chunk:
> 
>     WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
> 
> Because kvm_mmu_reload() is called higher up in the loop, which internally
> invokes kvm_check_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu), it will
> clear the request bit.
> 
> If an invalidation occurs during the kvm_tdp_mmu_map() execution, the loop
> correctly retries, but the next iteration's kvm_mmu_reload() will silently
> clear the request bit before execution reaches the WARN_ON_ONCE() statement.
> 
> This seems to leave the assertion only covering the small window between the
> reload and the WARN itself, masking potential regressions where roots are
> unexpectedly invalidated during the map operation.

Yep.  It's honestly more for documentation than it is for actually catching bugs.
Though I do think that if a bug were introduced, there would actually be a decent
chance of catching it because KVM_REQ_MMU_FREE_OBSOLETE_ROOTS is set while holding
mmu_lock for write.

Moving the WARN up isn't an option, because it could get a false positive on the
first iteration.  And the odds of this firing are stupidly low, i.e. it's not
worth reworking the code to skip the WARN on the first iteration.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3