Re: [PATCH 5/6] x86/virt/tdx: Make TDX module initialize the extensions

[Edgecombe, Rick P]

On Fri, 2026-08-21 at 11:29 +0800, Xu Yilun wrote:
> TDX module extensions need memory for their own internal state and data
> to serve SEAMCALL leaves.
> 

For "their own", I think "own" is superfluous.

>  Several add-on features depend on the
> extensions to execute their SEAMCALL leaves.
> 
> After providing all required memory to the TDX module, initialize TDX
> module extensions via TDH.EXT.INIT
> 

What exactly does TDH.EXT.INIT actually do, and why can't whatever that is
happen during TDH.EXT.MEM.ADD? Not saying we need to jam anything in awkwardly.
Can we say this is analogous to TDH.SYS.CONFIG but for the extensions? It only
exists because SYS.CONFIG gives the features to enable before the memory is
there to actually do it. So it needs a second call?


> , then those add-on features can use
> their SEAMCALL leaves normally.

Out of curiosity, what happens if you don't complete these steps and call one of
the extension seamcalls?

> 
> Signed-off-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> Reviewed-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
> Reviewed-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>
> Reviewed-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>

Code looks good. Except that it leaves me wondering if this could have all been
a while loop around tdh_sys_confg() that gives memory on a memory error code.
Actually it looks like we have a TDX_EXT_MEMORY_POOL_REQUIRED error code to use.

Consider something like this:

do {
	ret = tdh_sys_init();
	if (ret == TDX_EXT_MEMORY_POOL_REQUIRED)
		tdh_ext_mem_add() //single page
while (ret == TDX_EXT_MEMORY_POOL_REQUIRED)

If that could technically work, I guess the benefit of the current approach is
that it allows to use contiguous physical allocations. Not sure if you think
that simpler snippet would actually be that simple in the real world. But if we
are basically building all this to help fragmentation, we should say it.

> ---
> v1:
>  - Fix return value for SEAMCALL helpers (Chao)
>  - Print SEAMCALL error code for SEAMCALL helpers (Xiaoyao)
>  - Changelog & code comments
> ---
>  arch/x86/virt/vmx/tdx/tdx.h |  1 +
>  arch/x86/virt/vmx/tdx/tdx.c | 23 ++++++++++++++++++++++-
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 52888424fe7d..1f43d2eb2345 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -48,6 +48,7 @@
>  #define TDH_SYS_CONFIG			45
>  #define TDH_SYS_SHUTDOWN		52
>  #define TDH_SYS_UPDATE			53
> +#define TDH_EXT_INIT			60
>  #define TDH_EXT_MEM_ADD			61
>  #define TDH_SYS_DISABLE			69
>  
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 8c2fdaf0b8c0..873b8393f32f 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1289,6 +1289,23 @@ static __init int tdx_ext_mem_setup(void)
>  	return ret;
>  }
>  
> +static __init int tdx_ext_init(void)
> +{
> +	struct tdx_module_args args = {};
> +	u64 ret;
> +
> +	do {
> +		ret = seamcall(TDH_EXT_INIT, &args);

What happens if no init is needed? Does it return success or error? I'm
wondering if we need to really check ext_required and can just call TDH_EXT_INIT
unconditionally. We need to check for TDX_FEATURES0_EXT in any case. But do we
need both checks?

> +	} while (ret == TDX_INTERRUPTED_RESUMABLE);
> +
> +	if (ret != TDX_SUCCESS) {
> +		pr_err("TDH.EXT.INIT failed: 0x%016llx\n", ret);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
>  static __init int init_tdx_module_extensions(void)
>  {
>  	int ret;
> @@ -1308,7 +1325,11 @@ static __init int init_tdx_module_extensions(void)
>  	if (!tdx_sysinfo.ext.ext_required)
>  		return 0;
>  
> -	return tdx_ext_mem_setup();
> +	ret = tdx_ext_mem_setup();
> +	if (ret)
> +		return ret;
> +
> +	return tdx_ext_init();
>  }
>  
>  static __init int init_tdx_module(void)