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

[sashiko-bot@kernel.org]

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

--- Patch [8]: [PATCH v7 08/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory ---
commit 3474807f1c1bc4dedc75e2a756b65254c986a41d
Author: Kiryl Shutsemau <kas@kernel.org>

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718014500.2231262-1-rick.p.edgecombe@intel.com?part=8