Re: [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support

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

On Thu, 2026-07-02 at 22:46 +0800, Xu Yilun wrote:
> TDX uses the SEAMCALL instruction to invoke various TDX module
> functions. Just like the syscall, a SEAMCALL specifies the operation
> using a function number and parameters. Moreover, TDX also uses SEAMCALL
> versions to extend the functionalities of existing SEAMCALLs while
> keeping backward compatibility. Unlike syscall versions that assign
> brand new numbers, TDX segments the function number into a basic
> function number field and a version field. Together, they encode the new
> function number.

So isn't this pretty much like numbered seamcalls, except there is a special
format for generating seamcall2, seamcall3, etc? In the end you just use a
different number for a different version of the call. So it's just like
syscalls, except there is a pattern in the specific number for calls of the same
family.

> 
> An existing SEAMCALL (TDH.VP.INIT) helper is already using the version
                                    ^ maybe swap the order of these two?
> field. However, having the caller pack the version into the function
          ^Can drop this?
> number open-codes the ABI layout, making the SEAMCALL helper definition
> obscure and error prone.

Do we need the second part of the sentence?

> 
> Add a version field in struct tdx_module_args, so that most existing
> SEAMCALL helpers get a default "version == 0" behavior without code
> churn, while callers requiring extended functionalities can specify the
> version descriptively.
> 
> As an internal implementation detail,

^ Is this needed to make it clear?

>  encode the
> tdx_module_args.version in the function number before calling into
> assembly code.
> 
> Two alternative schemes were considered:
> 
> 1. Define versioned macros like TDH_VP_INIT_V0, TDH_VP_INIT_V1, etc.
>    However, this breaks naming consistency unless all existing stable
>    function macros are changed to TDH_XXX_V0.
> 
> 2. Add an explicit 'version' parameter to the base seamcall() API. This
>    unnecessarily forces all stable SEAMCALL helpers to pass a
>    meaningless '0' argument. Additionally, the magic '0' or '1' values
>    at caller sites are not descriptive.
> 

Dave was recently saying something to the effect of "make every word count". I
think we could lose some filler words.

> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>

How about a link to the thread where this was suggested.

> ---
>  arch/x86/include/asm/shared/tdx.h         |  2 ++
>  arch/x86/virt/vmx/tdx/seamcall_internal.h | 19 ++++++++++++++++++-
>  arch/x86/virt/vmx/tdx/tdx.h               |  8 --------
>  arch/x86/virt/vmx/tdx/tdx.c               |  5 +++--
>  4 files changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index f20e91d7ac35..b9aac2de233a 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -143,6 +143,8 @@ struct tdx_module_args {
>  	u64 rbx;
>  	u64 rdi;
>  	u64 rsi;
> +	/* ABI version, encoded in rax */
> +	u8  version;
>  };
>  
>  /* Used to communicate with the TDX module */
> diff --git a/arch/x86/virt/vmx/tdx/seamcall_internal.h b/arch/x86/virt/vmx/tdx/seamcall_internal.h
> index be5f446467df..7002e41cddad 100644
> --- a/arch/x86/virt/vmx/tdx/seamcall_internal.h
> +++ b/arch/x86/virt/vmx/tdx/seamcall_internal.h
> @@ -11,6 +11,7 @@
>  #ifndef _X86_VIRT_SEAMCALL_INTERNAL_H
>  #define _X86_VIRT_SEAMCALL_INTERNAL_H
>  
> +#include <linux/bitfield.h>
>  #include <linux/printk.h>
>  #include <linux/types.h>
>  #include <asm/archrandom.h>
> @@ -23,6 +24,22 @@ u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
>  
>  typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
>  
> +/*
> + * SEAMCALL leaf:
> + *
> + * Bit 15:0	Leaf number
> + * Bit 23:16	Version number
> + */
> +#define SEAMCALL_VERSION_MASK		GENMASK_U64(23, 16)

The annoying thing is that the path touched here is also used for seamldr calls
now, which afaict has no concept of version. Underscoring how much of a mess the
wrapper stack is.

> +
> +static __always_inline u64 __seamcall_encode_fn(sc_func_t func, u64 fn,
> +						struct tdx_module_args *args)
> +{

I have the same question as Xiaoyao. We have so many wrappers already.

> +	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
> +
> +	return func(fn, args);
> +}
> +
>  static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>  						  struct tdx_module_args *args)
>  {
> @@ -39,7 +56,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>  	 */
>  	this_cpu_write(cache_state_incoherent, true);
>  
> -	return func(fn, args);
> +	return __seamcall_encode_fn(func, fn, args);
>  }
>  
>  static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index bdfd0e1e337a..63e3acfb5d0c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -50,14 +50,6 @@
>  #define TDH_SYS_UPDATE			53
>  #define TDH_SYS_DISABLE			69
>  
> -/*
> - * SEAMCALL leaf:
> - *
> - * Bit 15:0	Leaf number
> - * Bit 23:16	Version number
> - */
> -#define TDX_VERSION_SHIFT		16
> -
>  /* TDX page types */
>  #define	PT_NDA		0x0
>  #define	PT_RSVD		0x1
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 42df8ea464c4..7a89e29b118c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1910,10 +1910,11 @@ u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
>  		.rcx = vp->tdvpr_pa,
>  		.rdx = initial_rcx,
>  		.r8 = x2apicid,
> +		/* apicid requires version == 1. */
> +		.version = 1,
>  	};
>  
> -	/* apicid requires version == 1. */
> -	return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
> +	return seamcall(TDH_VP_INIT, &args);
>  }
>  EXPORT_SYMBOL_FOR_KVM(tdh_vp_init);
>