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

[Chao Gao <chao.gao@intel.com>]

On Thu, Nov 12, 2026 at 01:09:52AM +0800, Xu Yilun wrote:
>On Fri, Nov 06, 2026 at 05:21:55AM -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
>
>Is it better we specify why 496 entries?
>
>  The struct seamldr_params is contained in a 4KB page, the first 128
>  bytes of the page are the metadata, the rests are the list of page
>  phsical addresses for the module binary.

...

>
>
>> caps a module at 496 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
>
>With the knowledge of the first 4KB page, it is nature to say the ABI
>extends the list to more 4KB pages.
>
>> 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
>
>Maybe we don't have to emphasize the 495/496? The last entry of each
>page points to the next page.

I would rather drop 496 and 495 from these paragraphs. Readers do not
need the exact limit here, since the code has the numbers. How about:


The module binary is handed to P-SEAMLDR as a list of page physical
addresses in 'struct seamldr_params'. The list must fit in a single 4KB
page along with a few other fields, which caps a module at just under
2MB.

TDX modules are gaining features and are about to outgrow that limit.

The P-SEAMLDR ABI was extended to continue the list on more chained 4KB
pages. The last u64 of each page holds the physical address of the next
page in the chain.

>
>> page can point to yet another page, and so on, until every module page
>> is listed.
>> 
>
>[...]
>
>> -#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))
>
>The number 496 is not meaningful when the seamldr_params is no longer
>one page size. We need comments to explain how it is derivee. But instead
>of text explanation, is it better:
>
>#define SEAMLDR_NR_MODULE_PA_ENTRIES	(SEAMLDR_PARAMS_NR_PAGES * 512 - 16)

The 16 here would still need explaining. It would also be confusing,
because the macro right below subtracts a different 16
(SEAMLDR_PARAMS_NR_PAGES) to get SEAMLDR_MAX_NR_MODULE_PAGES.

>
>Or:
>
>#define SEAMLDR_NR_MODULE_PA_ENTRIES	\
>	((SEAMLDR_PARAMS_NR_PAGES * PAGE_SIZE -	\
>	  offsetof(struct seamldr_params, module_pages_pa_list)) / \
>	 sizeof(u64))

offsetof() does not work here. The macro sizes module_pages_pa_list[]
inside 'struct seamldr_params', where the struct is still incomplete. The
compiler reports:

	error: invalid use of undefined type 'struct seamldr_params

How about:

/*
 * module_pages_pa_list[] fills the rest of 'struct seamldr_params' after
 * its first 128 bytes. The last entry of each page is reserved to point
 * to the next page and cannot hold a module address. The last entry of
 * the final page is never used.
 */
#define SEAMLDR_NR_MODULE_PA_ENTRIES	(((SEAMLDR_PARAMS_NR_PAGES * PAGE_SIZE - 128)) / sizeof(u64))
#define SEAMLDR_MAX_NR_MODULE_PAGES	(SEAMLDR_NR_MODULE_PA_ENTRIES - SEAMLDR_PARAMS_NR_PAGES)

>
>> +#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);