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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The WARN_ON_ONCE check for obsolete roots in kvm_tdp_mmu_map_private_pfn is largely ineffective due to being placed after kvm_mmu_reload, which clears the request bit.
--

--- Patch [3]: [PATCH 3/4] KVM: x86/mmu: Top-up memory caches when retrying "map private PFN" ---
commit 7a1e8c6872a95084f82e79ad3438f87300a97a91
Author: Sean Christopherson <[email protected]>

KVM: x86/mmu: Top-up memory caches when retrying "map private PFN"

This commit moves the top-up of memory caches into the retry loop in
kvm_tdp_mmu_map_private_pfn(). This ensures that the caches are replenished
on every attempt to map the PFN, hardening KVM against potential cache
consumption issues during TDX post-populate callbacks.

Closes: https://lore.kernel.org/all/[email protected]

> 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.

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