Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request

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

On Fri, Feb 20, 2026 at 06:31:24AM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> P-SEAMLDR uses the SEAMLDR_PARAMS structure to describe TDX Module
>> update requests. This structure contains physical addresses pointing to
>> the module binary and its signature file (or sigstruct), along with an
>> update scenario field.
>> 
>> TDX Modules are distributed in the tdx_blob format defined at [1]. A
>> tdx_blob contains a header, sigstruct, and module binary. This is also
>> the format supplied by the userspace to the kernel.
>> 
>> Parse the tdx_blob format and populate a SEAMLDR_PARAMS structure
>> accordingly. This structure will be passed to P-SEAMLDR to initiate the
>> update.
>> 
>> Note that the sigstruct_pa field in SEAMLDR_PARAMS has been extended to
>> a 4-element array. The updated "SEAM Loader (SEAMLDR) Interface
>> Specification" will be published separately. The kernel does not
>> validate P-SEAMLDR compatibility (for example, whether it supports 4KB
>> or 16KB sigstruct); 
>> 
>
>Nit:
>
>This sounds like the kernel can validate but chooses not to.  But I thought
>the fact is the kernel cannot validate because there's no P-SEAMLDR ABI to
>enumerate such compatibility?

Emm, the kernel could validate this by parsing mapping_file.json, but the
complexity wouldn't be worth it.

>
>> userspace must ensure the P-SEAMLDR version is
>> compatible with the selected TDX Module by checking the minimum
>> P-SEAMLDR version requirements at [2].
>> 
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/blob_structure.txt # [1]
>> Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/mapping_file.json # [2]
>> 
>
>Nit:
>
>As mentioned in v3, can the link be considered as "stable", e.g., won't
>disappear couple of years later?

I'm not sure when this link will be outdated, but we'll definitely have a TDX
Module release repository with a blob_structure.txt file describing the format.

>
>Not sure we should just have a documentation patch for 'tdx_blob' layout.  I
>suspect the content won't be changed in the future anyway, at least for
>foreseeable future, given you have already updated the sigstruct part.
>
>We can include the links to the actual doc too, and if necessarily, point
>out the links may get updated in the future.  We can actually update the
>links if they are in some doc.

Regarding the documentation patch, I don't see the value in adding one. It
would just mirror the code and become outdated when 'tdx_blob' layout is
updated.

If the concern is that tdx_blob layout changes could cause incompatibilities,
that's not the kernel's responsibility to prevent; the kernel has no control
over external format changes.

If the issue is simply that links may become outdated, that's a common problem.
We can address this by referring to blob_structure.txt in the "Intel TDX Module
Binaries Repository" and dropping the specific link. For example:

  TDX Modules are distributed in the tdx_blob format defined in
  blob_structure.txt from the "Intel TDX Module Binaries Repository". A
  tdx_blob contains a header, sigstruct, and module binary. This is also the
  format supplied by the userspace to the kernel.

>
>[...]
>
>> +/*
>> + * Intel TDX Module blob. Its format is defined at:
>> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt

I will drop this link as well.

>> + *
>> + * Note this structure differs from the reference above: the two variable-length
>> + * fields "@sigstruct" and "@module" are represented as a single "@data" field
>> + * here and split programmatically using the offset_of_module value.
>> + */
>> +struct tdx_blob {
>> +	u16	version;
>> +	u16	checksum;
>> +	u32	offset_of_module;
>> +	u8	signature[8];
>> +	u32	length;
>> +	u32	resv0;
>> +	u64	resv1[509];
>> +	u8	data[];
>> +} __packed;
>
>Nit:
>
>It appeared you said you will s/resv/rsvd in v3.
>
>I don't quite mind if other people are fine with 'resv'.  Or you can spell
>out 'reserved' in full to match the one in 'struct seamldr_params' above.

Sorry, I missed this feedback. I'll use "reserved".

I even updated "len" to "length" and changed the index to start from 0 (to match
blob_structure.txt) but somehow missed updating "resv."