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

[Nikolay Borisov <nik.borisov@suse.com>]

On 15.09.26 г. 13:26 ч., 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@linux.intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> ---
> 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;

I'm slightly confused by the parlance introduced here. TDX_FEATURES0_EXT 
indicates whether this tdx module supports extensions. And ext_required 
aka 0x3100000000000001 is, as per td_scope_metadata.json: "Extended 
Features Available Mask:  indicates the extended user and system 
features which are available for  the TD." So aren't by definition all 
add-on features extensions. How do you distinguish add-on which are 
extensions and those which aren't? I think it's best if ext_required is 
referred to as "ext_mask" or some such and not ext_required. That value 
is not about what is required but rather what is available, no ?

<snip>