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

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

[Xu Yilun]

SEAMCALL invokes TDX module functions using a function number and
parameters. To extend the functionalities 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.

An existing SEAMCALL helper (TDH.VP.INIT) is already using the version
field. Having the caller pack the version into the function number
open-codes the ABI layout.

Add a version field in struct tdx_module_args [1], 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. Encode the tdx_module_args.version in the
function number before calling into 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>
---
Two alternative schemes were considered:

1. Define versioned macros like TDH_VP_INIT_V0, TDH_VP_INIT_V1, etc.
   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
   forces all stable SEAMCALL helpers to pass a meaningless '0'
   argument. The magic '0' or '1' values at caller sites are not
   descriptive.

Change in v2:
- 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         |  2 ++
 arch/x86/virt/vmx/tdx/seamcall_internal.h | 10 ++++++++++
 arch/x86/virt/vmx/tdx/tdx.h               |  8 --------
 arch/x86/virt/vmx/tdx/tdx.c               |  5 +++--
 4 files changed, 15 insertions(+), 10 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..53d7ab037750 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,14 @@ 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)
+
 static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
 						  struct tdx_module_args *args)
 {
@@ -39,6 +48,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
 	 */
 	this_cpu_write(cache_state_incoherent, true);
 
+	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
 	return func(fn, args);
 }
 
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);
 
-- 
2.25.1

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

[Dave Hansen]

On 7/8/26 10:03, Xu Yilun wrote:
> +/*
> + * SEAMCALL leaf:
> + *
> + * Bit 15:0	Leaf number
> + * Bit 23:16	Version number
> + */
> +#define SEAMCALL_VERSION_MASK		GENMASK_U64(23, 16)
> +
>  static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>  						  struct tdx_module_args *args)
>  {
> @@ -39,6 +48,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>  	 */
>  	this_cpu_write(cache_state_incoherent, true);
>  
> +	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
>  	return func(fn, args);
>  }

This is really looking fragmented and inconsistent.

What if someone *does* set the version bits in 'fn'? Also, if the "leaf
number" is just 16 bits, why is it a u64 in the API?

Additionally, look at this:

> /*
>  * 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
>  */
> struct tdx_module_args {

"version" doesn't fit this comment, does it? It's not a register.

If "fn" is just the 16-bit leaf number it should be a u16 everywhere.
Then there's no worry about the version number leaking in there
somewhere. The type can't even _carry_ the version number.

As a general rule, I dislike doing things in assembly that can be done
in C. But, in this case, we have some pretty darn simple assembly doing
a pretty simple job: marshaling data out of 'tdx_module_args' and in to
registers.

If we add a new argument to 'tdx_module_args' it seems like the most
consistent thing to do would be to extend the assembly to marshal it too.

We already have:

        /* Move Leaf ID to RAX */
        mov %rdi, %rax

and it wouldn't be rocket science to add two instructions to get
->version in to place:

	/* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
	movzbl	TDX_MODULE_version(%rsi), %eax
	shl	$16, %eax
	/* Leaf number arg -> RAX[15:0]; Preserve [23:16]. */
	mov	%di, %ax

I don't *love* this. But, to be honest, I'd never even seen
FIELD_MODIFY() before.

Also, please double and triple check the assembly above. This does
depend pretty heavily on the rules around register naming and width, and
x86 is not exactly the most straightforward ISA in the world there.

I also wouldn't be shocked if there's a nicer way to do this that
someone else dreams up.

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

[Xu Yilun]

On Wed, Jul 08, 2026 at 11:03:14AM -0700, Dave Hansen wrote:
> On 7/8/26 10:03, Xu Yilun wrote:
> > +/*
> > + * SEAMCALL leaf:
> > + *
> > + * Bit 15:0	Leaf number
> > + * Bit 23:16	Version number
> > + */
> > +#define SEAMCALL_VERSION_MASK		GENMASK_U64(23, 16)
> > +
> >  static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >  						  struct tdx_module_args *args)
> >  {
> > @@ -39,6 +48,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >  	 */
> >  	this_cpu_write(cache_state_incoherent, true);
> >  
> > +	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
> >  	return func(fn, args);
> >  }
> 
> This is really looking fragmented and inconsistent.
> 
> What if someone *does* set the version bits in 'fn'? Also, if the "leaf

Then these bits would be ignored. FIELD_MODIFY() would overwrite the
version bits with the value in args->version.

> number" is just 16 bits, why is it a u64 in the API?

