Re: [PATCH 01/15] x86/virt/tdx: Read global metadata for TDX Module Extensions

[Sohil Mehta <sohil.mehta@intel.com>]

On 5/21/2026 8:41 PM, Xu Yilun wrote:
> Add reading of the global metadata for TDX Module Extensions.
> 
> TDX Module Extensions is an add-on feature enumerated by TDX_FEATURES0.
> But for the Module's integrity, Linux requires that all features that a
> Module advertises must have a complete, valid set of metadata, and the
> validation must succeed at core TDX initialization time.
> 
> Check TDX_FEATURES0 before reading these metadata. If a feature is
> advertised, a failure in reading associated metadata causes the entire
> TDX initialization to fail, otherwise skip.
> 
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
>  arch/x86/include/asm/tdx_global_metadata.h  |  6 ++++++
>  arch/x86/virt/vmx/tdx/tdx.h                 |  1 +
>  arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 16 ++++++++++++++++

The top comments in tdx_global_metadata.h and tdx_global_metadata.c say
that these files are autogenerated. I believe the script lives outside
the tree. Is there a plan to merge the script?

The generated code is optimized for space instead of readability. Also,
I see odd uncommented assignments u64 => u8/u16 all over the file. I am
assuming the upper bits are expected to be zero.

The patch is hard to review without the script. Can you post a link to
the updated script that led to this patch?


>  3 files changed, 23 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index 40689c8dc67e..533afe50a3f1 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -40,12 +40,18 @@ struct tdx_sys_info_td_conf {
>  	u64 cpuid_config_values[128][2];
>  };
>  
> +struct tdx_sys_info_ext {
> +	u16 memory_pool_required_pages;
> +	u8 ext_required;

The name ext_required seems like a boolean. It is also used like a
boolean later.
	if (!tdx_sysinfo.ext.ext_required)
		return 0;

But, IIUC, is it actually a mask that lists any feature that needs
extensions to work correctly? If so, it would be good to give it a name
that reflects its usage. Maybe:
features_requiring_ext or something better

As Xiaoyao mentioned, the struct requires a better explanation in the
commit log.

> +};

...

>  static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>  {
>  	int ret = 0;
> @@ -116,5 +129,8 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>  	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
>  	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
>  
> +	if (sysinfo->features.tdx_features0 & TDX_FEATURES0_EXT)

Other metadata reads aren't gated on feature checking. Is this check
manually added or autogenerated. If manually added, it should have a
code comment clarifying that.

> +		ret = ret ?: get_tdx_sys_info_ext(&sysinfo->ext);
> +
>  	return ret;
>  }