Re: [RFC PATCH v2 2/2] x86/virt/seamldr: Support multiple sigstruct pages

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

On Fri, 2026-11-06 at 05:21 -0800, Chao Gao wrote:
> Each TDX module update image carries a sigstruct, which is passed along
> with the module binary to P-SEAMLDR. 'struct seamldr_params' has a single
> physical address slot for it, so a sigstruct can be at most one 4KB page.

"it" could refer to the module binary or the sigstruct, until you piece it
together at the end. Also struct seamldr_params could be introduced. To answer:
Is it the TDX module ABI? Or the kernels internal struct?

> 
> TDX modules are moving to a post-quantum signature algorithm. Its
> signatures are far larger than the ones in use today, so the sigstruct
> no longer fits in 4KB.
> 
> The P-SEAMLDR ABI was extended for this. It now has four sigstruct
> address slots instead of one, for sigstructs of up to 16KB. A new
> 'version' value of 1 tells P-SEAMLDR that this layout is in use.
> 
> Extend 'struct seamldr_params' to match the new ABI. The sigstruct
> address list is already populated in a loop over all sigstruct pages, so
> raising SEAMLDR_MAX_NR_SIG_PAGES is enough to fill the new slots

Missing period. Also, the existing loop is a bit strange. I wonder if it needs
to be explained, but the explanation is probably that it was not adapted
properly after this change was removed from the original series.

> 
> Set 'version' to 1 only when the sigstruct spans more than one page. Old
> P-SEAMLDRs reject version 1, so this keeps updates with a single-page
> sigstruct working on them.
> 
> Do not query P-SEAMLDR for version 1 support. Userspace selects the
> update image and is responsible for ensuring the module is compatible
> with P-SEAMLDR.
> 
> AI was used under supervision to review code and workshop logs

Missing period. But this phrase is starting to become kind of boiler plate in
all of these patches. I wonder if it's still needed. AI review is now the
exception. Maybe move it below the "---" for this time? Dave can move it up
easily if he thinks it helps.

> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---

Otherwise lgtm.

> v2:
>  - drop macros for versions to keep the diff minimal
>  - explain why the sigstruct filling code needs no change
> ---
>  arch/x86/virt/vmx/tdx/seamldr.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 087bdf10a9a7..9cef6fff2a6c 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -36,7 +36,7 @@
>  #define SEAMLDR_PARAMS_NR_PAGES		16
>  #define SEAMLDR_NR_MODULE_PA_ENTRIES	(496 + 512 * (SEAMLDR_PARAMS_NR_PAGES - 1))
>  #define SEAMLDR_MAX_NR_MODULE_PAGES	(SEAMLDR_NR_MODULE_PA_ENTRIES - SEAMLDR_PARAMS_NR_PAGES)
> -#define SEAMLDR_MAX_NR_SIG_PAGES	1
> +#define SEAMLDR_MAX_NR_SIG_PAGES	4
>  
>  /*
>   * The seamldr_params "scenario" field specifies the operation mode:
> @@ -57,7 +57,7 @@ struct seamldr_params {
>  	u32	version;
>  	u32	scenario;
>  	u64	sigstruct_pages_pa_list[SEAMLDR_MAX_NR_SIG_PAGES];
> -	u8	reserved[104];
> +	u8	reserved[80];
>  	u64	module_nr_pages;
>  	u64	module_pages_pa_list[SEAMLDR_NR_MODULE_PA_ENTRIES];
>  } __packed;
> @@ -166,6 +166,13 @@ static void populate_seamldr_params(struct seamldr_params *params,
>  				    const u8 *mod, u32 mod_nr_pages)
>  {
>  	params->version		= 0;
> +	/*
> +	 * Use version 1 only for multi-page sigstructs, to stay
> +	 * compatible with old P-SEAMLDRs that lack version 1 support.
> +	 */
> +	if (sig_nr_pages > 1)
> +		params->version	= 1;
> +
>  	params->scenario	= SEAMLDR_SCENARIO_UPDATE;
>  	params->module_nr_pages	= mod_nr_pages;
>