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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Transient TDX_OPERAND_BUSY errors during private memory mapping cause KVM to indefinitely retry the fault without restocking vCPU memory caches, leading to cache depletion and deterministic VM or host crash.
--

--- Patch [8]: [PATCH v9 08/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory ---
commit caefd7119dad264568b4b1abac0ec41595539508
Author: Kiryl Shutsemau <[email protected]>
Subject: KVM: TDX: Get/put PAMT pages when (un)mapping private memory

This patch adds Dynamic PAMT support to KVM's S-EPT MMU by getting a PAMT
page when adding guest memory, and putting the page when removing it.
The kernel supplies the TDX module with a few pages that cover 2MB of host
physical memory.

> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 957078e8656ef..09573112540e9 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c

> [ ... ]

> @@ -1679,16 +1694,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 (KVM_BUG_ON(ret, kvm))
> +		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));
> +

[Severity: Critical]
When tdh_mem_sept_add() returns an error like TDX_OPERAND_BUSY, this path
calls tdx_pamt_put(), which frees the page back to the global allocator
instead of returning it to the vCPU cache.

Since a busy error returns -EBUSY and triggers a RET_PF_RETRY loop in
kvm_tdp_mmu_map_private_pfn():

kvm_tdp_mmu_map_private_pfn() {
    ...
    do {
        ...
        r = kvm_tdp_mmu_map(vcpu, &fault);
    } while (r == RET_PF_RETRY);
    ...
}

Does this repeatedly drain the vCPU's memory cache without topping it up?

The loop re-attempts the mapping without calling mmu_topup_memory_caches(),
meaning repeated contentions could quickly exhaust the cache capacity.

>  	if (unlikely(tdx_operand_busy(err)))
>  		return -EBUSY;
>  
> [ ... ]
> @@ -1701,8 +1728,13 @@ static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
>  static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level level,
>  				  u64 new_spte)
>  {
> +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
>  	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
>  	kvm_pfn_t pfn = spte_to_pfn(new_spte);
> +	int ret;
> +
> +	if (KVM_BUG_ON(!vcpu, kvm))
> +		return -EIO;
>  
>  	/* TODO: handle large pages. */
>  	if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
> @@ -1710,6 +1742,10 @@ static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level leve
>  
>  	WARN_ON_ONCE((new_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
>  
> +	ret = tdx_pamt_get(pfn, &to_tdx(vcpu)->pamt_cache);
> +	if (KVM_BUG_ON(ret, kvm))
> +		return ret;
> +

[Severity: Critical]
If the vCPU's pamt_cache is exhausted due to the retry loop mentioned above,
tdx_pamt_get() will return -ENOMEM.

Does this KVM_BUG_ON() trigger when the cache depletes, permanently killing
the VM?

Could a busy guest intentionally induce TDX_OPERAND_BUSY contentions by
performing memory operations simultaneously with TDH.VP.ENTER on other vCPUs
to reliably trigger this crash?

>  	/*
>  	 * Ensure pre_fault_allowed is read by kvm_arch_vcpu_pre_fault_memory()
>  	 * before kvm_tdx->state.  Userspace must not be allowed to pre-fault

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8