mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs
@ 2026-10-30  8:52 Chao Gao
  2026-10-30  8:52 ` [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation Chao Gao
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Chao Gao @ 2026-10-30  8:52 UTC (permalink / raw)
  To: x86, linux-coco, kvm, linux-kernel
  Cc: yilun.xu, arjan.van.de.ven, Chao Gao, Kiryl Shutsemau,
	Rick Edgecombe, Dave Hansen, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin

This series lets TDX module updates handle large TDX modules and
sigstructs. It is an RFC for early review.

Dave, feel free to ignore this RFC for now. TDX developers, please take
a look.

I would especially like feedback on one question. Does the kernel need
to tell userspace that it supports large modules and sigstructs? I would
rather not add such an interface.

Userspace can simply try such an image, because a kernel without this
series rejects it with -EINVAL before calling P-SEAMLDR, so a failed try
has no side effects. If userspace should not have to try an update to
find out, the alternative is to backport this series to all kernels that
support TDX module updates. Userspace could then assume that any kernel
with TDX module updates handles such images.

====

This series is a follow-up to the TDX module update series. As thanks
for merging that series, we agreed to take on the TDX metadata cleanup [1].
With the cleanup converging, we are starting on this work and expect it
to be merged after the cleanup.

Today a TDX module binary can be at most 496 pages and its sigstruct at
most one page. Both limits come from the P-SEAMLDR ABI.

TDX modules are outgrowing them. A post-quantum signature algorithm
pushes the sigstruct past 4KB, and new module features push the binary
past 496 pages. The P-SEAMLDR ABI was extended for both.

The kernel rejects any image over these limits. TDX modules for Diamond
Rapids exceed them, so without this series, TDX module updates fail on
Diamond Rapids. This series is all that is needed to make them work.

The image format that userspace uploads ('struct tdx_image') is generic
enough and needs no change.

== AI Usage ==
AI was used under supervision to review code and workshop logs. It
spotted a u32 overflow and an oversized allocation that removing the 496
page cap would cause in patch 2. The module size limit in that patch
prevents both. It also suggested renaming init_seamldr_params() in
patch 1 and updating the comments around it.

== Testing ==
Tested on a Diamond Rapids system. Updates with a module larger than 2MB
and an 8KB sigstruct work, which exercises both the chained page list
and version 1 of 'struct seamldr_params'. TDs keep running across
updates.

[1]: https://lore.kernel.org/kvm/20261012094754.27702-1-chao.gao@intel.com/

Chao Gao (3):
  x86/virt/seamldr: Allocate 'struct seamldr_params' after image
    validation
  x86/virt/seamldr: Support large TDX module binaries
  x86/virt/seamldr: Support multiple sigstruct pages

 arch/x86/virt/vmx/tdx/seamldr.c | 154 ++++++++++++++++++++++++++------
 1 file changed, 127 insertions(+), 27 deletions(-)


base-commit: a49e2d257594931772ab8f0024708c3076f3aa1d
-- 
2.52.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation
  2026-10-30  8:52 [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs Chao Gao
@ 2026-10-30  8:52 ` Chao Gao
  2026-11-02 23:58   ` Edgecombe, Rick P
  2026-10-30  8:52 ` [RFC PATCH v1 2/3] x86/virt/seamldr: Support large TDX module binaries Chao Gao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Chao Gao @ 2026-10-30  8:52 UTC (permalink / raw)
  To: linux-kernel, linux-coco, kvm
  Cc: yilun.xu, arjan.van.de.ven, Chao Gao, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Kiryl Shutsemau, Rick Edgecombe

'struct seamldr_params' is the in-memory ABI the kernel passes to P-SEAMLDR
to update the TDX module. It has a fixed size and is allocated before the
update image is validated.

To support larger TDX modules, the ABI was extended to let
'struct seamldr_params' link to extra pages. The number of extra pages
depends on the module page count, which can be trusted only after the image
header is validated.

Move the allocation after the image validation in init_seamldr_params(),
so a later change can allocate 'struct seamldr_params' and the extra
pages as one buffer. Rename the function to build_seamldr_params() since
it now allocates and returns a buffer rather than filling in one the
caller provides.

No functional change intended.

AI was used to review the code and workshop the logs. It suggested renaming
init_seamldr_params() and updating the comments above it and at its call
site.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/seamldr.c | 40 ++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index b1137ca6150d..3513fd28536a 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -157,13 +157,14 @@ static void populate_seamldr_params(struct seamldr_params *params,
 
 /*
  * @image points to a vmalloc()'d 'struct tdx_image'. Transform
- * it into @params which is the P-SEAMLDR ABI format.
+ * it into a newly allocated 'struct seamldr_params', which is
+ * the P-SEAMLDR ABI format.
  */
-static int init_seamldr_params(struct seamldr_params *params,
-			       const struct tdx_image *image,
-			       u32 image_len)
+static struct seamldr_params *build_seamldr_params(const struct tdx_image *image,
+						   u32 image_len)
 {
 	const struct tdx_image_header *header	= &image->header;
+	struct seamldr_params *params;
 
 	u32 sigstruct_len	= header->sigstruct_nr_pages * PAGE_SIZE;
 	u32 module_len		= header->module_nr_pages    * PAGE_SIZE;
@@ -178,25 +179,29 @@ static int init_seamldr_params(struct seamldr_params *params,
 
 	/* Check the calculated payload size against the image size. */
 	if (TDX_IMAGE_HEADER_SIZE + sigstruct_len + module_len != image_len)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	/* Reject unsupported tdx_image ABI versions. */
 	if (header->version != TDX_IMAGE_VERSION_2)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	if (header->sigstruct_nr_pages > SEAMLDR_MAX_NR_SIG_PAGES ||
 	    header->module_nr_pages    > SEAMLDR_MAX_NR_MODULE_PAGES)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	if (memcmp(header->signature, "TDX-BLOB", sizeof(header->signature)))
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	if (memchr_inv(header->reserved, 0, sizeof(header->reserved)))
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
+
+	params = kzalloc_obj(*params);
+	if (!params)
+		return ERR_PTR(-ENOMEM);
 
 	populate_seamldr_params(params, sigstruct_start, header->sigstruct_nr_pages,
 					module_start,    header->module_nr_pages);
-	return 0;
+	return params;
 }
 
 /*
@@ -321,7 +326,7 @@ int seamldr_install_module(const u8 *data, u32 data_len)
 	int ret;
 
 	/*
-	 * init_seamldr_params() reads the header early.
+	 * build_seamldr_params() reads the header early.
 	 * Ensure there is enough data to do at least that.
 	 */
 	if (data_len < TDX_IMAGE_HEADER_SIZE)
@@ -329,14 +334,10 @@ int seamldr_install_module(const u8 *data, u32 data_len)
 
 	image = (const struct tdx_image *)data;
 
-	params = kzalloc_obj(*params);
-	if (!params)
-		return -ENOMEM;
-
-	/* Populate 'params' from 'image'. */
-	ret = init_seamldr_params(params, image, data_len);
-	if (ret)
-		goto out;
+	/* Build 'params' from 'image'. */
+	params = build_seamldr_params(image, data_len);
+	if (IS_ERR(params))
+		return PTR_ERR(params);
 
 	/* Ensure a stable set of online CPUs for the update process. */
 	cpus_read_lock();
@@ -345,7 +346,6 @@ int seamldr_install_module(const u8 *data, u32 data_len)
 				      cpu_online_mask);
 	cpus_read_unlock();
 
-out:
 	kfree(params);
 	return ret;
 }
