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.