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

Xu Yilun posted 31 patches 5 days, 23 hours ago
[PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
Posted by Xu Yilun 5 days, 23 hours ago
TDX Module supports optional TDX features (e.g. TDX Connect & TDX Module
Extensions) that won't be enabled by default. 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.

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();
+		/* These parameters requires version >= 1 */
+		seamcall_fn = TDH_SYS_CONFIG;
+	}
+
+	ret = seamcall_prerr(seamcall_fn, &args);
 
 	/* Free the array as it is not required anymore. */
 	kfree(tdmr_pa_array);
@@ -1537,6 +1546,11 @@ static int init_tdx_module(void)
 	if (ret)
 		goto err_free_pamts;
 
+	/* configuration to tdx module may change tdx_sysinfo, update it */
+	ret = get_tdx_sys_info(&tdx_sysinfo);
+	if (ret)
+		goto err_reset_pamts;
+
 	/* Config the key of global KeyID on all packages */
 	ret = config_global_keyid();
 	if (ret)
-- 
2.25.1
Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
Posted by Huang, Kai 15 hours ago
>  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();
> +		/* These parameters requires version >= 1 */
> +		seamcall_fn = TDH_SYS_CONFIG;
> +	}
> +
> +	ret = seamcall_prerr(seamcall_fn, &args);
>  
>  	/* Free the array as it is not required anymore. */
>  	kfree(tdmr_pa_array);
> @@ -1537,6 +1546,11 @@ static int init_tdx_module(void)
>  	if (ret)
>  		goto err_free_pamts;
>  
> +	/* configuration to tdx module may change tdx_sysinfo, update it */
> +	ret = get_tdx_sys_info(&tdx_sysinfo);
> +	if (ret)
> +		goto err_reset_pamts;
> +
>  	/* Config the key of global KeyID on all packages */
>  	ret = config_global_keyid();
>  	if (ret)

Maybe a more generic comment:

I don't quite like hard-coding opt-in TDX_FEATURES0_TDXCONNECT inside
config_tdx_module(), especially currently we just unconditionally opt it in
if the module support this feature.

Initializing TDX Connect (and other features via TDX Module Extensions)
consumes more memory.  It would be better if we can choose to opt-in when
the kernel has enabled TDX Connect (or any other feature via TDX module
Extensions) in the Kconfig.

Unfortunately we need to opt-in all these features together during module
initialization, so we cannot make tdx_enable() to accept the additional
features to enable, and in each in-kernel TDX user, call tdx_enable() with
the new feature that that TDX user concerns.

But I think it makes sense to have a dedicated place to calculate all opt-in
features.  E.g., assuming we eventually are going to support TDX Connect and
live migration:

static u64 get_ext_features_tdx_connect(struct tdx_sys_info * sysinfo)
{
	if (!IS_ENABLED(TDX_CONNECT))
		return 0;

	return sysinfo->features.tdx_features0 & TDX_FEATURES0_TDXCONNECT ?
		TDX_FEATURES0_TDXCONNECT : 0;
}

static u64 get_ext_features_live_migration(struct tdx_sys_info *sysinfo)
{
	u64 mig_features = TDX_FEATURES0_NRX | TDX_FEATURES0_NON_BLOCKING;

	if (!IS_ENABLED(TDX_LIVE_MIGRATION))
		return 0;

	return sysinfo->features.tdx_features0 & mig_features;
}

static u64 calculate_ext_features(struct tdx_sys_info *sysinfo)
{
	u64 ext_features = 0;

	ext_features |= get_ext_features_tdx_connect(sysinfo);

	ext_features |= get_ext_features_live_migration(sysinfo);

	return ext_features;
}

int init_tdx_module()
{
	u64 ext_features = calculate_ext_features(&tdx_sysinfo);

	ret = config_tdx_module(&tdx_tdmr_list, &tdx_global_keyid,
				ext_features);

	/* do other initializations like TDH.SYS.KEY.CONFIG */
	...
	/*
	 * TDX Module Extension features must be initialized
	 * after TDH.SYS.KEY.CONFIG.
	 */
	if (ext_features)
		ret = init_tdx_ext();	

	...
}

One nasty thing is per public spec R11 of TDH.SYS.CONFIG needs to be RTC if
TDX_CONNECT is on, so we still need some special handing in
config_tdx_module():

	if (ext_features & TDX_RFEAURES0_TDXCONNECT)
		args.r11 = ktime_get_real_seconds();

But I think this is acceptable.
Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
Posted by Edgecombe, Rick P 15 hours ago
On Wed, 2026-04-01 at 23:42 +0000, Huang, Kai wrote:
> Maybe a more generic comment:
> 
> I don't quite like hard-coding opt-in TDX_FEATURES0_TDXCONNECT inside
> config_tdx_module(), especially currently we just unconditionally opt it in
> if the module support this feature.
> 
> Initializing TDX Connect (and other features via TDX Module Extensions)
> consumes more memory.  It would be better if we can choose to opt-in when
> the kernel has enabled TDX Connect (or any other feature via TDX module
> Extensions) in the Kconfig.