-- 
2.52.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH v1 2/3] x86/virt/seamldr: Support large TDX module binaries
  2026-10-30  8:52 [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs Chao Gao
  2026-10-30  8:52 ` [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation Chao Gao
@ 2026-10-30  8:52 ` Chao Gao
  2026-11-03  0:35   ` Edgecombe, Rick P
  2026-10-30  8:53 ` [RFC PATCH v1 3/3] x86/virt/seamldr: Support multiple sigstruct pages Chao Gao
  2026-11-02 23:50 ` [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs Edgecombe, Rick P
  3 siblings, 1 reply; 9+ messages in thread
From: Chao Gao @ 2026-10-30  8:52 UTC (permalink / raw)
  To: linux-kernel, linux-coco, kvm
  Cc: yilun.xu, arjan.van.de.ven, Chao Gao, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Kiryl Shutsemau, Rick Edgecombe

The module binary is handed to P-SEAMLDR as a list of page physical
addresses in 'struct seamldr_params'. The list has 496 entries, which
caps a module at 496 pages.

TDX modules are gaining features and are about to outgrow that limit.

The P-SEAMLDR ABI was extended for this. An upcoming revision of the
SEAMLDR specification adds the following to the description of the
module page address list in 'struct seamldr_params'.

  For TDX module image with larger than 495 pages, the last entry will
  point to a new 4 KB page that contains up to 511 additional HPAs. Each
  page will use its last HPA to reference another page if required. This
  chain continues until all module pages are referenced.

Allocate the needed chain pages together with 'struct seamldr_params',
as a flexible array at its end. Link them up at allocation time. Then
fill in the module page addresses as before, but skip the link entries.
A single allocation leaves nothing to unwind on failure and keeps the
existing kfree() as the only cleanup.

Do not query P-SEAMLDR for chaining support. Userspace selects the update
image and is responsible for ensuring the module is compatible with
P-SEAMLDR.

Chaining removes the 496 page cap, but the module page count still needs
a bound. Without one, a crafted header can overflow the u32 module length
('module_nr_pages * PAGE_SIZE') and pass the image size check. It can
also make the chain page allocation exceed KMALLOC_MAX_SIZE, which warns
in the page allocator.

Raise SEAMLDR_MAX_NR_MODULE_PAGES to a 1GB software limit to rule out
both problems. 1GB is far above the few MB of current modules.

The size of module_pages_pa_list[] no longer reflects the maximum module
page count. Define a new SEAMLDR_NR_MODULE_PA_ENTRIES for it.

Thanks to Yilun for the idea of allocating the chain pages together
with 'struct seamldr_params'.

AI was used under supervision to review code and workshop logs. It
spotted both problems that the software limit addresses.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/seamldr.c | 95 +++++++++++++++++++++++++++++++--
 1 file changed, 90 insertions(+), 5 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 3513fd28536a..09c8f6e30e9f 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -24,7 +24,14 @@
 #define P_SEAMLDR_INFO			0x8000000000000000
 #define P_SEAMLDR_INSTALL		0x8000000000000001
 
-#define SEAMLDR_MAX_NR_MODULE_PAGES	496
+/*
+ * Arbitrary software limit, far above the few MB of current modules.
+ * It keeps the module length within a u32, and 'struct seamldr_params'
+ * plus its chain pages within KMALLOC_MAX_SIZE.
+ */
+#define SEAMLDR_MAX_NR_MODULE_PAGES	(SZ_1G / PAGE_SIZE)
+
+#define SEAMLDR_NR_MODULE_PA_ENTRIES	496
 #define SEAMLDR_MAX_NR_SIG_PAGES	1
 
 /*
@@ -34,6 +41,19 @@
  */
 #define SEAMLDR_SCENARIO_UPDATE		1
 
+#define SEAMLDR_NR_CHAIN_PA_ENTRIES	(PAGE_SIZE / sizeof(u64) - 1)
+/*
+ * A 4KB page holding the physical addresses of the module pages that do
+ * not fit in module_pages_pa_list[]. @next_page_pa points to the next
+ * page, forming a singly linked list.
+ */
+struct chain_page {
+	u64	pa[SEAMLDR_NR_CHAIN_PA_ENTRIES];
+	u64	next_page_pa;
+} __packed;
+
+static_assert(sizeof(struct chain_page) == 4096);
+
 /*
  * This is the "SEAMLDR_PARAMS" data structure defined in the
  * "SEAM Loader (SEAMLDR) Interface Specification".
@@ -48,10 +68,13 @@ struct seamldr_params {
 	u64	sigstruct_pages_pa_list[SEAMLDR_MAX_NR_SIG_PAGES];
 	u8	reserved[104];
 	u64	module_nr_pages;
-	u64	module_pages_pa_list[SEAMLDR_MAX_NR_MODULE_PAGES];
+	u64	module_pages_pa_list[SEAMLDR_NR_MODULE_PA_ENTRIES];
+
+	/* Hold module pages that cannot fit in module_pages_pa_list[]. */
+	struct chain_page	chain_pages[];
 } __packed;
 
-static_assert(sizeof(struct seamldr_params) == 4096);
+static_assert(offsetof(struct seamldr_params, chain_pages) == 4096);
 
 /*
  * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
@@ -143,6 +166,68 @@ static void populate_pa_list(u64 *pa_list, const u8 *vmalloc_addr, u32 vmalloc_l
 	}
 }
 
+/*
+ * Like populate_pa_list(), but skip entries pointing to the next page.
+ */
+static void populate_module_pa_list(struct seamldr_params *params,
+				    const u8 *vmalloc_addr, u32 vmalloc_len_pages)
+{
+	u64 *pa_list = params->module_pages_pa_list;
+	int i, j;
+
+	for (i = 0, j = 0; i < vmalloc_len_pages; i++, j++) {
+		unsigned long offset = i * PAGE_SIZE;
+		unsigned long pfn = vmalloc_to_pfn(&vmalloc_addr[offset]);
+
+		/*
+		 * Skip non-zero entries. They point to the next page and
+		 * are populated during allocation.
+		 */
+		if (pa_list[j])
+			j++;
+
+		pa_list[j] = pfn << PAGE_SHIFT;
+	}
+}
+
+static struct seamldr_params *alloc_seamldr_params(u32 nr_module_pages)
+{
+	struct seamldr_params *params;
+	u32 nr_chain_pages;
+	u32 nr_direct;
+	u64 *link;
+
+	/*
+	 * Number of module pages that fit directly in
+	 * module_pages_pa_list[]. Minus one as the last entry is
+	 * reserved to point to the first chain page.
+	 */
+	nr_direct = SEAMLDR_NR_MODULE_PA_ENTRIES - 1;
+
+	/* Calculate the number of extra chain pages needed. */
+	nr_chain_pages = 0;
+	if (nr_module_pages > nr_direct) {
+		u32 nr_left = nr_module_pages - nr_direct;
+
+		nr_chain_pages = DIV_ROUND_UP(nr_left, SEAMLDR_NR_CHAIN_PA_ENTRIES);
+	}
+
+	params = kzalloc_flex(*params, chain_pages, nr_chain_pages);
+	if (!params)
+		return NULL;
+
+	/* Link all the chain pages. */
+	link = &params->module_pages_pa_list[nr_direct];
+	for (int i = 0; i < nr_chain_pages; i++) {
+		struct chain_page *page = &params->chain_pages[i];
+
+		*link = __pa(page);
+		link = &page->next_page_pa;
+	}
+
+	return params;
+}
+
 static void populate_seamldr_params(struct seamldr_params *params,
 				    const u8 *sig, u32 sig_nr_pages,
 				    const u8 *mod, u32 mod_nr_pages)
@@ -152,7 +237,7 @@ static void populate_seamldr_params(struct seamldr_params *params,
 	params->module_nr_pages	= mod_nr_pages;
 
 	populate_pa_list(params->sigstruct_pages_pa_list, sig, sig_nr_pages);
-	populate_pa_list(params->module_pages_pa_list,	  mod, mod_nr_pages);
+	populate_module_pa_list(params, mod, mod_nr_pages);
 }
 
 /*
@@ -195,7 +280,7 @@ static struct seamldr_params *build_seamldr_params(const struct tdx_image *image
 	if (memchr_inv(header->reserved, 0, sizeof(header->reserved)))
 		return ERR_PTR(-EINVAL);
 
-	params = kzalloc_obj(*params);
+	params = alloc_seamldr_params(header->module_nr_pages);
 	if (!params)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.52.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH v1 3/3] x86/virt/seamldr: Support multiple sigstruct pages
  2026-10-30  8:52 [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs Chao Gao
  2026-10-30  8:52 ` [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation Chao Gao
  2026-10-30  8:52 ` [RFC PATCH v1 2/3] x86/virt/seamldr: Support large TDX module binaries Chao Gao
@ 2026-10-30  8:53 ` Chao Gao
  2026-11-02 23:50 ` [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs Edgecombe, Rick P
  3 siblings, 0 replies; 9+ messages in thread
From: Chao Gao @ 2026-10-30  8:53 UTC (permalink / raw)
  To: linux-kernel, linux-coco, kvm
  Cc: yilun.xu, arjan.van.de.ven, Chao Gao, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Kiryl Shutsemau, Rick Edgecombe

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.

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.

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

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/seamldr.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 09c8f6e30e9f..04681e67072f 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -32,7 +32,15 @@
 #define SEAMLDR_MAX_NR_MODULE_PAGES	(SZ_1G / PAGE_SIZE)
 
 #define SEAMLDR_NR_MODULE_PA_ENTRIES	496
-#define SEAMLDR_MAX_NR_SIG_PAGES	1
+#define SEAMLDR_MAX_NR_SIG_PAGES	4
+
+/*
+ * The seamldr_params "version" field specifies the layout of the
+ * structure. Version 0 supports only 1 sigstruct page while version 1
+ * supports up to 4.
+ */
+#define SEAMLDR_PARAMS_VERSION_0	0
+#define SEAMLDR_PARAMS_VERSION_1	1
 
 /*
  * The seamldr_params "scenario" field specifies the operation mode:
@@ -66,7 +74,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];
 
@@ -232,7 +240,14 @@ static void populate_seamldr_params(struct seamldr_params *params,
 				    const u8 *sig, u32 sig_nr_pages,
 				    const u8 *mod, u32 mod_nr_pages)
 {
-	params->version		= 0;
+	params->version		= SEAMLDR_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	= SEAMLDR_PARAMS_VERSION_1;
+
 	params->scenario	= SEAMLDR_SCENARIO_UPDATE;
 	params->module_nr_pages	= mod_nr_pages;
 
-- 
2.52.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs
  2026-10-30  8:52 [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs Chao Gao
                   ` (2 preceding siblings ...)
  2026-10-30  8:53 ` [RFC PATCH v1 3/3] x86/virt/seamldr: Support multiple sigstruct pages Chao Gao
@ 2026-11-02 23:50 ` Edgecombe, Rick P
  3 siblings, 0 replies; 9+ messages in thread
From: Edgecombe, Rick P @ 2026-11-02 23:50 UTC (permalink / raw)
  To: kvm, linux-coco, linux-kernel, Gao, Chao, x86
  Cc: Van De Ven, Arjan, Xu, Yilun, bp, kas, hpa, mingo, dave.hansen, tglx

On Fri, 2026-10-30 at 01:52 -0700, Chao Gao wrote:
> This series lets TDX module updates handle large TDX modules and
> sigstructs. It is an RFC for early review.
> 
> Dave, feel free to ignore this RFC for now. TDX developers, please take
> a look.
> 
> I would especially like feedback on one question. Does the kernel need
> to tell userspace that it supports large modules and sigstructs? I would
> rather not add such an interface.
> 
> Userspace can simply try such an image, because a kernel without this
> series rejects it with -EINVAL before calling P-SEAMLDR, so a failed try
> has no side effects. If userspace should not have to try an update to
> find out, the alternative is to backport this series to all kernels that
> support TDX module updates. Userspace could then assume that any kernel
> with TDX module updates handles such images.

Sounds good to me.

> 
> ====
> 
> This series is a follow-up to the TDX module update series. As thanks
> for merging that series, we agreed to take on the TDX metadata cleanup [1].
> With the cleanup converging, we are starting on this work and expect it
> to be merged after the cleanup.
> 
> Today a TDX module binary can be at most 496 pages and its sigstruct at
> most one page. Both limits come from the P-SEAMLDR ABI.
> 
> TDX modules are outgrowing them. A post-quantum signature algorithm
> pushes the sigstruct past 4KB, and new module features push the binary
> past 496 pages. The P-SEAMLDR ABI was extended for both.
> 
> The kernel rejects any image over these limits. TDX modules for Diamond
> Rapids exceed them, so without this series, TDX module updates fail on
> Diamond Rapids. This series is all that is needed to make them work.
> 
> The image format that userspace uploads ('struct tdx_image') is generic
> enough and needs no change.
> 
> == AI Usage ==
> AI was used under supervision to review code and workshop logs. It
> spotted a u32 overflow and an oversized allocation that removing the 496
> page cap would cause in patch 2. The module size limit in that patch
> prevents both. It also suggested renaming init_seamldr_params() in
> patch 1 and updating the comments around it.
> 
> == Testing ==
> Tested on a Diamond Rapids system. Updates with a module larger than 2MB
> and an 8KB sigstruct work, which exercises both the chained page list
> and version 1 of 'struct seamldr_params'. TDs keep running across
> updates.
> 
> [1]: https://lore.kernel.org/kvm/20261012094754.27702-1-chao.gao@intel.com/
> 
> Chao Gao (3):
>   x86/virt/seamldr: Allocate 'struct seamldr_params' after image
>     validation
>   x86/virt/seamldr: Support large TDX module binaries
>   x86/virt/seamldr: Support multiple sigstruct pages
> 
>  arch/x86/virt/vmx/tdx/seamldr.c | 154 ++++++++++++++++++++++++++------
>  1 file changed, 127 insertions(+), 27 deletions(-)

This is a lot bigger than I expected. The old SEAMLDR_PARAMS_VERSION_1 chunk
that got deferred was tiny.

So I guess DMR support takes a bit more.

> 
> 
> base-commit: a49e2d257594931772ab8f0024708c3076f3aa1d


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation
  2026-10-30  8:52 ` [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation Chao Gao
@ 2026-11-02 23:58   ` Edgecombe, Rick P
  2026-11-03  6:30     ` Chao Gao
  0 siblings, 1 reply; 9+ messages in thread
From: Edgecombe, Rick P @ 2026-11-02 23:58 UTC (permalink / raw)
  To: kvm, linux-coco, linux-kernel, Gao, Chao
  Cc: Van De Ven, Arjan, Xu, Yilun, dave.hansen, x86, kas, hpa, mingo,
	bp, tglx

On Fri, 2026-10-30 at 01:52 -0700, Chao Gao wrote:
> 'struct seamldr_params' is the in-memory ABI the kernel passes to P-SEAMLDR
> to update the TDX module. It has a fixed size and is allocated before the
> update image is validated.
> 
> To support larger TDX modules, the ABI was extended to let
> 'struct seamldr_params' link to extra pages. The number of extra pages
> depends on the module page count, which can be trusted only after the image
> header is validated.
> 
> Move the allocation after the image validation in init_seamldr_params(),
> so a later change can allocate 'struct seamldr_params' and the extra
> pages as one buffer. Rename the function to build_seamldr_params() since
> it now allocates and returns a buffer rather than filling in one the
> caller provides.
> 
> No functional change intended.

So we want to have the allocation happen after header->module_nr_pages is
verified so we don't allocate some ridiculous size. And the verification
currently happens all in one place. So move the allocation.

Did you look at just moving the one header field check earlier? Wondering if it
requires less churn.

Hmm, also, this is an admin only thing right? What goes wrong if the admin user
passes a huge value?

In any case, I think the log could be simpler and more plain while still
explaining what is happening better.

> 
> AI was used to review the code and workshop the logs. It suggested renaming
> init_seamldr_params() and updating the comments above it and at its call
> site.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
>  arch/x86/virt/vmx/tdx/seamldr.c | 40 ++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index b1137ca6150d..3513fd28536a 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -157,13 +157,14 @@ static void populate_seamldr_params(struct
> seamldr_params *params,
>  
>  /*
>   * @image points to a vmalloc()'d 'struct tdx_image'. Transform
> - * it into @params which is the P-SEAMLDR ABI format.
> + * it into a newly allocated 'struct seamldr_params', which is
> + * the P-SEAMLDR ABI format.
>   */
> -static int init_seamldr_params(struct seamldr_params *params,
> -			       const struct tdx_image *image,
> -			       u32 image_len)
> +static struct seamldr_params *build_seamldr_params(const struct tdx_image
> *image,
> +						   u32 image_len)
>  {
>  	const struct tdx_image_header *header	= &image->header;
> +	struct seamldr_params *params;
>  
>  	u32 sigstruct_len	= header->sigstruct_nr_pages * PAGE_SIZE;
>  	u32 module_len		= header->module_nr_pages    * PAGE_SIZE;
> @@ -178,25 +179,29 @@ static int init_seamldr_params(struct seamldr_params
> *params,
>  
>  	/* Check the calculated payload size against the image size. */
>  	if (TDX_IMAGE_HEADER_SIZE + sigstruct_len + module_len != image_len)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	/* Reject unsupported tdx_image ABI versions. */
>  	if (header->version != TDX_IMAGE_VERSION_2)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	if (header->sigstruct_nr_pages > SEAMLDR_MAX_NR_SIG_PAGES ||
>  	    header->module_nr_pages    > SEAMLDR_MAX_NR_MODULE_PAGES)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	if (memcmp(header->signature, "TDX-BLOB", sizeof(header->signature)))
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	if (memchr_inv(header->reserved, 0, sizeof(header->reserved)))
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
> +
> +	params = kzalloc_obj(*params);
> +	if (!params)
> +		return ERR_PTR(-ENOMEM);
>  
>  	populate_seamldr_params(params, sigstruct_start, header-
> >sigstruct_nr_pages,
>  					module_start,    header-
> >module_nr_pages);
> -	return 0;
> +	return params;
>  }
>  
>  /*
> @@ -321,7 +326,7 @@ int seamldr_install_module(const u8 *data, u32 data_len)
>  	int ret;
>  
>  	/*
> -	 * init_seamldr_params() reads the header early.
> +	 * build_seamldr_params() reads the header early.
>  	 * Ensure there is enough data to do at least that.
>  	 */
>  	if (data_len < TDX_IMAGE_HEADER_SIZE)
> @@ -329,14 +334,10 @@ int seamldr_install_module(const u8 *data, u32 data_len)
>  
>  	image = (const struct tdx_image *)data;
>  
> -	params = kzalloc_obj(*params);
> -	if (!params)
> -		return -ENOMEM;
> -
> -	/* Populate 'params' from 'image'. */
> -	ret = init_seamldr_params(params, image, data_len);
> -	if (ret)
> -		goto out;
> +	/* Build 'params' from 'image'. */
> +	params = build_seamldr_params(image, data_len);
> +	if (IS_ERR(params))
> +		return PTR_ERR(params);
>  
>  	/* Ensure a stable set of online CPUs for the update process. */
>  	cpus_read_lock();
> @@ -345,7 +346,6 @@ int seamldr_install_module(const u8 *data, u32 data_len)
>  				      cpu_online_mask);
>  	cpus_read_unlock();
>  
> -out:
>  	kfree(params);
>  	return ret;
>  }


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v1 2/3] x86/virt/seamldr: Support large TDX module binaries
  2026-10-30  8:52 ` [RFC PATCH v1 2/3] x86/virt/seamldr: Support large TDX module binaries Chao Gao
@ 2026-11-03  0:35   ` Edgecombe, Rick P
  2026-11-03  8:57     ` Chao Gao
  0 siblings, 1 reply; 9+ messages in thread
From: Edgecombe, Rick P @ 2026-11-03  0:35 UTC (permalink / raw)
  To: kvm, linux-coco, linux-kernel, Gao, Chao
  Cc: Van De Ven, Arjan, Xu, Yilun, dave.hansen, x86, kas, hpa, mingo,
	bp, tglx

On Fri, 2026-10-30 at 01:52 -0700, Chao Gao wrote:
> The module binary is handed to P-SEAMLDR as a list of page physical
> addresses in 'struct seamldr_params'. The list has 496 entries, which
> caps a module at 496 pages.
> 
> TDX modules are gaining features and are about to outgrow that limit.
> 
> The P-SEAMLDR ABI was extended for this. An upcoming revision of the
> SEAMLDR specification adds the following to the description of the
> module page address list in 'struct seamldr_params'.
> 
>   For TDX module image with larger than 495 pages, the last entry will
>   point to a new 4 KB page that contains up to 511 additional HPAs. Each
>   page will use its last HPA to reference another page if required. This
>   chain continues until all module pages are referenced.
> 
> Allocate the needed chain pages together with 'struct seamldr_params',
> as a flexible array at its end. Link them up at allocation time. Then
> fill in the module page addresses as before, but skip the link entries.
> A single allocation leaves nothing to unwind on failure and keeps the
> existing kfree() as the only cleanup.
> 
> Do not query P-SEAMLDR for chaining support. Userspace selects the update
> image and is responsible for ensuring the module is compatible with
> P-SEAMLDR.
> 
> Chaining removes the 496 page cap, but the module page count still needs
> a bound. Without one, a crafted header can overflow the u32 module length
> ('module_nr_pages * PAGE_SIZE') and pass the image size check. It can
> also make the chain page allocation exceed KMALLOC_MAX_SIZE, which warns
> in the page allocator.
> 
> Raise SEAMLDR_MAX_NR_MODULE_PAGES to a 1GB software limit to rule out
> both problems. 1GB is far above the few MB of current modules.
> 
> The size of module_pages_pa_list[] no longer reflects the maximum module
> page count. Define a new SEAMLDR_NR_MODULE_PA_ENTRIES for it.
> 
> Thanks to Yilun for the idea of allocating the chain pages together
> with 'struct seamldr_params'.
> 
> AI was used under supervision to review code and workshop logs. It
> spotted both problems that the software limit addresses.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
>  arch/x86/virt/vmx/tdx/seamldr.c | 95 +++++++++++++++++++++++++++++++--
>  1 file changed, 90 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 3513fd28536a..09c8f6e30e9f 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -24,7 +24,14 @@
>  #define P_SEAMLDR_INFO			0x8000000000000000
>  #define P_SEAMLDR_INSTALL		0x8000000000000001
>  
> -#define SEAMLDR_MAX_NR_MODULE_PAGES	496
> +/*
> + * Arbitrary software limit, far above the few MB of current modules.
> + * It keeps the module length within a u32, and 'struct seamldr_params'
> + * plus its chain pages within KMALLOC_MAX_SIZE.
> + */
> +#define SEAMLDR_MAX_NR_MODULE_PAGES	(SZ_1G / PAGE_SIZE)
> +
> +#define SEAMLDR_NR_MODULE_PA_ENTRIES	496
>  #define SEAMLDR_MAX_NR_SIG_PAGES	1
>  
>  /*
> @@ -34,6 +41,19 @@
>   */
>  #define SEAMLDR_SCENARIO_UPDATE		1
>  
> +#define SEAMLDR_NR_CHAIN_PA_ENTRIES	(PAGE_SIZE / sizeof(u64) - 1)
> +/*
> + * A 4KB page holding the physical addresses of the module pages that do
> + * not fit in module_pages_pa_list[]. @next_page_pa points to the next
> + * page, forming a singly linked list.
> + */
> +struct chain_page {
> +	u64	pa[SEAMLDR_NR_CHAIN_PA_ENTRIES];
> +	u64	next_page_pa;
> +} __packed;
> +
> +static_assert(sizeof(struct chain_page) == 4096);
> +
>  /*
>   * This is the "SEAMLDR_PARAMS" data structure defined in the
>   * "SEAM Loader (SEAMLDR) Interface Specification".
> @@ -48,10 +68,13 @@ struct seamldr_params {
>  	u64	sigstruct_pages_pa_list[SEAMLDR_MAX_NR_SIG_PAGES];
>  	u8	reserved[104];
>  	u64	module_nr_pages;
> -	u64	module_pages_pa_list[SEAMLDR_MAX_NR_MODULE_PAGES];
> +	u64	module_pages_pa_list[SEAMLDR_NR_MODULE_PA_ENTRIES];
> +
> +	/* Hold module pages that cannot fit in module_pages_pa_list[]. */
> +	struct chain_page	chain_pages[];
>  } __packed;
>  
> -static_assert(sizeof(struct seamldr_params) == 4096);
> +static_assert(offsetof(struct seamldr_params, chain_pages) == 4096);
>  
>  /*
>   * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> @@ -143,6 +166,68 @@ static void populate_pa_list(u64 *pa_list, const u8
> *vmalloc_addr, u32 vmalloc_l
>  	}
>  }
>  
> +/*
> + * Like populate_pa_list(), but skip entries pointing to the next page.
> + */
> +static void populate_module_pa_list(struct seamldr_params *params,
> +				    const u8 *vmalloc_addr, u32
> vmalloc_len_pages)
> +{
> +	u64 *pa_list = params->module_pages_pa_list;
> +	int i, j;
> +
> +	for (i = 0, j = 0; i < vmalloc_len_pages; i++, j++) {
> +		unsigned long offset = i * PAGE_SIZE;
> +		unsigned long pfn = vmalloc_to_pfn(&vmalloc_addr[offset]);
> +
> +		/*
> +		 * Skip non-zero entries. They point to the next page and
> +		 * are populated during allocation.
> +		 */
> +		if (pa_list[j])
> +			j++;

We know what position in the page this should be right? Can we do some modulus
and skip it that way? Feels less hacky.

> +
> +		pa_list[j] = pfn << PAGE_SHIFT;
> +	}
> +}
> +
> +static struct seamldr_params *alloc_seamldr_params(u32 nr_module_pages)
> +{
> +	struct seamldr_params *params;
> +	u32 nr_chain_pages;
> +	u32 nr_direct;
> +	u64 *link;
> +
> +	/*
> +	 * Number of module pages that fit directly in

"fit directly" feels like adding jargon where there doesn't need to be.

> +	 * module_pages_pa_list[]. Minus one as the last entry is
> +	 * reserved to point to the first chain page.
> +	 */
> +	nr_direct = SEAMLDR_NR_MODULE_PA_ENTRIES - 1;
> +
> +	/* Calculate the number of extra chain pages needed. */
> +	nr_chain_pages = 0;
> +	if (nr_module_pages > nr_direct) {
> +		u32 nr_left = nr_module_pages - nr_direct;
> +
> +		nr_chain_pages = DIV_ROUND_UP(nr_left,
> SEAMLDR_NR_CHAIN_PA_ENTRIES);

Left pages, module pages, direct pages and chain pages. It's a lot...

> +	}
> +
> +	params = kzalloc_flex(*params, chain_pages, nr_chain_pages);
> +	if (!params)
> +		return NULL;
> +
> +	/* Link all the chain pages. */
> +	link = &params->module_pages_pa_list[nr_direct];
> +	for (int i = 0; i < nr_chain_pages; i++) {
> +		struct chain_page *page = &params->chain_pages[i];
> +
> +		*link = __pa(page);
> +		link = &page->next_page_pa;
> +	}
> +
> +	return params;
> +}

This all seems like more than should be needed.

> +
>  static void populate_seamldr_params(struct seamldr_params *params,
>  				    const u8 *sig, u32 sig_nr_pages,
>  				    const u8 *mod, u32 mod_nr_pages)
> @@ -152,7 +237,7 @@ static void populate_seamldr_params(struct seamldr_params
> *params,
>  	params->module_nr_pages	= mod_nr_pages;
>  
>  	populate_pa_list(params->sigstruct_pages_pa_list, sig, sig_nr_pages);
> -	populate_pa_list(params->module_pages_pa_list,	  mod, mod_nr_pages);
> +	populate_module_pa_list(params, mod, mod_nr_pages);
>  }
>  
>  /*
> @@ -195,7 +280,7 @@ static struct seamldr_params *build_seamldr_params(const
> struct tdx_image *image
>  	if (memchr_inv(header->reserved, 0, sizeof(header->reserved)))
>  		return ERR_PTR(-EINVAL);
>  
> -	params = kzalloc_obj(*params);
> +	params = alloc_seamldr_params(header->module_nr_pages);
>  	if (!params)
>  		return ERR_PTR(-ENOMEM);
>  


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation
  2026-11-02 23:58   ` Edgecombe, Rick P
@ 2026-11-03  6:30     ` Chao Gao
  0 siblings, 0 replies; 9+ messages in thread
From: Chao Gao @ 2026-11-03  6:30 UTC (permalink / raw)
  To: Edgecombe, Rick P
  Cc: kvm, linux-coco, linux-kernel, Van De Ven, Arjan, Xu, Yilun,
	dave.hansen, x86, kas, hpa, mingo, bp, tglx

On Tue, Nov 03, 2026 at 07:58:06AM +0800, Edgecombe, Rick P wrote:
>On Fri, 2026-10-30 at 01:52 -0700, Chao Gao wrote:
>> 'struct seamldr_params' is the in-memory ABI the kernel passes to P-SEAMLDR
>> to update the TDX module. It has a fixed size and is allocated before the
>> update image is validated.
>> 
>> To support larger TDX modules, the ABI was extended to let
>> 'struct seamldr_params' link to extra pages. The number of extra pages
>> depends on the module page count, which can be trusted only after the image
>> header is validated.
>> 
>> Move the allocation after the image validation in init_seamldr_params(),
>> so a later change can allocate 'struct seamldr_params' and the extra
>> pages as one buffer. Rename the function to build_seamldr_params() since
>> it now allocates and returns a buffer rather than filling in one the
>> caller provides.
>> 
>> No functional change intended.
>
>So we want to have the allocation happen after header->module_nr_pages is
>verified so we don't allocate some ridiculous size. And the verification
>currently happens all in one place. So move the allocation.
>
>Did you look at just moving the one header field check earlier? Wondering if it
>requires less churn.

I didn't. It works too, and we can drop this patch by doing the check in
alloc_seamldr_params() (added in patch 2) instead:

static struct seamldr_params *alloc_seamldr_params(u32 nr_module_pages)
{
	struct seamldr_params *params;
	u32 nr_chain_pages;
	u32 nr_direct;
	u64 *link;

+	if (nr_module_pages > SEAMLDR_MAX_NR_MODULE_PAGES)
+		return ERR_PTR(-EINVAL);
+
	/*
	 * Number of module pages that fit directly in
	 * module_pages_pa_list[]. Minus one as the last entry is
	 * reserved to point to the first chain page.
	 */
	nr_direct = SEAMLDR_NR_MODULE_PA_ENTRIES - 1;

My concern is that this spreads the header checks across two functions.
The churn is a one-time cost, while keeping all the checks in one place
helps readability for the long term. That said, I am fine with going
with less churn if you prefer.

>
>Hmm, also, this is an admin only thing right? What goes wrong if the admin user
>passes a huge value?

Right, it is admin only, and nothing too bad happens. A u32
module_nr_pages can ask for about 32GB of chain pages. Anything above
KMALLOC_MAX_SIZE (4MB) hits the WARN_ON_ONCE() in the page allocator,
taints the kernel and fails the update with -ENOMEM.

So it is not a serious problem. But relying on "it does not hurt" has a
cost of its own. Every reader has to work out why the unvalidated value
is safe to use. Using the value only after it is validated is more
straightforward.

>
>In any case, I think the log could be simpler and more plain while still
>explaining what is happening better.

If we keep this patch, how about:

  'struct seamldr_params' is the in-memory ABI passed to P-SEAMLDR to
  update the TDX module. It has a fixed size and is allocated before
  the update image header is validated.

  A later change will append a variable number of pages to the
  structure, sized by header->module_nr_pages. The allocation then
  needs to come after that field is checked, so that a bogus header
  cannot make the kernel attempt a huge allocation.

  All header checks are in one place. Move the allocation after them
  instead of splitting the module_nr_pages check out.

  Rename init_seamldr_params() to build_seamldr_params() since it now
  allocates and returns the structure rather than filling in one the
  caller provides.

  No functional change intended.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v1 2/3] x86/virt/seamldr: Support large TDX module binaries
  2026-11-03  0:35   ` Edgecombe, Rick P
@ 2026-11-03  8:57     ` Chao Gao
  0 siblings, 0 replies; 9+ messages in thread
