Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature

Xu Yilun
2026-04-23T16:11:41Z

On Tue, Apr 21, 2026 at 06:19:59PM -0700, Dan Williams wrote:
> Xu Yilun wrote:
> > TDX Module supports optional TDX features (e.g. TDX Connect & TDX Module
> > Extensions) that won't be enabled by default.
>
> So this is another place where "optional" is misleading. For simplicity
> there is no mechanism to fallback from TDX Connect operation if present,
> at least in the core. The only optional aspects would be the mechanism
> that could be unloaded through the tdx_host driver.

I see. I think I should use 'extra' instead of 'optional' in all places.
I'll rephase the comments.

>
> .../me notices that other comments on this patch say the same, but do
> read on, another important detail about ktime_get_real_seconds() below.
>
> > It extends TDH.SYS.CONFIG for host to choose to enable them on bootup.
> >
> > Call TDH.SYS.CONFIG with a new bitmap input parameter to specify which
> > features to enable. The bitmap uses the same definitions as
> > TDX_FEATURES0. But note not all bits in TDX_FEATURES0 are valid for
> > configuration, e.g. TDX Module Extensions is a service that supports TDX
> > Connect, it is implicitly enabled when TDX Connect is enabled. Setting
> > TDX_FEATURES0_EXT in the bitmap has no effect.
> >
> > TDX Module advances the version of TDH.SYS.CONFIG for the change, so
> > use the latest version (v1) for optional feature enabling. But
> > supporting existing Modules which only support v0 is still necessary
> > until they are deprecated, enumerate via TDX_FEATURES0 to decide which
> > version to use.
>
> I would say this differently, it will always be the case that new
> kernels are needed to enable new features, but it is unlikely that
> TDH.SYS.CONFIG ever needs to change again. The v0 -> v1 transition means
> that feature bits are to be used here on out. So there is little value
> in worrying about deprecating v0 to save a couple lines of code in 5-7
> years when these original TDX platforms sunset.

OK. I'll use your rationale.

>
> > TDX Module updates global metadata when optional features are enabled.
> > Host should update the cached tdx_sysinfo to reflect these changes.
> >
> > Co-developed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> > ---
> > arch/x86/virt/vmx/tdx/tdx.h | 3 ++-
> > arch/x86/virt/vmx/tdx/tdx.c | 16 +++++++++++++++-
> > 2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> > index e5a9331df451..870bb75da3ba 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.h
> > +++ b/arch/x86/virt/vmx/tdx/tdx.h
> > @@ -58,7 +58,8 @@
> > #define TDH_PHYMEM_CACHE_WB 40
> > #define TDH_PHYMEM_PAGE_WBINVD 41
> > #define TDH_VP_WR 43
> > -#define TDH_SYS_CONFIG 45
> > +#define TDH_SYS_CONFIG_V0 45
> > +#define TDH_SYS_CONFIG SEAMCALL_LEAF_VER(TDH_SYS_CONFIG_V0, 1)
> >
> > /* TDX page types */
> > #define PT_NDA 0x0
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index 130214933c2f..0c5d6bdd810f 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1353,6 +1353,7 @@ static int construct_tdmrs(struct list_head *tmb_list,
> > static int config_tdx_module(struct tdmr_info_list *tdmr_list, u64 global_keyid)
> > {
> > struct tdx_module_args args = {};
> > + u64 seamcall_fn = TDH_SYS_CONFIG_V0;
> > u64 *tdmr_pa_array;
> > size_t array_sz;
> > int i, ret;
> > @@ -1377,7 +1378,15 @@ static int config_tdx_module(struct tdmr_info_list *tdmr_list, u64 global_keyid)
> > args.rcx = __pa(tdmr_pa_array);
> > args.rdx = tdmr_list->nr_consumed_tdmrs;
> > args.r8 = global_keyid;
> > - ret = seamcall_prerr(TDH_SYS_CONFIG, &args);
> > +
> > + if (tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_TDXCONNECT) {
> > + args.r9 |= TDX_FEATURES0_TDXCONNECT;
> > + args.r11 = ktime_get_real_seconds();
>
> Mainline has reason to not entertain this module requirement. The fact
> that passing zero is an error is useful to detect unsupported modules.

Ah, it is "useful"... :)

> An updated module would accept zero as indicating "VMM requests module
> disable all policy and mechanisms related to untrusted wall clock time".
> Specifically, there are several problems with this:
>
> 1/ No other TSM implementation requires the VMM to pass in an untrusted time
> 2/ The wall time may change and may require hooks to keep the module time
> up to date, but see point 1/, this would be a TDX special flower hook.
> 3/ Presumably this allows the module or the guest to do certificate expiration
> checks, but that is the responsibility of the relying party. The
> relying party may have reason to accept an "expired" cert as determined
> by VMM wall clock, and the guest presumably already has mechanisms to
> determine untrusted wall clock time from the VMM if it wants. Guests
> do not need TDX ABI for that.
>
> So I think Linux wants to pass 0 here and wait for modules that accept
> that as the start of TDX Connect support. As you said, given there are
> no released modules with TDX Connect there is time to make that first
> release drop this requirement.

I see. Actually the Module is about to remove the r11 RTC.

https://cdrdv2.intel.com/v1/dl/getContent/871617

I'll remove this r11.