Re: [RFC PATCH v2 1/2] x86/virt/seamldr: Support large TDX module binaries

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Fri, 2026-11-06 at 05:21 -0800, Chao Gao wrote:
> The module binary is handed to P-SEAMLDR as a list of page physical
> addresses in 'struct seamldr_params'. The list has 496 entries, which
> caps a module at 496 pages.

nit. change it to "...caps a module at the same number of pages".

> 
> TDX modules are gaining features and are about to outgrow that limit.
> 
> The P-SEAMLDR ABI was extended to chain the list across 4KB pages. For
> a module over 495 pages, the last of the 496 entries points to another
> page of entries instead of to a module page. The last entry of that
> page can point to yet another page, and so on, until every module page
> is listed.
> 
> Grow module_pages_pa_list[] so that 'struct seamldr_params' spans
> multiple pages (16 for now), and use the extra pages as the chained
> pages. The last entry of each page then points to the entry right after
> it, which starts the next page. These link entries do not hold module
> pages, so leave them out of the maximum module page count.
> 
> The 16-page size is somewhat arbitrary. It fits modules of up to about
> 32MB, far above the roughly 2MB of current modules. It wastes 60KB for
> today's modules, which is fine because the memory is freed right after
> the update. Sizing module_pages_pa_list[] per module would mean either
> moving the allocation after the image header is validated, or allocating
> from an unvalidated page count. Either way it is more code churn.
> 
> Do not query P-SEAMLDR for chaining support. Userspace selects the update
> image and is responsible for ensuring the module is compatible with
> P-SEAMLDR.
> 
> Thanks to Yilun for the idea of allocating the chain pages together
> with 'struct seamldr_params'.

I'm not sure what this means.

> 
> AI was used under supervision to review code and workshop logs.

Same comment as on the next patch.

> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
> v2:
>  - Make 'struct seamldr_params' a fixed 16 pages instead of sizing it
>    to each module, capping modules at about 32MB. [Rick]
>  - Drop the patch that moved the allocation after image validation. [Rick]
> ---
>  arch/x86/virt/vmx/tdx/seamldr.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index b1137ca6150d..087bdf10a9a7 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -24,7 +24,18 @@
>  #define P_SEAMLDR_INFO			0x8000000000000000
>  #define P_SEAMLDR_INSTALL		0x8000000000000001
>  
> -#define SEAMLDR_MAX_NR_MODULE_PAGES	496
> +/*
> + * module_pages_pa_list[] spans multiple pages. The last entry of each
> + * page points to the next page and does not hold a module page, so the
> + * maximum number of module pages is the array size minus one entry per
> + * page.
> + *
> + * SEAMLDR_PARAMS_NR_PAGES is somewhat arbitrary. It allows modules of
> + * up to about 32MB. Increase it if modules outgrow that.
> + */
> +#define SEAMLDR_PARAMS_NR_PAGES		16
> +#define SEAMLDR_NR_MODULE_PA_ENTRIES	(496 + 512 * (SEAMLDR_PARAMS_NR_PAGES - 1))

I think you should describe the math a little bit here. 496 is the space in the
original format, but the last one is now a pa pointer, right? So can't be used
as a page of module data. And then each successive page is 511 entries and then
a pa pointer? So is the math actually right?

> +#define SEAMLDR_MAX_NR_MODULE_PAGES	(SEAMLDR_NR_MODULE_PA_ENTRIES - SEAMLDR_PARAMS_NR_PAGES)
>  #define SEAMLDR_MAX_NR_SIG_PAGES	1
>  
>  /*
> @@ -48,10 +59,10 @@ struct seamldr_params {
>  	u64	sigstruct_pages_pa_list[SEAMLDR_MAX_NR_SIG_PAGES];
>  	u8	reserved[104];
>  	u64	module_nr_pages;
> -	u64	module_pages_pa_list[SEAMLDR_MAX_NR_MODULE_PAGES];
> +	u64	module_pages_pa_list[SEAMLDR_NR_MODULE_PA_ENTRIES];
>  } __packed;
>  
> -static_assert(sizeof(struct seamldr_params) == 4096);
> +static_assert(sizeof(struct seamldr_params) == SEAMLDR_PARAMS_NR_PAGES * PAGE_SIZE);
>  
>  /*
>   * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> @@ -129,17 +140,24 @@ struct tdx_image {
>  /*
>   * Given a vmalloc() allocation, write all of the backing physical
>   * addresses to pa_list[]. Caller guarantees that the array is big
> - * enough.
> + * enough. If pa_list[] spans multiple pages, point the last entry
> + * of each page to the next page.
>   */
>  static void populate_pa_list(u64 *pa_list, const u8 *vmalloc_addr, u32 vmalloc_len_pages)
>  {
> -	int i;
> +	int i, j;
>  
> -	for (i = 0; i < vmalloc_len_pages; i++) {
> +	for (i = 0, j = 0; i < vmalloc_len_pages; i++) {
>  		unsigned long offset = i * PAGE_SIZE;
>  		unsigned long pfn = vmalloc_to_pfn(&vmalloc_addr[offset]);
>  
> -		pa_list[i] = pfn << PAGE_SHIFT;

How about a little:
/* If the current entry is the last in the page. */

> +		if (PAGE_ALIGNED(&pa_list[j + 1])) {
> +			pa_list[j] = __pa(&pa_list[j + 1]);
> +			j++;
> +		}
> +
> +		pa_list[j] = pfn << PAGE_SHIFT;
> +		j++;
>  	}
>  }
>