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

[Xu Yilun]

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@xxxxxxxxx>
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
> > Signed-off-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> > ---
> >  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.