From: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"yilun.xu@linux.intel.com" <yilun.xu@linux.intel.com>,
"x86@kernel.org" <x86@kernel.org>
Cc: "Gao, Chao" <chao.gao@intel.com>,
"Xu, Yilun" <yilun.xu@intel.com>,
"Hansen, Dave" <dave.hansen@intel.com>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"kas@kernel.org" <kas@kernel.org>,
"Li, Xiaoyao" <xiaoyao.li@intel.com>,
"djbw@kernel.org" <djbw@kernel.org>,
"linux-coco@lists.linux.dev" <linux-coco@lists.linux.dev>,
"Fang, Peter" <peter.fang@intel.com>
Subject: Re: [PATCH v3] x86/virt/tdx: Formalize SEAMCALL version encoding support
Date: Thu, 23 Jul 2026 18:13:22 +0000 [thread overview]
Message-ID: <d3d33a22f57883f937f28919b96a5fdccdc080af.camel@intel.com> (raw)
In-Reply-To: <20260722084634.131020-1-yilun.xu@linux.intel.com>
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);
>
next prev parent reply other threads:[~2026-07-23 18:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-22 8:46 [PATCH v3] x86/virt/tdx: Formalize SEAMCALL version encoding support Xu Yilun
2026-07-22 11:23 ` Kiryl Shutsemau
2026-07-23 5:43 ` Xu Yilun
2026-07-23 9:35 ` Nikolay Borisov
2026-07-23 18:13 ` Edgecombe, Rick P [this message]
2026-07-24 3:14 ` Xu Yilun
2026-07-24 17:05 ` Edgecombe, Rick P
2026-09-11 19:58 ` Kishen Maloor
2026-09-14 4:58 ` Xu Yilun
2026-09-15 0:18 ` Kishen Maloor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d3d33a22f57883f937f28919b96a5fdccdc080af.camel@intel.com \
--to=rick.p.edgecombe@intel.com \
--cc=chao.gao@intel.com \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=djbw@kernel.org \
--cc=kas@kernel.org \
--cc=linux-coco@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=peter.fang@intel.com \
--cc=x86@kernel.org \
--cc=xiaoyao.li@intel.com \
--cc=yilun.xu@intel.com \
--cc=yilun.xu@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.