Better how? TDX uses a lot of memory. There are many possible optimizations to
reduce this. Why focus on this one? Do we think any TDX users would actually
reconfigure there kernel for this reason?

I mean, I don't actually know how much memory this is, but to me the reasoning
doesn't seem in balance with the wider TDX situation.
Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
Posted by Huang, Kai 14 hours ago
On Wed, 2026-04-01 at 23:53 +0000, Edgecombe, Rick P wrote:
> On Wed, 2026-04-01 at 23:42 +0000, Huang, Kai wrote:
> > Maybe a more generic comment:
> > 
> > I don't quite like hard-coding opt-in TDX_FEATURES0_TDXCONNECT inside
> > config_tdx_module(), especially currently we just unconditionally opt it in
> > if the module support this feature.
> > 
> > Initializing TDX Connect (and other features via TDX Module Extensions)
> > consumes more memory.  It would be better if we can choose to opt-in when
> > the kernel has enabled TDX Connect (or any other feature via TDX module
> > Extensions) in the Kconfig.
> 
> Better how? TDX uses a lot of memory. There are many possible optimizations to
> reduce this. Why focus on this one? Do we think any TDX users would actually
> reconfigure there kernel for this reason?
> 
> I mean, I don't actually know how much memory this is, but to me the reasoning
> doesn't seem in balance with the wider TDX situation.

In another patch, Yilun said:

  For now, TDX Module Extensions consume relatively large amount of
  memory (~50MB).

Distros tend to enable all I assume.  I guess the CSPs tend to enable all as
well, but I am not sure whether CSPs can also choose to only enable basic
TDX w/o other features like runtime update, TDX Connect etc, depending on
how they want to "sell" TDX VMs.

E.g., I assume a TD without GPU passthrough could be cheaper the one which
has, right?  Can the CSPs host such TDs on dedicated machine pools?  Can
they choose to disable TDX Connect on these machines?

They might not care about losing ~50M memory, though, but that's a different
story, and it could be more in the future.

My thinking is, this series actually introduced a new "TDX_CONNNECT"
Kconfig, so why not only consume the memory when it's on?

At last, just my 2cents, I kinda overall don't agree we always assume
"everything will be on" but neglect avoiding unnecessary code/cost that is
not reachable when some option is off.  That would defeat the purpose of
having Kconfig option.
Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
Posted by Dave Hansen 14 hours ago
On 4/1/26 17:40, Huang, Kai wrote:
...
> They might not care about losing ~50M memory, though, but that's a different
> story, and it could be more in the future.

If anyone _does_ care about 50MB of memory, I expect they'll speak up.
Until they do, could we please err on the side of least complexity?

I'm not even sure the Kconfig is worth it to be honest.
Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
Posted by Huang, Kai 14 hours ago
On Wed, 2026-04-01 at 17:48 -0700, Dave Hansen wrote:
> On 4/1/26 17:40, Huang, Kai wrote:
> ...
> > They might not care about losing ~50M memory, though, but that's a different
> > story, and it could be more in the future.
> 
> If anyone _does_ care about 50MB of memory, I expect they'll speak up.
> Until they do, could we please err on the side of least complexity?

Yeah agreed.

> 
> I'm not even sure the Kconfig is worth it to be honest.

I am not sure either.

My main comment is to make the code more friendly to opt-in more features,
actually.
Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
Posted by Huang, Kai 1 day, 5 hours ago
> 
> TDX Module updates global metadata when optional features are enabled.
> Host should update the cached tdx_sysinfo to reflect these changes.
> 
> 
[...]

>  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();
> +		/* These parameters requires version >= 1 */
> +		seamcall_fn = TDH_SYS_CONFIG;
> +	}
> +
> +	ret = seamcall_prerr(seamcall_fn, &args);
>  
>  	/* Free the array as it is not required anymore. */
>  	kfree(tdmr_pa_array);
> @@ -1537,6 +1546,11 @@ static int init_tdx_module(void)
>  	if (ret)
>  		goto err_free_pamts;
>  
> +	/* configuration to tdx module may change tdx_sysinfo, update it */
> +	ret = get_tdx_sys_info(&tdx_sysinfo);
> +	if (ret)
> +		goto err_reset_pamts;
> +

How about put this into config_tdx_module()?

In this way you can only update global metadata when there's new feature
being opted in, and at the meantime, avoid making init_tdx_module() more
complicated.
Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
Posted by Nikolay Borisov 2 days, 4 hours ago

On 27.03.26 г. 18:01 ч., Xu Yilun wrote:
> TDX Module supports optional TDX features (e.g. TDX Connect & TDX Module
> Extensions) that won't be enabled by default. 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.
> 
> 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)

Since newer versions of tdx module apis are backwards compatible with 
older ones, and v0 are actually deprecated why have both definitions?


<snip>