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

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

[Kiryl Shutsemau]

On Wed, Jul 22, 2026 at 04:46:34PM +0800, Xu Yilun wrote:
> While both options have merits, I chose the latter from a
> software-centric perspective. I sent v3 to invite more discussion.

I still don't see what postponing the encoding to assembly buys us,
but it is not the hill I want to die on. The result is centralized
and correct, so let's move on.

Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

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

[Xu Yilun]

On Wed, Jul 22, 2026 at 12:23:04PM +0100, Kiryl Shutsemau wrote:
> On Wed, Jul 22, 2026 at 04:46:34PM +0800, Xu Yilun wrote:
> > While both options have merits, I chose the latter from a
> > software-centric perspective. I sent v3 to invite more discussion.
> 
> I still don't see what postponing the encoding to assembly buys us,

Sorry, I overlooked it. I think this gives us consistent view of what
'fn' is for all seamcall-related functions.

> but it is not the hill I want to die on. The result is centralized
> and correct, so let's move on.
> 
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>

Thanks!

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

[Edgecombe, Rick P]

On Wed, 2026-07-22 at 16:46 +0800, Xu Yilun wrote:
> SEAMCALL invokes TDX module functions using a function number and
> parameters. To extend the functionality of existing SEAMCALLs while
> keeping backward compatibility, TDX adds more numbered SEAMCALLs of the
> same family. This is just like syscalls, except that TDX defines a
> specific function number encoding pattern: a base function number and a
> version together encode the full function number.
> 
> One concern is the compatibility with older TDX modules which don't
> recognize the new SEAMCALLs. The kernel should decide which SEAMCALL
> version to use at runtime, selecting the minimum version number for the
> required functionality. It can't overwrite the function number with a
> new value at compile time.
> 
> Another concern is the obscure usage of the 'fn' parameter for SEAMCALL
> wrappers. An existing caller for TDH.VP.INIT packs the version into the
> 'fn' to match the low-level TDX ABI RAX layout. The RAX layout
> interprets some bits differently, such as INTERRUPT_MODE, SEAMLDR flag,
> which are not a good fit for the function number definition.
> 
> Refactor the SEAMCALL wrappers to allow for version selection. Add a
> version field in struct tdx_module_args [1], so that most existing
> callers get a default "version == 0" behavior without code churn, while
> callers requiring extended functionalities can specify the version
> descriptively. Enforce the 'fn' parameter as the base function number.
> Encode the base function number and tdx_module_args.version in RAX
> before invoking the SEAMCALL instruction in assembly code.
> 
> Link: https://lore.kernel.org/kvm/4f4b0f29-424b-45ed-8cfd-c77da2ea390f@intel.com/ # [1]
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> The main concern in v2 is: should we make 'fn' exactly match the RAX
> layout of TDX module ABI?
> 
> Kiryl's point is that SEAMCALL wrappers are the representation of the
> ABI functions, so make the 'fn' match the RAX and struct tdx_module_args
> match the other CPU registers, compose base leaf number & version into
> 'fn'. This saves callers from digging into low-level code to understand
> what happens.
> 
> Dave's point is that the RAX layout is logically unfriendly, we want
> SEAMCALL wrappers to be the logically sound software APIs to the rest of
> the system. So 'fn' only contains the base leaf number, version and
> other RAX fields are in struct tdx_module_args.
> 
> While both options have merits, I chose the latter from a
> software-centric perspective. I sent v3 to invite more discussion.

Yea, I think it's good enough. Let's keep the option to revisit it if needed,
when we get to shrinking the layers of seamcall wrappers. One question below...