From: Chao Gao @ 2026-11-03  8:57 UTC (permalink / raw)
  To: Edgecombe, Rick P
  Cc: kvm, linux-coco, linux-kernel, Van De Ven, Arjan, Xu, Yilun,
	dave.hansen, x86, kas, hpa, mingo, bp, tglx

>> +/*
>> + * Like populate_pa_list(), but skip entries pointing to the next page.
>> + */
>> +static void populate_module_pa_list(struct seamldr_params *params,
>> +				    const u8 *vmalloc_addr, u32
>> vmalloc_len_pages)
>> +{
>> +	u64 *pa_list = params->module_pages_pa_list;
>> +	int i, j;
>> +
>> +	for (i = 0, j = 0; i < vmalloc_len_pages; i++, j++) {
>> +		unsigned long offset = i * PAGE_SIZE;
>> +		unsigned long pfn = vmalloc_to_pfn(&vmalloc_addr[offset]);
>> +
>> +		/*
>> +		 * Skip non-zero entries. They point to the next page and
>> +		 * are populated during allocation.
>> +		 */
>> +		if (pa_list[j])
>> +			j++;
>
>We know what position in the page this should be right? Can we do some modulus
>and skip it that way? Feels less hacky.

Yes, the link is always the last entry of a page.

how about:
		/*
		 * The last entry of each page points to the next chain
		 * page.
		 */
		if (PAGE_ALIGNED(&pa_list[j + 1]))
			j++;

>
>> +
>> +		pa_list[j] = pfn << PAGE_SHIFT;
>> +	}
>> +}
>> +
>> +static struct seamldr_params *alloc_seamldr_params(u32 nr_module_pages)
>> +{
>> +	struct seamldr_params *params;
>> +	u32 nr_chain_pages;
>> +	u32 nr_direct;
>> +	u64 *link;
>> +
>> +	/*
>> +	 * Number of module pages that fit directly in
>
>"fit directly" feels like adding jargon where there doesn't need to be.

[...]

>
>> +	 * module_pages_pa_list[]. Minus one as the last entry is
>> +	 * reserved to point to the first chain page.
>> +	 */
>> +	nr_direct = SEAMLDR_NR_MODULE_PA_ENTRIES - 1;
>> +
>> +	/* Calculate the number of extra chain pages needed. */
>> +	nr_chain_pages = 0;
>> +	if (nr_module_pages > nr_direct) {
>> +		u32 nr_left = nr_module_pages - nr_direct;
>> +
>> +		nr_chain_pages = DIV_ROUND_UP(nr_left,
>> SEAMLDR_NR_CHAIN_PA_ENTRIES);
>
>Left pages, module pages, direct pages and chain pages. It's a lot...

[...]

>
>> +	}
>> +
>> +	params = kzalloc_flex(*params, chain_pages, nr_chain_pages);
>> +	if (!params)
>> +		return NULL;
>> +
>> +	/* Link all the chain pages. */
>> +	link = &params->module_pages_pa_list[nr_direct];
>> +	for (int i = 0; i < nr_chain_pages; i++) {
>> +		struct chain_page *page = &params->chain_pages[i];
>> +
>> +		*link = __pa(page);
>> +		link = &page->next_page_pa;
>> +	}
>> +
>> +	return params;
>> +}
>
>This all seems like more than should be needed.

