Re: [PATCH v3 6/6] x86/virt/tdx: Support DPAMT when adding memory for the extensions

[Xu Yilun <yilun.xu@linux.intel.com>]

On Mon, Oct 05, 2026 at 05:57:13PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] tdx: allocator thrashing due to unconditional DPAMT allocations
> --
> 
> commit 02adad93d9f4243664b0cea8f1ad9c0b433e79a8
> Author: Xu Yilun <yilun.xu@linux.intel.com>
> 
> x86/virt/tdx: Support DPAMT when adding memory for the extensions
> 
> This commit updates the TDX module extension memory setup to install a 4K
> Dynamic PAMT (DPAMT) for each page before adding it. It calls tdx_pamt_get()
> for each page within the memory chunk.
> 
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index 5d5f9da1ab028..27a7039ee443c 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1330,8 +1330,15 @@ static __init int tdx_ext_mem_setup(void)
> >  		struct page *chunk = page + added_pages;
> >  		unsigned int i;
> >  
> > -		for (i = 0; i < chunk_pages; i++)
> > +		for (i = 0; i < chunk_pages; i++) {
> > +			ret = tdx_pamt_get(page_to_pfn(chunk + i), NULL);
> 
> [Severity: Medium]
> Will calling tdx_pamt_get() sequentially without a DPAMT cache cause severe
> allocator thrashing during boot?
> 
> Looking at tdx_pamt_get(), providing a NULL cache forces an unconditional 
> allocation of DPAMT backing pages before checking the reference count:
> 
> arch/x86/virt/vmx/tdx/tdx.c:tdx_pamt_get() {
>     ...
> 	ret = alloc_pamt_array(pamt_pages, cache);
> 	if (ret)
> 		return ret;
> 
> 	dpamt_refcount = tdx_find_dpamt_refcount(pfn);
> 
> 	spin_lock(&dpamt_lock);
> 
> 	/*
> 	 * If the DPAMT entry is already added (i.e. refcount >= 1),
> 	 * then just increment the refcount.
> 	 */
> 	if (atomic_inc_not_zero(dpamt_refcount))
> 		goto out_free;
>     ...
> out_free:
> 	spin_unlock(&dpamt_lock);
> 	free_pamt_array(pamt_pages);
>     ...
> }
> 
> Since 511 out of 512 pages in each 2MB block will find the entry already
> present, the newly allocated pages are immediately freed. For tens of
> megabytes of extensions, this results in thousands of unnecessary allocations
> and frees. Furthermore, free_pamt_array() performs expensive memory resets,
> leading to unnecessary boot delays.
> 
> Could a struct tdx_pamt_cache be allocated locally in tdx_ext_mem_setup() and
> passed into tdx_pamt_get() to avoid this thrashing?

tdx_pamt_cache is for cases where memory allocation is not possible when
adding DPAMT. I don't think it's good to use tdx_pamt_cache in normal
cases.

> 
> > +			if (ret) {
> > +				WARN(1, "DPAMT setup error for extensions, stranded all pages\n");
> > +				goto out_free_hpa_list;
> > +			}
> > +
> >  			hpa_list->phys[i] = page_to_phys(chunk + i);
> > +		}
> >  
> >  		ret = tdx_ext_mem_add(hpa_list, chunk_pages);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261006-tdx-module-ext-v3-0-db52cb05b918@linux.intel.com?part=6