Re: [PATCH v2 03/17] x86/virt/tdx: Detect if the extensions initialization is required
Xiaoyao Li <[email protected]> Fri, 24 Jul 2026 16:42:33 +0800
| Newsgroups | dev.linux.lists.linux-coco,org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 6/18/2026 4:13 PM, Xu Yilun wrote: > TDX module extensions support extension SEAMCALLs that are preemptible > and resumable, unlike normal SEAMCALLs that run to completion while > monopolizing the CPU. This is not true. There are some normal (non-extension) SEAMCALLs also are preemptible and resumable. For example, - TDH.PHYMEM.CACHE.WB - TDH.SYS.DISABLE - TDH.MEM.SEPT.ADD with version 1 (There might be more. I didn't check all.) > This allows for higher-level API constructions, > so better supports some add-on features that implement higher order > security protocols. I think we don't need to explain this? Just that some add-on features require the functionalities of TDX module extensions is enough? > Add infrastructure to initialize TDX module extensions. Introduce the > initial step of this process by detecting if the extensions are required > by checking: > > 1. If the extensions are supported via TDX_FEATURES0_EXT. > 2. If any TDX add-on feature needs the extensions via a boolean > metadata field ext_required. > > Currently all metadata fields are read at the very beginning of basic > TDX initialization and stored in a global var. However, ext_required is > only valid after the add-on feature configuration, making it > incompatible with the existing metadata reading method. > > To resolve this lifetime conflict, add a dedicated runtime metadata > reading interface for the extensions, call it when the extensions > initialization starts, and leave the field out of the global var. In > this way, there is no confusion of when the metadata should be read. > > Signed-off-by: Xu Yilun <[email protected]> > --- > arch/x86/include/asm/tdx.h | 1 + > arch/x86/include/asm/tdx_global_metadata.h | 4 ++++ > arch/x86/virt/vmx/tdx/tdx.c | 25 +++++++++++++++++++++ > arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 14 ++++++++++++ > 4 files changed, 44 insertions(+) > > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h > index e5a9cf656c07..5fbf89d5317c 100644 > --- a/arch/x86/include/asm/tdx.h > +++ b/arch/x86/include/asm/tdx.h > @@ -35,6 +35,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..83fc657a438e 100644 > --- a/arch/x86/include/asm/tdx_global_metadata.h > +++ b/arch/x86/include/asm/tdx_global_metadata.h > @@ -52,4 +52,8 @@ struct tdx_sys_info { > struct tdx_sys_info_td_conf td_conf; > }; > > +struct tdx_sys_info_ext { > + bool ext_required; > +}; > + > #endif > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > index 92305b5ea90d..6f3596f11d25 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.c > +++ b/arch/x86/virt/vmx/tdx/tdx.c > @@ -1166,6 +1166,27 @@ static __init int init_tdmrs(struct tdmr_info_list *tdmr_list) > return 0; > } > > +static __init int init_tdx_module_extensions(void) > +{ > + struct tdx_sys_info_ext sysinfo_ext; > + int ret; > + > + if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT)) > + return 0; > + > + ret = get_tdx_sys_info_ext(&sysinfo_ext); > + if (ret) > + return ret; > + > + /* Skip if no feature requires TDX module extensions. */ > + if (!sysinfo_ext.ext_required) > + return 0; There was the discussion around it in [1]. I agree with the point to make kernel code simple. But it requires change/clarification from TDX spec, right? So will TDX spec change? If so, you would need to mention it somewhere. [1] https://lore.kernel.org/all/[email protected]/ > + /* TODO: add the extensions enabling steps here */ > + > + return 0; > +} > + > static __init int init_tdx_module(void) > { > int ret; > @@ -1220,6 +1241,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; > +}