Re: [PATCH v2 02/17] x86/virt/tdx: Configure add-on features on TDX module init and update

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Thu, 2026-06-18 at 16:13 +0800, Xu Yilun wrote:
> In addition to basic TDX functionalities, TDX module provides add-on
> features that can be progressively enabled as the kernel supports them.
> The kernel should explicitly configure these features at boot or
> post-update initialization time. Configuring an add-on feature, such as
> TDX Quoting, that uses extension SEAMCALLs is the prerequisite for
> initializing TDX module extensions. TDX Quoting is the target feature to
> enable but defer it for now until full kernel support is in place.

I think this is too much project management type info for the commit log. This
patch is generic and could support any type of extension. I'd just list a few as
examples of extensions.

> 
> TDX module extends TDH.SYS.CONFIG and TDH.SYS.UPDATE with new bitmap
> input parameters to specify which add-on features to configure. The
> bitmap uses the same definitions as TDX_FEATURES0.

Assuming the v3 seamcall version patch is the final one, this should be updated
to talk about this with respect to how that patch exposes the seamcall version
concept.

> 
> For runtime update, Linux applies a policy

"applies a policy" sticks out to me here. Can we put this in simple terms of why
this isn't done. "Policy" makes it sound arbitrary rule that we are blindly
following.

>  that no newer features should

"newer" is ambiguous here. I think it means no features that were not previously
enabled, but it could come off as saying no features that were released after a
certain point.

> be added after update to avoid disrupting live TDX operations.



>  To adhere
> to this, TDH.SYS.UPDATE must configure the same features as the
> TDH.SYS.CONFIG. Record the kernel required add-on feature bitmap in a
> global var so that both phases can use it.
> 
> TDX module advances the version of TDH.SYS.CONFIG and TDH.SYS.UPDATE for
> the change, so use the latest version (v1) for add-on feature enabling.
> But supporting existing modules which only support v0 is still necessary
> until they are deprecated. In fact, it is unlikely that TDH.SYS.CONFIG
> ever needs to change again and the code would stay in v1.

Ha, these are hard things to predict.

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

We can't deprecate it today, so what is this saying exactly?

> 
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
>  arch/x86/virt/vmx/tdx/tdx.h |  6 ++++--
>  arch/x86/virt/vmx/tdx/tdx.c | 28 ++++++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index fbb520704662..a47e872480c7 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -58,9 +58,11 @@
>  #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)
>  #define TDH_SYS_SHUTDOWN		52
> -#define TDH_SYS_UPDATE			53
> +#define TDH_SYS_UPDATE_V0		53
> +#define TDH_SYS_UPDATE			SEAMCALL_LEAF_VER(TDH_SYS_UPDATE_V0,
> 1)
>  #define TDH_SYS_DISABLE			69
>  
>  /* TDX page types */
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 2a03152796e6..92305b5ea90d 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -57,6 +57,7 @@ static struct tdx_module_state tdx_module_state;
>  static u32 tdx_global_keyid __ro_after_init;
>  static u32 tdx_guest_keyid_start __ro_after_init;
>  static u32 tdx_nr_guest_keyids __ro_after_init;
> +static u64 tdx_addon_feature0 __ro_after_init;
>  
>  static DEFINE_IDA(tdx_guest_keyid_pool);
>  
> @@ -1004,9 +1005,18 @@ static __init int construct_tdmrs(struct list_head
> *tmb_list,
>  	return ret;
>  }
>  
> +static __init void set_tdx_addon_features(void)
> +{
> +	/*
> +	 * To add DICE-based TDX Quoting feature bit in tdx_addon_feature0
> when
> +	 * kernel is ready.
> +	 */

Can we not make this DICE specific?

> +}
> +
>  static __init int config_tdx_module(struct tdmr_info_list *tdmr_list,
>  				    u64 global_keyid)
>  {
> +	u64 seamcall_fn = TDH_SYS_CONFIG_V0;
>  	struct tdx_module_args args = {};
>  	u64 *tdmr_pa_array;
>  	size_t array_sz;
> @@ -1032,7 +1042,15 @@ static __init int config_tdx_module(struct
> tdmr_info_list *tdmr_list,
>  	args.rcx = __pa(tdmr_pa_array);
>  	args.rdx = tdmr_list->nr_consumed_tdmrs;
>  	args.r8 = global_keyid;
> -	ret = seamcall_prerr(TDH_SYS_CONFIG, &args);
> +
> +	set_tdx_addon_features();

This is a little opaque. Why not something like

tdx_addon_feature0 = TDX_KERNEL_SUPPORTED_ADDONS &
tdx_sysinfo.features.tdx_features0;

tdx_addon_feature0 could probably be named better too. It means the features
that the kernel has decided to use. HMm, any ideas?

> +
> +	if (tdx_addon_feature0) {
> +		args.r9 = tdx_addon_feature0;
> +		seamcall_fn = TDH_SYS_CONFIG;
> +	}

This kind of feels hacky to me. tdx_addon_feature0 is zero if there are none
supported. It shouldn't interfere with the v0 call to set it unconditionally?

I think the connection between tdx_addon_feature0 and TDH_SYS_CONFIG is a bit
tenuous too. It is set if a extension is found that the kernel supports.

What do you think about wrapping TDH_SYS_CONFIG. Have the wrapper take all the
args including tdx_addon_feature0. Then have the wrapper decide which seamcall
to use. Have a nice comment that says "Use version of seamcall that supports add
ons if we have any, use the old one if none are supported for backward
compatibility".

BTW, looking at the tdx module, if I'm reading this right, r9 used to be
ignored. So did it need a new seamcall version? Some other VMM was passing
garbage?

> +
> +	ret = seamcall_prerr(seamcall_fn, &args);
>  
>  	/* Free the array as it is not required anymore. */
>  	kfree(tdmr_pa_array);
> @@ -1314,10 +1332,16 @@ int tdx_module_shutdown(void)
>  
>  int tdx_module_run_update(void)
>  {
> +	u64 seamcall_fn = TDH_SYS_UPDATE_V0;
>  	struct tdx_module_args args = {};
>  	int ret;
>  
> -	ret = seamcall_prerr(TDH_SYS_UPDATE, &args);
> +	if (tdx_addon_feature0) {
> +		args.r9 = tdx_addon_feature0;
> +		seamcall_fn = TDH_SYS_UPDATE;
> +	}
> +
> +	ret = seamcall_prerr(seamcall_fn, &args);
>  	if (ret)
>  		return ret;
>