A module update request is a struct used to describe information about
the TDX module to install. It is part of the P-SEAMLDR <-> kernel ABI
and is accepted by the SEAMLDR_INSTALL SEAMCALL.

The request includes pointers to pages that contain the module binary, a
pointer to a sigstruct file, and an update scenario.

Define the request struct according to the P-SEAMLDR spec [1], and parse
the bitstream from userspace to populate that struct for later module
updates.

Note that the bitstream format is specified in [2]. It consists of a
header, a sigstruct, a module binary, and reserved fields for future
extensions. The header includes fields like a simple checksum and a
signature for error detection.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Tested-by: Farrah Chen <farrah.chen@intel.com>
Link: https://cdrdv2.intel.com/v1/dl/getContent/733584 # [1]
Link: https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt # [2]
---
v3:
 - Print tdx_blob version in hex [Binbin]
 - Drop redundant sigstruct alignment check [Yilun]
 - Note buffers passed from firmware upload infrastructure are
   vmalloc()'d above alloc_seamldr_params()
---
 arch/x86/virt/vmx/tdx/seamldr.c | 158 ++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index d1d4f96c4963..d136ef89cd36 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -6,10 +6,12 @@
  */
 #define pr_fmt(fmt)	"seamldr: " fmt
 
+#include <linux/cleanup.h>
 #include <linux/cpuhplock.h>
 #include <linux/cpumask.h>
 #include <linux/irqflags.h>
 #include <linux/mm.h>
+#include <linux/slab.h>
 #include <linux/types.h>
 
 #include <asm/seamldr.h>
@@ -19,6 +21,26 @@
 /* P-SEAMLDR SEAMCALL leaf function */
 #define P_SEAMLDR_INFO			0x8000000000000000
 
+/* P-SEAMLDR can accept up to 496 4KB pages for TDX module binary */
+#define SEAMLDR_MAX_NR_MODULE_4KB_PAGES	496
+
+/* scenario field in struct seamldr_params */
+#define SEAMLDR_SCENARIO_UPDATE		1
+
+/*
+ * Passed to P-SEAMLDR to describe information about the TDX module to install.
+ * Defined in "SEAM Loader (SEAMLDR) Interface Specification", Revision
+ * 343755-003, Section 3.2.
+ */
+struct seamldr_params {
+	u32	version;
+	u32	scenario;
+	u64	sigstruct_pa;
+	u8	reserved[104];
+	u64	num_module_pages;
+	u64	mod_pages_pa_list[SEAMLDR_MAX_NR_MODULE_4KB_PAGES];
+} __packed;
+
 static struct seamldr_info seamldr_info __aligned(256);
 
 static inline int seamldr_call(u64 fn, struct tdx_module_args *args)
@@ -73,6 +95,137 @@ const struct seamldr_info *seamldr_get_info(void)
 }
 EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
 
