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

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

On Tue, Nov 10, 2026 at 06:12:32AM +0800, Edgecombe, Rick P wrote:
>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?

Agreed. How about:

Each TDX module update image carries a sigstruct, which is passed to
P-SEAMLDR along with the module binary. The P-SEAMLDR ABI ('struct
seamldr_params') has a single physical address slot for the sigstruct,
so the sigstruct can be at most one 4KB page.

>
>> 
>> 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.

Will fix the period.

The loop was intentional. Commit 23a81e6cce15 ("x86/virt/seamldr: Allocate
and populate a module update request") kept sigstruct_pages_pa_list as an
array because it was expected to grow to four pages. This was called out in
the changelog of that commit.

I would add one sentence to say so:

  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. The
  loop was written that way in anticipation of this change.

>
>> 
>> 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.

Will drop this phrase from both patches.