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

On Mon, Aug 10, 2026, Rick P Edgecombe wrote:
> On Fri, 2026-08-07 at 15:13 -0700, Sean Christopherson wrote:
> > > I think it is the same for the other caches consumed by the fault. I guess
> > > "e.g." covers it. But it's not new after DPAMT.
> > 
> > I don't think so?  Especially since as you point out below, nothing else can
> > muck with the SPTEs.  The TDP MMU only consumes an cache entry if it
> > successfully creates a SPTE, and since nothing can muck with SPTEs, anything
> > created on the first attempt will still be there on subsequent attempts.  I.e.
> > the TDP MMU might create SPTEs that are ultimately unused, but I don't think
> > it can exhaust a cache.
> 
> Functionally we won't see multiple kvm_tdp_mmu_map() calls during
> kvm_tdp_mmu_map_private_pfn() because of the locks as we discussed. But... if we
> did, then I think we would have the same pattern of freeing but not topping up
> before retrying:
> 
> kvm_tdp_mmu_map():
> ...
> 	/*
> 	 * The SPTE is either non-present or points to a huge page that
> 	 * needs to be split.
> 	 */
> 	sp = tdp_mmu_alloc_sp(vcpu);
> 	tdp_mmu_init_child_sp(sp, &iter); <- shrink mirror
> 	if (is_mirror_sp(sp))
> 		kvm_mmu_alloc_external_spt(vcpu, sp); <- shrink external
> 
> 	sp->nx_huge_page_disallowed = fault->huge_page_disallowed;
> 
> 	if (is_shadow_present_pte(iter.old_spte)) {
> 		/* Don't support large page for mirrored roots (TDX) */
> 		KVM_BUG_ON(is_mirror_sptep(iter.sptep), vcpu->kvm);
> 		r = tdp_mmu_split_huge_page(kvm, &iter, sp, true);
> 	} else {
> 		r = tdp_mmu_link_sp(kvm, &iter, sp, true);<- TDX
> error(BUSY,etc)
> 	}
> 
> 	/*
> 	 * Force the guest to retry if installing an upper level SPTE
> 	 * failed, e.g. because a different task modified the SPTE.
> 	 */
> 	if (r) {
> 		tdp_mmu_free_unused_sp(sp); <- free_page() external and mirror
> 		goto retry; <- return to kvm_tdp_mmu_map_private_pfn()
> 	}
> ...
> 
> Without the additional top up in kvm_tdp_mmu_map_private_pfn(), the retry
> wouldn't have enough, right? The 'sp' goes back to the cache, but the
> external_pt doesn't. Actually, hmm...

Huh.  Right you are.  I completely forgot that flow existed.  Seems stupidly
obvious in hindsight that something like that would have to exist.

> But that is why I thought: No functional issue with or without DPAMT. And the
> brittleness is existing.

Ya, agreed.