+static void free_seamldr_params(struct seamldr_params *params)
+{
+	free_page((unsigned long)params);
+}
+
+/*
+ * Allocate and populate a seamldr_params.
+ * Note that both @module and @sig should be vmalloc'd memory.
+ */
+static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
+						   const void *sig, unsigned int sig_size)
+{
+	struct seamldr_params *params;
+	const u8 *ptr;
+	int i;
+
+	BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
+	if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
+		return ERR_PTR(-EINVAL);
+
+	if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
+	    !IS_ALIGNED((unsigned long)module, SZ_4K) ||
+	    !IS_ALIGNED((unsigned long)sig, SZ_4K))
+		return ERR_PTR(-EINVAL);
+
+	params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
+	if (!params)
+		return ERR_PTR(-ENOMEM);
+
+	params->scenario = SEAMLDR_SCENARIO_UPDATE;
+
+	/*
+	 * Don't assume @sig is page-aligned although it is 4KB-aligned.
+	 * Always add the in-page offset to get the physical address.
+	 */
+	params->sigstruct_pa = (vmalloc_to_pfn(sig) << PAGE_SHIFT) +
+			       ((unsigned long)sig & ~PAGE_MASK);
+	params->num_module_pages = module_size / SZ_4K;
+
+	ptr = module;
+	for (i = 0; i < params->num_module_pages; i++) {
+		params->mod_pages_pa_list[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
+					       ((unsigned long)ptr & ~PAGE_MASK);
+		ptr += SZ_4K;
+	}
+
+	return params;
+}
+
+/*
+ * Intel TDX Module blob. Its format is defined at:
+ * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
+ */
+struct tdx_blob {
+	u16	version;
+	u16	checksum;
+	u32	offset_of_module;
+	u8	signature[8];
+	u32	len;
+	u32	resv1;
+	u64	resv2[509];
+	u8	data[];
+} __packed;
+
+/*
+ * Verify that the checksum of the entire blob is zero. The checksum is
+ * calculated by summing up all 16-bit words, with carry bits dropped.
+ */
+static bool verify_checksum(const struct tdx_blob *blob)
+{
+	u32 size = blob->len;
+	u16 checksum = 0;
+	const u16 *p;
+	int i;
+
+	/* Handle the last byte if the size is odd */
+	if (size % 2) {
+		checksum += *((const u8 *)blob + size - 1);
+		size--;
+	}
+
+	p = (const u16 *)blob;
+	for (i = 0; i < size; i += 2) {
+		checksum += *p;
+		p++;
+	}
+
+	return !checksum;
+}
+
+static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
+{
+	const struct tdx_blob *blob = (const void *)data;
+	int module_size, sig_size;
+	const void *sig, *module;
+
+	if (blob->version != 0x100) {
+		pr_err("unsupported blob version: %x\n", blob->version);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (blob->resv1 || memchr_inv(blob->resv2, 0, sizeof(blob->resv2))) {
+		pr_err("non-zero reserved fields\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* Split the given blob into a sigstruct and a module */
+	sig		= blob->data;
+	sig_size	= blob->offset_of_module - sizeof(struct tdx_blob);
+	module		= data + blob->offset_of_module;
+	module_size	= size - blob->offset_of_module;
+
+	if (sig_size <= 0 || module_size <= 0 || blob->len != size)
+		return ERR_PTR(-EINVAL);
+
+	if (memcmp(blob->signature, "TDX-BLOB", 8)) {
+		pr_err("invalid signature\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!verify_checksum(blob)) {
+		pr_err("invalid checksum\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	return alloc_seamldr_params(module, module_size, sig, sig_size);
+}
+
+DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
+	    if (!IS_ERR_OR_NULL(_T)) free_seamldr_params(_T))
+
 /**
  * seamldr_install_module - Install a new TDX module
  * @data: Pointer to the TDX module binary data. It should be vmalloc'd
@@ -94,6 +247,11 @@ int seamldr_install_module(const u8 *data, u32 size)
 	if (!is_vmalloc_addr(data))
 		return -EINVAL;
 
+	struct seamldr_params *params __free(free_seamldr_params) =
+						init_seamldr_params(data, size);
+	if (IS_ERR(params))
+		return PTR_ERR(params);
+
 	guard(cpus_read_lock)();
 	if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
 		pr_err("Cannot update TDX module if any CPU is offline\n");
-- 
2.47.3

> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> +{
> +	const struct tdx_blob *blob = (const void *)data;
> +	int module_size, sig_size;
> +	const void *sig, *module;

You need to firstly check if size is big enough for the header before
offset into it.

	if (size < sizeof(struct tdx_blob))
		return XXX;

> +/*
> + * Allocate and populate a seamldr_params.
> + * Note that both @module and @sig should be vmalloc'd memory.

Nit:

How about actually using is_vmalloc_addr() to check in the code rather than
documenting in the comment?

I see you have already checked the overall 'data' buffer is vmalloc()'ed in
seamldr_install_module() so the 'module' and 'sig' (part of 'data') must be
too.  But since is_vmalloc_addr() is cheap so I think it's also fine to do
the check here.  We can also WARN() so it can be used to catch bug.

> + */
> +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
> +						   const void *sig, unsigned int sig_size)
> +{
> 

[...]

> +	ptr = module;
> +	for (i = 0; i < params->num_module_pages; i++) {
> +		params->mod_pages_pa_list[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
> +					       ((unsigned long)ptr & ~PAGE_MASK);
> +		ptr += SZ_4K;
> +	}
> +
> +	return params;
> +}
> 

[...]

> +/*
> + * Verify that the checksum of the entire blob is zero. The checksum is
> + * calculated by summing up all 16-bit words, with carry bits dropped.
> + */
> +static bool verify_checksum(const struct tdx_blob *blob)
> +{
> +	u32 size = blob->len;
> +	u16 checksum = 0;
> +	const u16 *p;
> +	int i;
> +
> +	/* Handle the last byte if the size is odd */
> +	if (size % 2) {
> +		checksum += *((const u8 *)blob + size - 1);
> +		size--;
> +	}
> +
> +	p = (const u16 *)blob;
> +	for (i = 0; i < size; i += 2) {
> +		checksum += *p;
> +		p++;
> +	}
> +
> +	return !checksum;
> +}
> +
> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> +{
> 

[...]

> +	if (!verify_checksum(blob)) {
> +		pr_err("invalid checksum\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return alloc_seamldr_params(module, module_size, sig, sig_size);
> +}

It's weird that we have do verify checksum manually, because hardware
normally catches that.

I suppose this is because we want to catch as many errors as possible before
actually asking P-SEAMLDR to do module update, since in order to do which we
have to shutdown the existing module first and there's no returning point
once we reach that?

If so a comment would be helpful.

Also, it's also weird that you have to write code for checksum on your own.
I guess the kernel should already have some library code for that.

I checked and it _seems_ the code in lib/checksum.c could be used?

I am not expert though, but I think we should use kernel lib code when we
can.

On Wed, Jan 28, 2026 at 12:03:25PM +0800, Huang, Kai wrote:
>
>> +/*
>> + * Allocate and populate a seamldr_params.
>> + * Note that both @module and @sig should be vmalloc'd memory.
>
>Nit:
>
>How about actually using is_vmalloc_addr() to check in the code rather than
>documenting in the comment?
>
>I see you have already checked the overall 'data' buffer is vmalloc()'ed in
>seamldr_install_module() so the 'module' and 'sig' (part of 'data') must be
>too.  But since is_vmalloc_addr() is cheap so I think it's also fine to do
>the check here.  We can also WARN() so it can be used to catch bug.

Kai,

Thanks a lot.

Looks good to me. I think WARN() is always better than comments.

>> +	if (!verify_checksum(blob)) {
>> +		pr_err("invalid checksum\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	return alloc_seamldr_params(module, module_size, sig, sig_size);
>> +}
>
>It's weird that we have do verify checksum manually, because hardware
>normally catches that.
>
>I suppose this is because we want to catch as many errors as possible before
>actually asking P-SEAMLDR to do module update, since in order to do which we
>have to shutdown the existing module first and there's no returning point
>once we reach that?

Yes. Exactly.

>
>If so a comment would be helpful.

Will do.

>
>Also, it's also weird that you have to write code for checksum on your own.
>I guess the kernel should already have some library code for that.
>
>I checked and it _seems_ the code in lib/checksum.c could be used?
>
>I am not expert though, but I think we should use kernel lib code when we
>can.

Good point. After a quick review, lib/checksum.c uses a different algorithm
than tdx_blob's checksum. It adds the carry bit to the checksum, while tdx_blob
drops the carry bit.

*sigh* when I designed the checksum algorithm, I wasn't aware of lib/checksum.c.

> +/*
> + * Allocate and populate a seamldr_params.
> + * Note that both @module and @sig should be vmalloc'd memory.
> + */
> +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
> +						   const void *sig, unsigned int sig_size)
> +{
> +	struct seamldr_params *params;
> +	const u8 *ptr;
> +	int i;
> +
> +	BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
> +	if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
> +	    !IS_ALIGNED((unsigned long)module, SZ_4K) ||
> +	    !IS_ALIGNED((unsigned long)sig, SZ_4K))
> +		return ERR_PTR(-EINVAL);
> +

Based on the the blob format link below, we have 

struct tdx_blob
{
	...
	_u64 sigstruct[256]; // 2KB sigstruct,intel_tdx_module.so.sigstruct
	_u64 reserved2[256]; // Reserved space
	...
}

So it's clear SIGSTRUCT is just 2KB and the second half 2KB is "reserved
space".

Why is the "reserved space" treated as part of SIGSTRUCT here? 

> +
> +/*
> + * Intel TDX Module blob. Its format is defined at:
> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
> + */
> +struct tdx_blob {
> +	u16	version;
> +	u16	checksum;
> +	u32	offset_of_module;
> +	u8	signature[8];
> +	u32	len;
> +	u32	resv1;
> +	u64	resv2[509];

Nit:  Perhaps s/resv/rsvd ?

"#grep rsvd arch/x86 -Rn" gave me a bunch of results but "#grep resv" gave
me much less (and part of the results were 'resvd' and 'resv_xx' instead of
plain 'resv').
  
> +	u8	data[];
> +} __packed;

For this structure, I need to click the link and open it in a browser to
understand where is the sigstruct and module, and ...

> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> +{
> +	const struct tdx_blob *blob = (const void *)data;
> +	int module_size, sig_size;
> +	const void *sig, *module;
> +
> +	if (blob->version != 0x100) {
> +		pr_err("unsupported blob version: %x\n", blob->version);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	if (blob->resv1 || memchr_inv(blob->resv2, 0, sizeof(blob->resv2))) {
> +		pr_err("non-zero reserved fields\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	/* Split the given blob into a sigstruct and a module */
> +	sig		= blob->data;
> +	sig_size	= blob->offset_of_module - sizeof(struct tdx_blob);
> +	module		= data + blob->offset_of_module;
> +	module_size	= size - blob->offset_of_module;
> +

... to see whether this code makes sense.

I understand the

	...
	u64	rsvd[N*512];
	u8	module[];

is painful to be declared explicitly in 'struct tdx_blob' because IIUC we
cannot put two flexible array members at the end of the structure.

But I think if we add 'sigstruct' to the 'struct tdx_blob', e.g.,

struct tdx_blob {
	u16	version;
	...
	u64	rsvd2[509];
	u64	sigstruct[256];
	u64	rsvd3[256];
	u64	data;
} __packed;

.. we can just use

	sig		= blob->sigstruct;
	sig_size	= 2K (or 4K I don't quite follow);

which is clearer to read IMHO?

> +	return alloc_seamldr_params(module, module_size, sig, sig_size);
> +}
> +

On Tue, Jan 27, 2026 at 11:21:06AM +0800, Huang, Kai wrote:
>
>> +/*
>> + * Allocate and populate a seamldr_params.
>> + * Note that both @module and @sig should be vmalloc'd memory.
>> + */
>> +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
>> +						   const void *sig, unsigned int sig_size)
>> +{
>> +	struct seamldr_params *params;
>> +	const u8 *ptr;
>> +	int i;
>> +
>> +	BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
>> +	if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
>> +	    !IS_ALIGNED((unsigned long)module, SZ_4K) ||
>> +	    !IS_ALIGNED((unsigned long)sig, SZ_4K))
>> +		return ERR_PTR(-EINVAL);
>> +
>
>Based on the the blob format link below, we have 
>
>struct tdx_blob
>{
>	...
>	_u64 sigstruct[256]; // 2KB sigstruct,intel_tdx_module.so.sigstruct
>	_u64 reserved2[256]; // Reserved space
>	...
>}
>
>So it's clear SIGSTRUCT is just 2KB and the second half 2KB is "reserved
>space".
>
>Why is the "reserved space" treated as part of SIGSTRUCT here? 

Good question. Because the space is reserved for sigstruct expansion.

The __current__ SEAMLDR ABI accepts one 4KB page, but all __existing__
sigstructs are only 2KB. so, tdx_blob currently defines a 2KB sigstruct field
followed by 2KB of reserved space. We anticipate that sigstructs will
eventually exceed 4KB, so we added reserved3[N*512] to accommodate future
growth.

You're right. The current tdx_blob definition doesn't clearly indicate that
reserved2/3 are actually part of the sigstruct.

Does this revised tdx_blob definition make that clearer and better align with
this patch? The idea is to make tdx_blob generic enough to clearly represent:
a 4KB header, followed by 4KB-aligned sigstruct, followed by the TDX Module
binary. Current SEAMLDR ABI details or current sigstruct sizes are irrelevant.

struct tdx_blob
{
        _u16 version;              // Version number
        _u16 checksum;             // Checksum of the entire blob should be zero
        _u32 offset_of_module;     // Offset of the module binary intel_tdx_module.bin in bytes
        _u8  signature[8];         // Must be "TDX-BLOB"
        _u32 length;               // The length in bytes of the entire blob
        _u32 reserved0;            // Reserved space
        _u64 reserved1[509];       // Reserved space
        _u64 sigstruct[512 + N*512]; // sigstruct, 4KB aligned

	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        _u8  module[];             // intel_tdx_module.bin, 4KB aligned, to the end of the file
}


>
>> +
>> +/*
>> + * Intel TDX Module blob. Its format is defined at:
>> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
>> + */
>> +struct tdx_blob {
>> +	u16	version;
>> +	u16	checksum;
>> +	u32	offset_of_module;
>> +	u8	signature[8];
>> +	u32	len;
>> +	u32	resv1;
>> +	u64	resv2[509];
>
>Nit:  Perhaps s/resv/rsvd ?
>

Sure. Will do.

>"#grep rsvd arch/x86 -Rn" gave me a bunch of results but "#grep resv" gave
>me much less (and part of the results were 'resvd' and 'resv_xx' instead of
>plain 'resv').
>  
>> +	u8	data[];
>> +} __packed;
>
>For this structure, I need to click the link and open it in a browser to
>understand where is the sigstruct and module, and ...
>
>> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
>> +{
>> +	const struct tdx_blob *blob = (const void *)data;
>> +	int module_size, sig_size;
>> +	const void *sig, *module;
>> +
>> +	if (blob->version != 0x100) {
>> +		pr_err("unsupported blob version: %x\n", blob->version);
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	if (blob->resv1 || memchr_inv(blob->resv2, 0, sizeof(blob->resv2))) {
>> +		pr_err("non-zero reserved fields\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	/* Split the given blob into a sigstruct and a module */
>> +	sig		= blob->data;
>> +	sig_size	= blob->offset_of_module - sizeof(struct tdx_blob);
>> +	module		= data + blob->offset_of_module;
>> +	module_size	= size - blob->offset_of_module;
>> +
>
>... to see whether this code makes sense.
>
>I understand the
>
>	...
>	u64	rsvd[N*512];
>	u8	module[];
>
>is painful to be declared explicitly in 'struct tdx_blob' because IIUC we
>cannot put two flexible array members at the end of the structure.

Yes.

>
>But I think if we add 'sigstruct' to the 'struct tdx_blob', e.g.,
>
>struct tdx_blob {
>	u16	version;
>	...
>	u64	rsvd2[509];
>	u64	sigstruct[256];
>	u64	rsvd3[256];
>	u64	data;
>} __packed;
>
>.. we can just use
>
>	sig		= blob->sigstruct;
>	sig_size	= 2K (or 4K I don't quite follow);
>
>which is clearer to read IMHO?

The problem is hard-coding the sigstruct size to 2KB/4KB. This will soon no
longer hold.

But
	sig		= blob->data;
	sig_size	= blob->offset_of_module - sizeof(struct tdx_blob);

doesn't make that assumption, making it more future-proof.

>
>> +	return alloc_seamldr_params(module, module_size, sig, sig_size);
>> +}
>> +
>
>
>

On Wed, 2026-01-28 at 19:28 +0800, Gao, Chao wrote:
> On Tue, Jan 27, 2026 at 11:21:06AM +0800, Huang, Kai wrote:
> > 
> > > +/*
> > > + * Allocate and populate a seamldr_params.
> > > + * Note that both @module and @sig should be vmalloc'd memory.
> > > + */
> > > +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
> > > +						   const void *sig, unsigned int sig_size)
> > > +{
> > > +	struct seamldr_params *params;
> > > +	const u8 *ptr;
> > > +	int i;
> > > +
> > > +	BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
> > > +	if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> > > +		return ERR_PTR(-EINVAL);
> > > +
> > > +	if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
> > > +	    !IS_ALIGNED((unsigned long)module, SZ_4K) ||
> > > +	    !IS_ALIGNED((unsigned long)sig, SZ_4K))
> > > +		return ERR_PTR(-EINVAL);
> > > +
> > 
> > Based on the the blob format link below, we have 
> > 
> > struct tdx_blob
> > {
> > 	...
> > 	_u64 sigstruct[256]; // 2KB sigstruct,intel_tdx_module.so.sigstruct
> > 	_u64 reserved2[256]; // Reserved space
> > 	...
> > }
> > 
> > So it's clear SIGSTRUCT is just 2KB and the second half 2KB is "reserved
> > space".
> > 
> > Why is the "reserved space" treated as part of SIGSTRUCT here? 
> 
> Good question. Because the space is reserved for sigstruct expansion.
> 
> The __current__ SEAMLDR ABI accepts one 4KB page, but all __existing__
> sigstructs are only 2KB. 
> 

Oh I see.

I think we have two perspectives here: 1) what P-SEAMLDR ABI requires for
module and sigstruct; 2) how does the kernel get them and pass to
alloc_seamldr_params().

IIUC, I now understand alloc_seamldr_params() is expecting the 'module',
'module_size', 'sig' and 'sig_size' to meet P-SEAMCALL's ABI.

Then would it be better to add a comment for the checks of 'module',
'module_size', 'sig' and 'sig_size' in alloc_seamldr_params() (below code)
that it is P-SEAMCALL ABI that has these requirement?

	if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
	    !IS_ALIGNED((unsigned long)module, SZ_4K) ||
	    !IS_ALIGNED((unsigned long)sig, SZ_4K))
		return ERR_PTR(-EINVAL);

Otherwise it's a bit confusing because these 4 arguments are passed to
alloc_seamldr_params() right from the layout of 'struct tdx_blob' which is a
"software-organized" structure which, theoretically, could have nothing to
do P-SEAMLDR ABI.


> so, tdx_blob currently defines a 2KB sigstruct field
> followed by 2KB of reserved space. We anticipate that sigstructs will
> eventually exceed 4KB, so we added reserved3[N*512] to accommodate future
> growth.
> 
> You're right. The current tdx_blob definition doesn't clearly indicate that
> reserved2/3 are actually part of the sigstruct.
> 
> Does this revised tdx_blob definition make that clearer and better align with
> this patch? 
> 

Yes it's clearer, from the perspective that how it matches your code to
calculate 'sig_size'.

> The idea is to make tdx_blob generic enough to clearly represent:
> a 4KB header, followed by 4KB-aligned sigstruct, followed by the TDX Module
> binary. Current SEAMLDR ABI details or current sigstruct sizes are irrelevant.
> 
> struct tdx_blob
> {
>         _u16 version;              // Version number
>         _u16 checksum;             // Checksum of the entire blob should be zero
>         _u32 offset_of_module;     // Offset of the module binary intel_tdx_module.bin in bytes
>         _u8  signature[8];         // Must be "TDX-BLOB"
>         _u32 length;               // The length in bytes of the entire blob
>         _u32 reserved0;            // Reserved space
>         _u64 reserved1[509];       // Reserved space
>         _u64 sigstruct[512 + N*512]; // sigstruct, 4KB aligned
> 
> 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>         _u8  module[];             // intel_tdx_module.bin, 4KB aligned, to the end of the file
> }
> 
> 
> > 

A side topic:

I checked the SEAMLDR.INSTALL.  It appears the only requirement of the
SIGSTRUCT is it is 4K aligned.  There's no where in the ABI (certainly not
in SEAMLDR_PARAMS) to tell how does SEAMLDR.INSTALL verifies the size of
SIGSTRUCT.

Is this right?

When we bumping SIGSTRUCT to a larger size, do we have some kinda
enumeration that reports such?

From your patch 24, IIUC I don't see such enumeration or explicit opt-in,
because you just changes the layout of SEAMLDR_PARAM w/o even changing it's
version.

[...]

> > But I think if we add 'sigstruct' to the 'struct tdx_blob', e.g.,
> > 
> > struct tdx_blob {
> > 	u16	version;
> > 	...
> > 	u64	rsvd2[509];
> > 	u64	sigstruct[256];
> > 	u64	rsvd3[256];
> > 	u64	data;
> > } __packed;
> > 
> > .. we can just use
> > 
> > 	sig		= blob->sigstruct;
> > 	sig_size	= 2K (or 4K I don't quite follow);
> > 
> > which is clearer to read IMHO?
> 
> The problem is hard-coding the sigstruct size to 2KB/4KB. This will soon no
> longer hold.
> 
> But
> 	sig		= blob->data;
> 	sig_size	= blob->offset_of_module - sizeof(struct tdx_blob);
> 
> doesn't make that assumption, making it more future-proof.

Sure.  I am certainly fine with making it future-proof (albeit arguably you
could also change the way that how sig_size is calculated in the future,
i.e., in your patch 24).

But the real point is the code here needs to reflect the 'struct tdx_blob'
description in the doc.  But with the current doc I don't see they match to
each other:

  The doc says SIGSTRUCT is 2K but the code says it's 4K.

So I think you need to update the 'struct tdx_blob' description in the doc
to justify such code.

Btw, I think the link

  https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt

is subject to change, both the link itself and it's content.

Do you think we should just make the layout of 'struct tdx_blob' as a
documentation patch and include that to this series?

On Fri, Jan 23, 2026 at 06:55:21AM -0800, Chao Gao wrote:
> A module update request is a struct used to describe information about
> the TDX module to install. It is part of the P-SEAMLDR <-> kernel ABI
> and is accepted by the SEAMLDR_INSTALL SEAMCALL.
> 
> The request includes pointers to pages that contain the module binary, a
> pointer to a sigstruct file, and an update scenario.
> 
> Define the request struct according to the P-SEAMLDR spec [1], and parse
> the bitstream from userspace to populate that struct for later module
> updates.

Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>