Re: [PATCH 02/15] x86/virt/tdx: Add extra memory to TDX Module for Extensions

[Xu Yilun]

On Fri, Jun 12, 2026 at 04:49:36PM -0700, Dan Williams (nvidia) wrote:
> Xu Yilun wrote:
> > TDX Module introduces a new concept called "TDX Module Extensions" to
> > support long running / hard-irq preemptible flows inside. This makes TDX
> > Module capable of handling complex tasks through "Extension SEAMCALLs".
> > Adding more memory to TDX Module is the first step to enable Extensions.
> 
> Like I said on the cover, I think "long running hard-irq preemptible"
> invites more questions that it answers. The service calls are not "long
> running" on their own. I think it is sufficient to say they are
> resumable unlike typical calls that run to completion while monopolizing
> the CPU.

Yes, I'll drop long running, keep preemptible and resumable.

> 
> > Currently, TDX Module memory use is relatively static. But, the
> > Extensions need to use memory more dynamically. While 'static' here
> > means the kernel provides necessary amount of memory to TDX Module for
> > its basic functionalities, 'dynamic' means extra memory is needed only
> > if new add-on features are to be enabled. So add a new memory feeding
> > process backed by a new SEAMCALL TDH.EXT.MEM.ADD.
> 
> Rick commented on this as well, but a simpler way to say it is
> extensions receive a one time memory pool allocation at init time.  The
> extension uses that pool as its baseline for its own internal state and
> data for the service APIs it offers.

Good to me.

> > For now, TDX Module Extensions consumes relatively large amount of
> > memory (~50MB). Use contiguous page allocation to avoid permanently
> > fragment too much memory. Print the allocation amount on TDX Module
> > Extensions initialization for visibility.
> 
> To be clear I believe there is a low chance of fragmentation given this
> allocation happening early. However, at 10s of MB the benefit of
> isolating blocks of PFNs that will never be returned, it makes to not
> use the buddy allocator for that.

Agree. I'll change it as:

For now, TDX module extensions consume tens of megabytes memory that
will never be returned to host. Use contiguous page allocation to
isolate these large blocks entirely, avoiding permanent memory
fragmentation and reducing buddy allocator efficiency. Print ...


> > +	u64 *root;

...

> > +	root = kzalloc(PAGE_SIZE, GFP_KERNEL);
> > +	if (!root)
> > +		return -ENOMEM;
> 
> I think this "root" term is a holdover from the complicated TDX Connect

Agree. I really don't have to introduce a new "root" page term. The SPEC
says "The HPA_LIST is a 4KB page which contains a list of HPAs", so
hpa_list page is a good name.

> case where it might sometimes be this odd "singleton" object? You could
> just make it this for actual type safety.
> 
> struct tdx_hpa_list {
> 	u64 phys[PAGE_SIZE/sizeof(u64)];
> }
> 
> > +
> > +	page = alloc_contig_pages(nr_pages, GFP_KERNEL, numa_mem_id(),
> > +				  &node_online_map);
> > +	if (!page) {
> > +		ret = -ENOMEM;
> > +		goto out_free_root;
> > +	}
> > +
> > +	for (i = 0; i < nr_pages;) {
> > +		unsigned int nents = min(nr_pages - i,
> > +					 PAGE_SIZE / sizeof(*root));
> 
> This looks wrong, sizeof(struct page)?, or size of physical address?
> 
> Becomes less error prone if you do:
> 
> min(nr_pages - i, ARRAY_SIZE(hpa_list->phys))

OK, let me try.

> > +		ret = tdx_ext_mem_add(virt_to_page(root), nents);
> > +		/*
> > +		 * No SEAMCALLs to reclaim the added pages. For simple error
> > +		 * handling, leak all pages.
> > +		 */
> > +		WARN_ON_ONCE(ret);
> 
> Perhaps to be friendlier to folks without the source code in front of
> them drop the comment and do:
> 
> WARN(ret, "Fatal: TDX Module failed (%d) to accept memory, stranded %ld pages\n", ret, nr_pages)
> 
> ...the once flavor not needed, right? It's toast at this point.

Yes no need the 'once'.

Since I'll print all memory for the extensions anyway below. I'll use:

	WARN(ret, "Fatal: TDX Module rejected (%d) memory for extensions, stranded all pages\n",
	     ret);

Thanks,
Yilun