Re: [PATCH v5 08/22] x86/virt/seamldr: Allocate and populate a module update request

[Kiryl Shutsemau <kas@kernel.org>]

On Sun, Mar 15, 2026 at 06:58:28AM -0700, 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 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.
> 
> 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. P-SEAMLDR compatibility
> validation (such as 4KB vs 16KB sigstruct support) is left to userspace,
> which must verify the P-SEAMLDR version meets the TDX module's minimum
> requirements.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> v5:
>  - use a macro for tdx_blob version (0x100) [Yan]
>  - don't do alignment checking for the binary/sigstruct [Rick]
>  - drop blob's sigstruct and validation checking
>  - set seamldr_params.version to 1 when necessary
>  - drop the link to blob_structure.txt which might be unstable [Kai]
> 
> v4:
>  - Remove checksum verification as it is optional
>  - Convert comments to is_vmalloc_addr() checks [Kai]
>  - Explain size/alignment checks in alloc_seamldr_params() [Kai]
> 
> 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 | 141 ++++++++++++++++++++++++++++++++
>  1 file changed, 141 insertions(+)
> 
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 7114326d7569..20cb6c797ce5 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -7,6 +7,7 @@
>  #define pr_fmt(fmt)	"seamldr: " fmt
>  
>  #include <linux/mm.h>
> +#include <linux/slab.h>
>  #include <linux/spinlock.h>
>  
>  #include <asm/seamldr.h>
> @@ -16,6 +17,33 @@
>  /* P-SEAMLDR SEAMCALL leaf function */
>  #define P_SEAMLDR_INFO			0x8000000000000000
>  
> +#define SEAMLDR_MAX_NR_MODULE_4KB_PAGES	496
> +#define SEAMLDR_MAX_NR_SIG_4KB_PAGES	4
> +
> +/*
> + * The seamldr_params "scenario" field specifies the operation mode:
> + * 0: Install TDX module from scratch (not used by kernel)
> + * 1: Update existing TDX module to a compatible version
> + */
> +#define SEAMLDR_SCENARIO_UPDATE		1
> +
> +/*
> + * This is called the "SEAMLDR_PARAMS" data structure and is defined
> + * in "SEAM Loader (SEAMLDR) Interface Specification".
> + *
> + * It describes the TDX module that will be installed.
> + */
> +struct seamldr_params {
> +	u32	version;
> +	u32	scenario;
> +	u64	sigstruct_pa[SEAMLDR_MAX_NR_SIG_4KB_PAGES];
> +	u8	reserved[80];
> +	u64	num_module_pages;
> +	u64	mod_pages_pa_list[SEAMLDR_MAX_NR_MODULE_4KB_PAGES];
> +} __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
> @@ -41,6 +69,114 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>  
> +static void free_seamldr_params(struct seamldr_params *params)
> +{
> +	free_page((unsigned long)params);
> +}
> +
> +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;
> +
> +	if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (sig_size > SEAMLDR_MAX_NR_SIG_4KB_PAGES * SZ_4K)
> +		return ERR_PTR(-EINVAL);
> +
> +	params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
> +	if (!params)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/*
> +	 * Only use version 1 when required (sigstruct > 4KB) for backward
> +	 * compatibility with P-SEAMLDR that lacks version 1 support.
> +	 */
> +	if (sig_size > SZ_4K)
> +		params->version = 1;
> +	else
> +		params->version = 0;
> +
> +	params->scenario = SEAMLDR_SCENARIO_UPDATE;
> +
> +	ptr = sig;
> +	for (i = 0; i < sig_size / SZ_4K; i++) {
> +		/*
> +		 * Don't assume @sig is page-aligned although it is 4KB-aligned.
> +		 * Always add the in-page offset to get the physical address.
> +		 */

I don't follow this. If @sig is 4k aligned in VA, it is page aligned.

If you want to handle case when @sig is not 4k aligned, than this is
broken. You need to bump ptr to the next 4k boundary, not by 4k.

> +		params->sigstruct_pa[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
> +					  ((unsigned long)ptr & ~PAGE_MASK);
> +		ptr += SZ_4K;
> +	}
> +
> +	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;

Same here.

> +	}
> +
> +	return params;
> +}
> +
> +/*
> + * Intel TDX module blob. Its format is defined at:
> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
> + *
> + * 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	reserved0;
> +	u64	reserved1[509];
> +	u8	data[];
> +} __packed;
> +
> +/* Supported versions of the tdx_blob */
> +#define TDX_BLOB_VERSION_1	0x100
> +
> +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;
> +
> +	/* Ensure the size is valid otherwise reading any field from the blob may overflow. */
> +	if (size <= sizeof(struct tdx_blob) || size <= blob->offset_of_module)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (blob->version != TDX_BLOB_VERSION_1) {
> +		pr_err("unsupported blob version: %x\n", blob->version);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	/* Split the 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->length != size)
> +		return ERR_PTR(-EINVAL);

Maybe add a comment somewhere that block->offset_of_module is relative
to start of struct tdx_blob, not blob->data and blob->length includes
length of struct tdx_blob.

It can be either way and it is better to give a reader a hint.

> +
> +	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 update blob.
> @@ -50,6 +186,11 @@ EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>   */
>  int seamldr_install_module(const u8 *data, u32 size)
>  {
> +	struct seamldr_params *params __free(free_seamldr_params) =
> +						init_seamldr_params(data, size);
> +	if (IS_ERR(params))
> +		return PTR_ERR(params);
> +
>  	/* TODO: Update TDX module here */
>  	return 0;
>  }
> -- 
> 2.47.3
> 

-- 
  Kiryl Shutsemau / Kirill A. Shutemov