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

[Xu Yilun]

On Tue, Sep 29, 2026 at 08:26:43PM +0300, Nikolay Borisov wrote:
> 
> 
> 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@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;
> 
> 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.

I checked the latest json files [1]. I think there is misreading somehow.

ext_required - 0x3100000000000001 is in global_metadata.json: "Return true if
the TDH.EXT.INIT is required to be called".

And what you've read is:
XFAM - 0x1110000300000001 - td_scope_metadata.json: "Extended Features
Available Mask:  indicates the extended user and system features which are
available for the TD."

[1] https://cdrdv2.intel.com/v1/dl/getContent/795381

> How do you distinguish add-on which are extensions and those
> which aren't?

In general TDX initialization phase, the host doesn't have to tell. The
TDX module knows which features are backed by extensions then decide
whether the extensions initialization is needed according to which
features host wants to enable.

According to TDX module ABI spec, now there are 5 add-on feature bits:
TDX_CONNECT, QUOTE, MIG_SETUP which need the extensions;
NON_BLOCKING_EXPORT, TDID_VMID_REPORTING which not need.

So for example:

  TDH.SYS.CONFIG(QUOTE | TDX_CONNECT | NON_BLOCKING_EXPORT) => ext_required == 1

  TDH.SYS.CONFIG(NON_BLOCKING_EXPORT | TDID_VMID_REPORTING) => ext_required == 0

Note that tdx module updates the ext_required after TDH.SYS.CONFIG, so
it is about what is required.


BTW: we've discussed that ext_required reading is not necessary [2],
just make TDH.EXT.INIT return SUCCESS if extensions are not required.

[2] https://lore.kernel.org/all/8c7a630c25f04dcf5b19e29d3b271c611bf90378.camel@xxxxxxxxx/

> 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>
>