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

"Edgecombe, Rick P" <[email protected]>
Newsgroups org.kernel.vger.kvm,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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...

But that is why I thought: No functional issue with or without DPAMT. And the
brittleness is existing.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.