Re: [PATCH v7 08/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory

[Edgecombe, Rick P]

On Sat, 2026-07-18 at 06:10 +0000, sashiko-bot@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Infinite kernel loop in `kvm_tdp_mmu_map_private_pfn` due to permanent PAMT cache depletion on transient TDX module contention.
> --

Our internal Sashiko found this too. It's a false positive as a real bug.

Today kvm_tdp_mmu_map_private_pfn() is only called tdx_gmem_post_populate()
during TD setup. It holds the heavyweight tdx_vm_state_guard which grabs vm-
>lock, kvm->slots_lock, and all vcpu->mutex. So there should be no contention
possible.

Any potential confusion is not new either, because a similar thing could happen
with the external page tables.

But Yan and I were discussing that it would be a good cleanup to fix this anyway
because the reason it is not a functional issue is not clear from the code. For
improved readability (and quieter sashiko reports) the topup can happen inside
the retry loop. Either by moving the retry loop or moving the topup.

> 
> --- Patch [8]: [PATCH v7 08/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory ---
> commit 3474807f1c1bc4dedc75e2a756b65254c986a41d
> Author: Kiryl Shutsemau <kas@xxxxxxxxxx>
> 
> KVM: TDX: Get/put PAMT pages when (un)mapping private memory
>     
> Add Dynamic PAMT support to KVM's S-EPT MMU by "getting" a PAMT page when
> adding guest memory (PAGE.ADD or PAGE.AUG), and "putting" the page when
> removing guest memory (PAGE.REMOVE).
> 
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index 528ebb3cb0389..049da8559882b 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -1679,16 +1693,28 @@ static struct page *tdx_spte_to_sept_pt(struct kvm *kvm, gfn_t gfn,
> >  static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
> >  				     enum pg_level level, u64 new_spte)
> >  {
> > +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> >  	gpa_t gpa = gfn_to_gpa(gfn);
> >  	u64 err, entry, level_state;
> >  	struct page *sept_pt;
> > +	int ret;
> > +
> > +	if (KVM_BUG_ON(!vcpu, kvm))
> > +		return -EIO;
> >  
> >  	sept_pt = tdx_spte_to_sept_pt(kvm, gfn, new_spte, level);
> >  	if (!sept_pt)
> >  		return -EIO;
> >  
> > +	ret = tdx_pamt_get(page_to_pfn(sept_pt), &to_tdx(vcpu)->pamt_cache);
> > +	if (ret)
> > +		return ret;
> > +
> >  	err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, level, sept_pt,
> >  			       &entry, &level_state);
> > +	if (err)
> > +		tdx_pamt_put(page_to_pfn(sept_pt));
> > +
> >  	if (unlikely(tdx_operand_busy(err)))
> >  		return -EBUSY;
> 
> [Severity: High]
> Does this error path risk causing an infinite loop if the TDX module
> contention leads to TDX_OPERAND_BUSY?
> 
> When tdh_mem_sept_add() returns TDX_OPERAND_BUSY, tdx_pamt_put() frees the
> PAMT pages directly back to the buddy allocator, not back into the vCPU's
> PAMT cache.
> 
> If this happens during VM initialization in a retry loop like
> kvm_tdp_mmu_map_private_pfn(), the vCPU memory caches are topped up outside
> of the loop:
> 
>     r = mmu_topup_memory_caches(vcpu, false);
>     if (r)
>         return r;
> 
>     do {
>         ...
>         r = kvm_tdp_mmu_map(vcpu, &fault);
>     } while (r == RET_PF_RETRY);
> 
> Since the PAMT pages are discarded rather than returned to the cache, a few
> busy retries will completely empty the vCPU's PAMT cache.
> 
> On the next retry, tdx_pamt_get() will return -ENOMEM because the cache is
> empty and it does not fall back to alloc_page() when a cache is provided.
> 
> The TDP MMU handles -ENOMEM by returning RET_PF_RETRY, but because the cache
> is never replenished inside the loop, the loop will infinitely fail and retry.
> 
> Could the PAMT pages be returned to the vCPU cache instead of the buddy
> allocator during transient failures, or should the cache top-up be moved
> inside the retry loop?
>