Re: [PATCH v2 3/5] x86/virt/tdx: Detect if the extensions initialization is required

[Edgecombe, Rick P]

On Tue, 2026-09-15 at 18:26 +0800, Xu Yilun wrote:
> Some add-on features require TDX module extensions. The TDX module
> provides a metadata field "ext_required" to indicate this requirement.
> 
> Add the first step of TDX module extensions initialization by detecting
> if the extensions are required:
> 
>   1. Check if the extensions are supported via TDX_FEATURES0_EXT. If
>      not, ext_required is not readable.
>   2. Check if any TDX feature needs the extensions via ext_required.
> 
> Skip the extensions initialization when it is not required.
> 
> Currently all metadata fields are read at the very beginning of TDX
> module initialization. However, ext_required is only valid after the
> add-on feature configuration, so it cannot use the existing metadata
> reading method.
> 
> Add a dedicated metadata reading interface for the extensions, call it
> after add-on feature configuration.
> 
> Signed-off-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> Reviewed-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>
> ---
> v1:
>  - Include struct tdx_sys_info_ext in struct tdx_sys_info.
> ---
>  arch/x86/include/asm/tdx.h                  |  1 +
>  arch/x86/include/asm/tdx_global_metadata.h  |  5 ++++
>  arch/x86/virt/vmx/tdx/tdx.c                 | 28 +++++++++++++++++++++
>  arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 14 +++++++++++
>  4 files changed, 48 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 89e97d5761d8..6657f2db0330 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -36,6 +36,7 @@
>  /* Bit definitions of TDX_FEATURES0 metadata field */
>  #define TDX_FEATURES0_TD_PRESERVING	BIT_ULL(1)
>  #define TDX_FEATURES0_NO_RBP_MOD	BIT_ULL(18)
> +#define TDX_FEATURES0_EXT		BIT_ULL(39)
>  
>  #ifndef __ASSEMBLER__
>  
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index 41150d546589..fe3fe91de71f 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -44,12 +44,17 @@ struct tdx_sys_info_handoff {
>  	u16 module_hv;
>  };
>  
> +struct tdx_sys_info_ext {
> +	bool ext_required;
> +};
> +
>  struct tdx_sys_info {
>  	struct tdx_sys_info_version version;
>  	struct tdx_sys_info_features features;
>  	struct tdx_sys_info_tdmr tdmr;
>  	struct tdx_sys_info_td_ctrl td_ctrl;
>  	struct tdx_sys_info_td_conf td_conf;
> +	struct tdx_sys_info_ext ext;
>  };
>  
>  #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 763c2d1b25d0..916a8906da10 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1181,6 +1181,30 @@ static __init int init_tdmrs(struct tdmr_info_list *tdmr_list)
>  	return 0;
>  }
>  
> +static __init int init_tdx_module_extensions(void)
> +{
> +	int ret;
> +
> +	if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> +		return 0;
> +
> +	ret = get_tdx_sys_info_ext(&tdx_sysinfo.ext);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * ext_required indicates if any add-on features requiring TDX module
> +	 * extensions are configured via TDH.SYS.CONFIG. If none, skip the
> +	 * initialization.
> +	 */
> +	if (!tdx_sysinfo.ext.ext_required)
> +		return 0;

Hmm, now that we don't need to do this on update, we don't need to save
ext.ext_required metadata.

Also, per:
https://lore.kernel.org/all/aox8DgpnSnhZRfCv@yilunxu-OptiPlex-7050/
the reason we even need to check anything is because TDH_EXT_INIT returns a
strange error code TDX_EXT_MEMORY_POOL_REQUIRED. In the case of a TDX module
that supports extensions, but none that the kernel knows about... what if
TDH_EXT_INIT returned SUCCESS meaning that all requested extensions (0 of them)
are initialized.

AFAICT the specs don't describe what to do in this situation. But does describe
SUCCESS as a possible error code. And TDX_EXT_MEMORY_POOL_REQUIRED is wrong. So
its probably the most sensible bug fix anyway.

Then basically this patch goes away? We still check TDX_FEATURES0_EXT, but it
can be squashed into the other one?


> +
> +	/* TODO: add the extensions enabling steps here */
> +
> +	return 0;
> +}
> +
>  static __init int init_tdx_module(void)
>  {
>  	int ret;
> @@ -1235,6 +1259,10 @@ static __init int init_tdx_module(void)
>  	if (ret)
>  		goto err_reset_pamts;
>  
> +	ret = init_tdx_module_extensions();
> +	if (ret)
> +		goto err_reset_pamts;
> +
>  	pr_info("%lu KB allocated for PAMT\n", tdmrs_count_pamt_kb(&tdx_tdmr_list));
>  
>  out_put_tdxmem:
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index e49c300f23d4..b9e1c011a990 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -131,3 +131,17 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>  
>  	return ret;
>  }
> +
> +static __init int get_tdx_sys_info_ext(struct tdx_sys_info_ext *sysinfo_ext)
> +{
> +	int ret;
> +	u64 val;
> +
> +	ret = read_sys_metadata_field(0x3100000000000001, &val);
> +	if (ret)
> +		return ret;
> +
> +	sysinfo_ext->ext_required = val;
> +
> +	return 0;
> +}