Re: [PATCH v3 08/26] x86/virt/seamldr: Retrieve P-SEAMLDR information

[Dave Hansen <dave.hansen@intel.com>]

On 1/23/26 06:55, Chao Gao wrote:
> P-SEAMLDR returns its information e.g., version and supported features, in
> response to the SEAMLDR.INFO SEAMCALL.
> 
> This information is useful for userspace. For example, the admin can decide
> which TDX module versions are compatible with the P-SEAMLDR according to
> the P-SEAMLDR version.
> 
> Add and export seamldr_get_info() which retrieves P-SEAMLDR information by

I don't need to know what the function name is. That's in the code.

> invoking SEAMLDR.INFO SEAMCALL in preparation for exposing P-SEAMLDR
> version and other necessary information to userspace.

I also want to know what spec you are getting this out of.

I think it's also worth calling out that there are SEAMLDR calls for both:

	SEAMLDR_INFO
and
	SEAMLDR_SEAMINFO

Which is astonishingly confusing. Please have mercy on folks that are
looking through the docs for the first time and explain this.

> diff --git a/arch/x86/include/asm/seamldr.h b/arch/x86/include/asm/seamldr.h
> new file mode 100644
> index 000000000000..d1e9f6e16e8d
> --- /dev/null
> +++ b/arch/x86/include/asm/seamldr.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_SEAMLDR_H
> +#define _ASM_X86_SEAMLDR_H
> +
> +#include <linux/types.h>
> +
> +struct seamldr_info {

/*
 * This called the "SEAMLDR_INFO" data structure and is defined
 * in "SEAM Loader (SEAMLDR) Interface Specification".
 */


> +	u32	version;
> +	u32	attributes;
> +	u32	vendor_id;
> +	u32	build_date;
> +	u16	build_num;
> +	u16	minor_version;
> +	u16	major_version;
> +	u16	update_version;
> +	u8	reserved0[4];

Why not label this:

	u32	acm_x2apicid: /* unused by kernel */

?

> +	u32	num_remaining_updates;
> +	u8	reserved1[224];
> +} __packed;
> +
> +#ifdef CONFIG_INTEL_TDX_MODULE_UPDATE
> +const struct seamldr_info *seamldr_get_info(void);
> +#else
> +static inline const struct seamldr_info *seamldr_get_info(void) { return NULL; }
> +#endif
> +
> +#endif
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index b99d73f7bb08..6a83ae405fac 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -9,9 +9,16 @@
>  #include <linux/irqflags.h>
>  #include <linux/types.h>
>  
> +#include <asm/seamldr.h>
> +
>  #include "seamcall.h"
>  
> -static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> +/* P-SEAMLDR SEAMCALL leaf function */
> +#define P_SEAMLDR_INFO			0x8000000000000000


/*
 * The SEAMLDR.INFO documentation requires
 * this to be aligned to a 256-byte boundary.
 */
> +static struct seamldr_info seamldr_info __aligned(256);
> +
> +static inline int seamldr_call(u64 fn, struct tdx_module_args *args)
>  {
>  	unsigned long flags;
>  	u64 vmcs;
> @@ -54,3 +61,11 @@ static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
>  	WARN_ONCE(1, "Failed to save/restore the current VMCS");
>  	return -EIO;
>  }
> +
> +const struct seamldr_info *seamldr_get_info(void)
> +{
> +	struct tdx_module_args args = { .rcx = __pa(&seamldr_info) };
> +
> +	return seamldr_call(P_SEAMLDR_INFO, &args) ? NULL : &seamldr_info;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");

I'd also prefer a

	BUILD_BUG_ON(sizeof(struct seamldr_info) != 2048);

just as a sanity check. It doesn't cost anything and it makes sure that
as you muck around with reserved fields and padding that there's at
least one check making sure it's OK.