Most of the complexity came from sizing the allocation exactly. If we
over-allocate a little instead, the size calculation becomes much
simpler. Making module_pages_pa_list[] a flexible array also removes
the need to treat its entries differently from those in the chain
pages. And linking the pages while filling in the module page addresses
removes the separate linking loop.

We could even fold populate_module_pa_list() into populate_pa_list().
That works because sigstruct_pages_pa_list[] never reaches the end of a
page. It would save a function, but it looks a bit hacky for the
sigstruct pages, because the spec defines links only for the module page
list. I am not sure which is better. Do you have a preference?


diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index b1137ca6150d..cd4a00a7b02e 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
 /*
@@ -48,11 +54,9 @@ struct seamldr_params {
 	u64	sigstruct_pages_pa_list[SEAMLDR_MAX_NR_SIG_PAGES];
 	u8	reserved[104];
 	u64	module_nr_pages;
-	u64	module_pages_pa_list[SEAMLDR_MAX_NR_MODULE_PAGES];
+	u64	module_pages_pa_list[];
 } __packed;
 
-static_assert(sizeof(struct seamldr_params) == 4096);
-
 /*
  * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
  * interact with P-SEAMLDR simultaneously. Use raw version as the calls can
@@ -143,6 +147,55 @@ static void populate_pa_list(u64 *pa_list, const u8 *vmalloc_addr, u32 vmalloc_l
 	}
 }
 
+/*
+ * Like populate_pa_list(), but skip entries pointing to the next page.
+ */
+static void populate_module_pa_list(struct seamldr_params *params,
+				    const u8 *vmalloc_addr, u32 vmalloc_len_pages)
+{
+	u64 *pa_list = params->module_pages_pa_list;
+	int i, j;
+
+	for (i = 0, j = 0; i < vmalloc_len_pages; i++, j++) {
+		unsigned long offset = i * PAGE_SIZE;
+		unsigned long pfn = vmalloc_to_pfn(&vmalloc_addr[offset]);
+
+		/*
+		 * The last entry of each page points to the next chain
+		 * page.
+		 */
+		if (PAGE_ALIGNED(&pa_list[j + 1])) {
+			pa_list[j] = __pa(&pa_list[j + 1]);
+			j++;
+		}
+
+		pa_list[j] = pfn << PAGE_SHIFT;
+	}
+}
+
+static struct seamldr_params *alloc_seamldr_params(u32 nr_module_pages)
+{
+	struct seamldr_params *params;
+	u32 nr_pages;
+
+	if (nr_module_pages > SEAMLDR_MAX_NR_MODULE_PAGES)
+		return ERR_PTR(-EINVAL);
+
+	/*
+	 * One page for 'struct seamldr_params', plus chain pages that
+	 * each hold 511 module page addresses and a link to the next
+	 * page. Ignoring the addresses held in the first page
+	 * over-allocates a little.
+	 */
+	nr_pages = 1 + DIV_ROUND_UP(nr_module_pages, 511);
+
+	params = kzalloc(nr_pages * PAGE_SIZE, GFP_KERNEL);
+	if (!params)
+		return ERR_PTR(-ENOMEM);
+
+	return params;
+}
+

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-11-03  8:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-30  8:52 [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs Chao Gao
2026-10-30  8:52 ` [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation Chao Gao
2026-11-02 23:58   ` Edgecombe, Rick P
2026-11-03  6:30     ` Chao Gao
2026-10-30  8:52 ` [RFC PATCH v1 2/3] x86/virt/seamldr: Support large TDX module binaries Chao Gao
2026-11-03  0:35   ` Edgecombe, Rick P
2026-11-03  8:57     ` Chao Gao
2026-10-30  8:53 ` [RFC PATCH v1 3/3] x86/virt/seamldr: Support multiple sigstruct pages Chao Gao
2026-11-02 23:50 ` [RFC PATCH v1 0/3] TDX: P-SEAMLDR: Support large TDX modules and sigstructs Edgecombe, Rick P

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®