Because the "leaf number" is actually "bit 15:0 + bit 63". Seamldr calls
also use this path. And their leaf definitions include bit 63, such as:

  /* P-SEAMLDR SEAMCALL leaf function */
  #define P_SEAMLDR_INFO                0x8000000000000000
  #define P_SEAMLDR_INSTALL             0x8000000000000001

Sorry I only moved existing "SEAMCALL leaf" comments here, but the full
definition in TDX module SPEC is:

  Bit 15:0      Leaf number
  Bit 23:16     Version number
  Bit 24        Interrupt mode /* Setting 1 causes irq-resume loop forever when irq disabled, Linux always sets 0 */
  Bit 62:25     Reserved, must be 0
  Bit 63        Invoke *P-SEAMLDR* calls

> 
> Additionally, look at this:
> 
> > /*
> >  * 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
> >  */
> > struct tdx_module_args {
> 
> "version" doesn't fit this comment, does it? It's not a register.

OK, I think I can add a sentence to the comment a bit:

  ...when requesting service from the TDX module. The 'version' is an exception,
  it is encoded in rax along with the Leaf number. This is a software...

[...]

> If we add a new argument to 'tdx_module_args' it seems like the most
> consistent thing to do would be to extend the assembly to marshal it too.
> 
> We already have:
> 
>         /* Move Leaf ID to RAX */
>         mov %rdi, %rax
> 
> and it wouldn't be rocket science to add two instructions to get
> ->version in to place:
> 
> 	/* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
> 	movzbl	TDX_MODULE_version(%rsi), %eax
> 	shl	$16, %eax
> 	/* Leaf number arg -> RAX[15:0]; Preserve [23:16]. */
> 	mov	%di, %ax

If we want to keep seamldr call work as is, we can't lose bit 63:

	/* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
	movzbl	TDX_MODULE_version(%rsi), %eax
	shl	$16, %eax
	/* Leaf number arg -> RCX[63] and RCX[15:0]. Zero rest of RCX */
	mov	%rdi, %rcx
	movabs	$0x800000000000ffff, %rdx
	and	%rdx, %rcx
	/* Combine Leaf number and version -> RAX */
	or	%rcx, %rax


Or, I prefer more to assert that the Leaf numbers only touch bit 63 & bit [15:0]
in C code, this catches buggy bits rather than ignore them, and simplifies
assembly code:

@@ -58,6 +58,8 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
        int retry = RDRAND_RETRY_LOOPS;
        u64 ret;

+       BUILD_BUG_ON(fn & ~0x800000000000ffffULL); /* TODO: proper Macros */

then:

	/* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
        movzbl  TDX_MODULE_version(%rsi), %eax
        shl     $16, %eax
	/* Combine Leaf ID and ABI version to RAX, they don't overlap */
        or      %rdi, %rax

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

[Kiryl Shutsemau]

On Wed, Jul 08, 2026 at 11:03:14AM -0700, Dave Hansen wrote:
> On 7/8/26 10:03, Xu Yilun wrote:
> > +/*
> > + * SEAMCALL leaf:
> > + *
> > + * Bit 15:0	Leaf number
> > + * Bit 23:16	Version number
> > + */
> > +#define SEAMCALL_VERSION_MASK		GENMASK_U64(23, 16)
> > +
> >  static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >  						  struct tdx_module_args *args)
> >  {
> > @@ -39,6 +48,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >  	 */
> >  	this_cpu_write(cache_state_incoherent, true);
> >  
> > +	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
> >  	return func(fn, args);
> >  }
> 
> This is really looking fragmented and inconsistent.
> 
> What if someone *does* set the version bits in 'fn'? Also, if the "leaf
> number" is just 16 bits, why is it a u64 in the API?
> 
> Additionally, look at this:
> 
> > /*
> >  * 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
> >  */
> > struct tdx_module_args {
> 
> "version" doesn't fit this comment, does it? It's not a register.
> 
> If "fn" is just the 16-bit leaf number it should be a u16 everywhere.
> Then there's no worry about the version number leaking in there
> somewhere. The type can't even _carry_ the version number.
> 
> As a general rule, I dislike doing things in assembly that can be done
> in C. But, in this case, we have some pretty darn simple assembly doing
> a pretty simple job: marshaling data out of 'tdx_module_args' and in to
> registers.

Ughh.. I think it is totally wrong direction.

The version is not a register operand, it is part of the function
number. And RAX carries more than leaf+version: bit 24 is
INTERRUPT_MODE (enumerated by TDX_FEATURES0 bit 62) and bit 63 selects
P-SEAMLDR. If the version gets a struct field marshaled in assembly,
do we add fields for these bits too? Composing 'fn' as u64 in the
caller is the honest representation of the ABI.

Rather than pushing the version all the way down to assembly, let's
look if we can salvage what was proposed initially[1].

I know you didn't like it, but the problem there was that the version
was invisible: TDH_SYS_UPDATE was silently v1 and the old one got
renamed to TDH_SYS_UPDATE_V0. With the version spelled out at the call
site, that objection goes away:

int tdx_module_run_update(void)
{
	struct tdx_module_args args = {};
	u64 seamcall_fn = TDH_SYS_UPDATE;
	int ret;

	if (tdx_addon_feature0) {
		seamcall_fn |= SEAMCALL_LEAF_VER(1);
		args.r9 = tdx_addon_feature0;
	}

	ret = seamcall_prerr(seamcall_fn, &args);
	if (ret)
		return ret;
	...
}

And tdh_vp_init() stays what it is in mainline today, just with the
macro instead of the open-coded shift:

	/* apicid requires version == 1. */
	return seamcall(TDH_VP_INIT | SEAMCALL_LEAF_VER(1), &args);

This also addresses all three complaints above at once: there is no
FIELD_MODIFY() to silently overwrite version bits in 'fn' as it is
exactly what the caller wrote, tdx_module_args stays a pure register
structure, and the assembly is untouched.

[1] https://lore.kernel.org/kvm/20260618081355.3253581-3-yilun.xu@linux.intel.com/

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

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

[Dave Hansen]

On 7/10/26 09:21, Kiryl Shutsemau wrote:
> The version is not a register operand, it is part of the function
> number.

I'm not sure why that's important.

Sure, the arg structure is all full registers now. But it's not even all
the registers because RAX isn't there. So let's say we made the data
structure smarter:

struct tdx_module_args {
        union {
                u64 rax;
                struct {
                        u16 function_nr;
                        u8  version
                        u8  padding0;
                        u32 flags;
                };
        };
        u64 rcx;
        u64 rdx;
	...
}

All of the:

        struct tdx_module_args args = {};

instances would default to version=0. It would take a one-liner to get
bit 63 set:

 static int seamldr_call(u64 fn, struct tdx_module_args *args)
 {
 ...
+	args->flags |= SEAMLDR_BIT;
        guard(raw_spinlock)(&seamldr_lock);
        return seamcall_prerr(fn, args);
 }

The truth of the matter is that 'fn' *IS* the RAX from the TDX ABI.
We're carrying it around the kernel in that format, and it just doesn't
work very well.

The alternative is to carry the logical pieces of RAX around the kernel
and them assemble RAX out of them in one (or very few) places. *Not* to
build RAX in the TDX module ABI early far from the TDX module ABI layer
itself.

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

[Kiryl Shutsemau]

On Thu, Jul 09, 2026 at 01:03:30AM +0800, Xu Yilun wrote:
> SEAMCALL invokes TDX module functions using a function number and
> parameters. To extend the functionalities 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.
> 
> An existing SEAMCALL helper (TDH.VP.INIT) is already using the version
> field. Having the caller pack the version into the function number
> open-codes the ABI layout.
> 
> Add a version field in struct tdx_module_args [1], 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. Encode the tdx_module_args.version in the
> function number before calling into assembly code.

The commit message covers the mechanics, but is silent on compatibility
with older TDX modules. That context matters for the design choice, so
regardless of where the implementation discussion lands, I think it
needs to be spelled out here.

Who is responsible for picking a version the module supports?

TDH.VP.INIT can hardcode version 1 only because KVM already refuses to
enable TDX on modules without TOPOLOGY_ENUM.

The TDH.SYS.UPDATE user from [1] is the opposite case: it has to pick
version 0 or 1 at runtime depending on whether add-on features are
configured, to keep working on modules that don't support them.

The second point is the actual argument for a version field in struct
tdx_module_args rather than encoding the version in the leaf defines:
the version is not always a compile-time property of the call site.

Without TDH.SYS.UPDATE context, the patch seems pointless.

> Link: https://lore.kernel.org/kvm/4f4b0f29-424b-45ed-8cfd-c77da2ea390f@intel.com/ # [1]

-- 
  Kiryl Shutsemau / Kirill A. Shutemov