Re: [PATCH v10 13/25] x86/virt/seamldr: Allocate and populate a module update request

[Kiryl Shutsemau <kas@kernel.org>]

On Wed, May 20, 2026 at 06:38:16AM -0700, Chao Gao wrote:
> +static void populate_pa_list(u64 *pa_list, const u8 *vmalloc_addr, u32 vmalloc_len_pages)
> +{
> +	int i;
> +
> +	for (i = 0; i < vmalloc_len_pages; i++) {
> +		unsigned long offset = i * PAGE_SIZE;
> +		unsigned long pfn = vmalloc_to_pfn(&vmalloc_addr[offset]);

I don't like that we need to assume how the image got allocated this
deep in the stack.

I can imagine situation in the future when we might want to load TDX
module to memory on boot, like initrd. And it won't be vmalloced in this
case.

Wouldn't be better to use a neutral way to get physical address that
doesn't have the assumption? Like, slow_virt_to_phys().

> +
> +		pa_list[i] = pfn << PAGE_SHIFT;
> +	}
> +}
> +
> +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->scenario		= SEAMLDR_SCENARIO_UPDATE;
> +	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);

I am not sure what the value to have this as a separate function.

Having it directly in init_seamldr_params() would be easier to follow.

> +}
> +
> +/*
> + * @image points to a vmalloc()'d 'struct tdx_image'. Transform
> + * it into @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)
> +{
> +	const struct tdx_image_header *header	= &image->header;
> +
> +	u32 sigstruct_len	= header->sigstruct_nr_pages * PAGE_SIZE;
> +	u32 module_len		= header->module_nr_pages * PAGE_SIZE;
> +
> +	u8 *header_start	= (u8 *)header;
> +	u8 *header_end		= header_start + TDX_IMAGE_HEADER_SIZE;
> +
> +	u8 *sigstruct_start	= header_end;
> +	u8 *sigstruct_end	= sigstruct_start + sigstruct_len;
> +
> +	u8 *module_start	= sigstruct_end;
> +
> +	/* Check the calculated payload size against the image size. */
> +	if (TDX_IMAGE_HEADER_SIZE + sigstruct_len + module_len != image_len)
> +		return -EINVAL;
> +
> +	/* Reject unsupported tdx_image ABI versions. */
> +	if (header->version != TDX_IMAGE_VERSION_2)
> +		return -EINVAL;
> +
> +	if (header->sigstruct_nr_pages > SEAMLDR_MAX_NR_SIG_PAGES ||
> +	    header->module_nr_pages > SEAMLDR_MAX_NR_MODULE_PAGES)
> +		return -EINVAL;
> +
> +	if (memcmp(header->signature, "TDX-BLOB", sizeof(header->signature)))
> +		return -EINVAL;
> +
> +	if (memchr_inv(header->reserved, 0, sizeof(header->reserved)))
> +		return -EINVAL;
> +
> +	populate_seamldr_params(params, sigstruct_start, header->sigstruct_nr_pages,
> +					module_start,    header->module_nr_pages);
> +	return 0;
> +}
> +

-- 
  Kiryl Shutsemau / Kirill A. Shutemov