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

[Huang, Kai]

On Tue, 2026-02-24 at 13:15 +0800, Chao Gao wrote:
> 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.

Oh making kernel parse JSON file is beyond my imagination, but I see you
have a point here :-)

I think my real comment is the sentence 

  The kernel does not validate ...

only describes what does the kernel do today, which is not the case here.

Instead, we are making a design choice here, so I think the sentence should
at least be something like:

  Don't make the kernel validate ...

> 
> > 
> > > 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@xxxxxxxxx>
> > > Reviewed-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>
> > > 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.

Sure.

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

No that's not the main concern.

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

I think I prefer this instead of using the Links.

My concern is the links in the changelog won't be stable.  If that is
acceptable, then that's fine too.

But in the patch 23, you will update the doc anyway, so I think we can just
provide the link there (you already mentioned the repo link there anyway).

> 
> > 
> > [...]
> > 
> > > +/*
> > > + * 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.
> 

I am fine keeping it here.  We need a link "somewhere in _this_ patch" to
review the code I think.

It's in the comment so we can change in the future if it changes.