> 
> Change in v3:
> - Move the RAX encoding in assembly code.
> - Compile-time check that fn only contain base leaf number bits
> - Add the full description of SEAMCALL leaf according to TDX module SPEC
> - Fix the description of struct tdx_module_args.
> 
> v2: https://lore.kernel.org/all/20260708170330.83850-1-yilun.xu@linux.intel.com/
> - Drop the C wrapper __seamcall_encode_fn()
> - Rewrite the first paragraph of the changelog
> - Make change log concise
> - Add a link to the thread where this was suggested
> - Move alternative schemes description below the separator
> 
> v1: https://lore.kernel.org/all/20260702144614.59464-1-yilun.xu@linux.intel.com/
> ---
>  arch/x86/include/asm/shared/tdx.h         | 10 +++++++---
>  arch/x86/virt/vmx/tdx/seamcall_internal.h | 19 +++++++++++++++++++
>  arch/x86/virt/vmx/tdx/tdx.h               |  8 --------
>  arch/x86/virt/vmx/tdx/tdxcall.S           | 10 ++++++++--
>  arch/x86/kernel/asm-offsets.c             |  1 +
>  arch/x86/virt/vmx/tdx/tdx.c               |  5 +++--
>  6 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index f20e91d7ac35..67490c330d91 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -122,9 +122,11 @@
>  #include <linux/compiler_attributes.h>
>  
>  /*
> - * Used in __tdcall*() to gather the input/output registers' values of the
> - * TDCALL instruction when requesting services from the TDX module. This is a
> - * software only structure and not part of the TDX module/VMM ABI
> + * Used in __tdcall*() to gather the input/output parameters of the TDCALL
> + * instruction when requesting services from the TDX module. 'version' is
> + * encoded in rax along with the Leaf number. Others are raw input/output
> + * registers' values. This is a software only structure and not part of the TDX
> + * module/VMM ABI.
>   */
>  struct tdx_module_args {
>  	/* callee-clobbered */
> @@ -143,6 +145,8 @@ struct tdx_module_args {
>  	u64 rbx;
>  	u64 rdi;
>  	u64 rsi;
> +	/* Leaf 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..c586ddad072f 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,9 +24,27 @@ 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	Leaf ABI version number
> + * Bit 24	Pending interrupts detection mode
> + * Bit 63	1 for P-SEAMLDR leaf, 0 for TDX module leaf
> + */
> +#define SEAMCALL_LEAF_MASK		GENMASK_U64(15, 0)
> +#define SEAMCALL_SEAMLDR_MASK		BIT_U64(63)
> +
>  static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>  						  struct tdx_module_args *args)
>  {
> +	/*
> +	 * fn contains base leaf number for TDX module calls and P-SEAMLDR
> +	 * calls. Other fields in SEAMCALL leaf like leaf ABI version number
> +	 * are in struct tdx_module_args.
> +	 */
> +	BUILD_BUG_ON(fn & ~(SEAMCALL_LEAF_MASK | SEAMCALL_SEAMLDR_MASK));
> +

I guess this works because the __always_inline is enough for the compiler to
trace fn to a compile time constant up the callchain. It seems somewhat compiler
sensitive though. Did you convince yourself somehow that some compiler will not
complain?

>  	lockdep_assert_preemption_disabled();
>  
>  	/*
> 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/tdxcall.S b/arch/x86/virt/vmx/tdx/tdxcall.S
> index 016a2a1ec1d6..a01542c02c36 100644
> --- a/arch/x86/virt/vmx/tdx/tdxcall.S
> +++ b/arch/x86/virt/vmx/tdx/tdxcall.S
> @@ -45,8 +45,14 @@
>  .macro TDX_MODULE_CALL host:req ret=0 saved=0
>  	FRAME_BEGIN
>  
> -	/* Move Leaf ID to RAX */
> -	mov %rdi, %rax
> +	/* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
> +	movzbl	TDX_MODULE_version(%rsi), %eax
> +	shl	$16, %eax
> +	/*
> +	 * Combine base leaf number arg and leaf ABI version into RAX, they
> +	 * don't overlap.
> +	 */
> +	or	%rdi, %rax
>  
>  	/* Move other input regs from 'struct tdx_module_args' */
>  	movq	TDX_MODULE_rcx(%rsi), %rcx
> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> index 081816888f7a..b3c00ff4d819 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -95,6 +95,7 @@ static void __used common(void)
>  	OFFSET(TDX_MODULE_rbx, tdx_module_args, rbx);
>  	OFFSET(TDX_MODULE_rdi, tdx_module_args, rdi);
>  	OFFSET(TDX_MODULE_rsi, tdx_module_args, rsi);
> +	OFFSET(TDX_MODULE_version, tdx_module_args, version);
>  
>  	BLANK();
>  	OFFSET(BP_scratch, boot_params, scratch);
> 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);
>