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

[Xu Yilun]

On Mon, Jul 06, 2026 at 09:44:29PM +0000, Edgecombe, Rick P wrote:
> 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.

OK. I will refactored this paragraph:

  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 for the same family: a base
  function number and a version together encode the full function number.

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

I think these are also about "make every word count", I'll drop all
3 places above.

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

OK. I'll delete these adverbs.

> 
> > Signed-off-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> 
> 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.

OK, I'll drop the wrapper.

> 
> > +
> > +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);
> >  }