[RFC PATCH 02/20] x86/virt/tdx: Prepare to support P-SEAMLDR SEAMCALLs

Chao Gao posted 20 patches 7 months ago
[RFC PATCH 02/20] x86/virt/tdx: Prepare to support P-SEAMLDR SEAMCALLs
Posted by Chao Gao 7 months ago
P-SEAMLDR is another component alongside the TDX module within the
protected SEAM range. Software can invoke its functions by executing the
SEAMCALL instruction with the 63 bit of RAX set to 1. P-SEAMLDR SEAMCALLs
differ from those of the TDX module in terms of error codes and the
handling of the current VMCS.

In preparation for calling P-SEAMLDR functions to update the TDX module,
adjust the SEAMCALL infrastructure to support P-SEAMLDR SEAMCALLs and
expose a helper function.

Specifically,
1) P-SEAMLDR SEAMCALLs use a different error code for lack of entropy.
   Tweak sc_retry() to handle this difference.

2) Add a separate function to log the SEAMCALL leaf number and the error
   code.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Tested-by: Farrah Chen <farrah.chen@intel.com>
---
checking bit63 in sc_need_retry() may be suboptimal. An alternative is to
pass the "NO ENTROPY" error code from seamcall_prerr* and seamldr_prerr()
to sc_retry(). but this would need more code changes. I am not sure if it
is worthwhile.
---
 arch/x86/include/asm/tdx.h  | 20 +++++++++++++++++++-
 arch/x86/virt/vmx/tdx/tdx.c | 16 ++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h |  4 ++++
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 26ffc792e673..b507d5233b03 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -32,6 +32,11 @@
 #define TDX_SUCCESS		0ULL
 #define TDX_RND_NO_ENTROPY	0x8000020300000000ULL
 
+/* SEAMLDR SEAMCALL leaf function error codes */
+#define SEAMLDR_RND_NO_ENTROPY	0x8000000000030001ULL
+
+#define SEAMLDR_SEAMCALL_MASK	_BITUL(63)
+
 #ifndef __ASSEMBLER__
 
 #include <uapi/asm/mce.h>
@@ -104,6 +109,19 @@ void tdx_init(void);
 
 typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
 
+static inline bool is_seamldr_call(u64 fn)
+{
+	return fn & SEAMLDR_SEAMCALL_MASK;
+}
+
+static inline bool sc_need_retry(u64 fn, u64 error_code)
+{
+	if (is_seamldr_call(fn))
+		return error_code == SEAMLDR_RND_NO_ENTROPY;
+	else
+		return error_code == TDX_RND_NO_ENTROPY;
+}
+
 static inline u64 sc_retry(sc_func_t func, u64 fn,
 			   struct tdx_module_args *args)
 {
@@ -112,7 +130,7 @@ static inline u64 sc_retry(sc_func_t func, u64 fn,
 
 	do {
 		ret = func(fn, args);
-	} while (ret == TDX_RND_NO_ENTROPY && --retry);
+	} while (sc_need_retry(fn, ret) && --retry);
 
 	return ret;
 }
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 49267c865f18..b586329dd87d 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -65,6 +65,17 @@ static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
 	pr_err("SEAMCALL (%lld) failed: 0x%016llx\n", fn, err);
 }
 
+static inline void seamldr_err(u64 fn, u64 err, struct tdx_module_args *args)
+{
+	/*
+	 * Get the actual leaf number. No need to print the bit used to
+	 * differentiate between SEAMLDR and TDX module as the "SEAMLDR"
+	 * string in the error message already provides that information.
+	 */
+	fn &= ~SEAMLDR_SEAMCALL_MASK;
+	pr_err("SEAMLDR (%lld) failed: 0x%016llx\n", fn, err);
+}
+
 static inline void seamcall_err_ret(u64 fn, u64 err,
 				    struct tdx_module_args *args)
 {
@@ -102,6 +113,11 @@ static inline int sc_retry_prerr(sc_func_t func, sc_err_func_t err_func,
 #define seamcall_prerr_ret(__fn, __args)					\
 	sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
 
+int seamldr_prerr(u64 fn, struct tdx_module_args *args)
+{
+	return sc_retry_prerr(__seamcall, seamldr_err, fn, args);
+}
+
 /*
  * Do the module global initialization once and return its result.
  * It can be done on any cpu.  It's always called with interrupts
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 82bb82be8567..48c0a850c621 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -4,6 +4,8 @@
 
 #include <linux/bits.h>
 
+#include <asm/tdx.h>
+
 /*
  * This file contains both macros and data structures defined by the TDX
  * architecture and Linux defined software data structures and functions.
@@ -118,4 +120,6 @@ struct tdmr_info_list {
 	int max_tdmrs;	/* How many 'tdmr_info's are allocated */
 };
 
+int seamldr_prerr(u64 fn, struct tdx_module_args *args);
+
 #endif
-- 
2.47.1
Re: [RFC PATCH 02/20] x86/virt/tdx: Prepare to support P-SEAMLDR SEAMCALLs
Posted by Huang, Kai 6 months, 2 weeks ago
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 49267c865f18..b586329dd87d 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -65,6 +65,17 @@ static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
>  	pr_err("SEAMCALL (%lld) failed: 0x%016llx\n", fn, err);
>  }
>  
> +static inline void seamldr_err(u64 fn, u64 err, struct tdx_module_args *args)
> +{
> +	/*
> +	 * Get the actual leaf number. No need to print the bit used to
> +	 * differentiate between SEAMLDR and TDX module as the "SEAMLDR"
> +	 * string in the error message already provides that information.
> +	 */
> +	fn &= ~SEAMLDR_SEAMCALL_MASK;
> +	pr_err("SEAMLDR (%lld) failed: 0x%016llx\n", fn, err);
> +}
> +
>  static inline void seamcall_err_ret(u64 fn, u64 err,
>  				    struct tdx_module_args *args)
>  {
> @@ -102,6 +113,11 @@ static inline int sc_retry_prerr(sc_func_t func, sc_err_func_t err_func,
>  #define seamcall_prerr_ret(__fn, __args)					\
>  	sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
>  
> +int seamldr_prerr(u64 fn, struct tdx_module_args *args)
> +{
> +	return sc_retry_prerr(__seamcall, seamldr_err, fn, args);
> +}
> +
>  /*
>   * Do the module global initialization once and return its result.
>   * It can be done on any cpu.  It's always called with interrupts
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 82bb82be8567..48c0a850c621 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -4,6 +4,8 @@
>  
>  #include <linux/bits.h>
>  
> +#include <asm/tdx.h>
> +
>  /*
>   * This file contains both macros and data structures defined by the TDX
>   * architecture and Linux defined software data structures and functions.
> @@ -118,4 +120,6 @@ struct tdmr_info_list {
>  	int max_tdmrs;	/* How many 'tdmr_info's are allocated */
>  };
>  
> +int seamldr_prerr(u64 fn, struct tdx_module_args *args);
> +
>  #endif

Given there will be a dedicated seamldr.c, I don't quite like having
seamldr_prerr() in "tdx.h" and tdx.c.

Now for all SEAMCALLs used by KVM, we have a dedicated wrapper implemented
in tdx.c and exported for KVM to use.  I think we can move seamcall*() out
of <asm/tdx.h> to TDX host local since no other kernel code except the TDX
host core is supposed to use seamcall*().

This also cleans up <asm/tdx.h> a little bit, which in general makes code
cleaner IMHO.

E.g., how about we do below patch, and then you can do changes to support
P-SEAMLDR on top of it?

---

From 1cba17cf832d87a6ea34cc4db1798902e54d7577 Mon Sep 17 00:00:00 2001
From: Kai Huang <kai.huang@intel.com>
Date: Tue, 3 Jun 2025 12:55:53 +1200
Subject: [PATCH] x86/virt/tdx: Move low level SEAMCALL helpers out of
 <asm/tdx.h>

Now for all the SEAMCALL leaf functions that used by KVM, each has a
dedicated wrapper function implemented in TDX host core tdx.c and
exported for KVM to use.  In the future, if KVM or any other kernel
component needs more SEAMCALL, the TDX host core tdx.c should provide a
wrapper.  In other words, other than TDX host core code, seamcall*() are
not supposed to be used by other kernel components thus don't need to be
in <asm/tdx.h>.

Move seamcall*() and related code out of <asm/tdx.h> and put them to TDX
aost local.  This also cleans up <asm/tdx.h> a little bit, which is
getting bigger and bigger.

Don't just put seamcall*() to tdx.c since it is already very heavy, but
put seamcall*() to a new local "seamcall.h" which is more readable.
Also, currently tdx.c has seamcall_prerr*() helpers which additionally
prints error message when calling seamcall*() fails.  Move them and the
related code to "seamcall.h" too.  In such way all low level SEAMCALL
helpers and related code are in a dedicated place, which is much more
readable.

Signed-off-by: Kai Huang <kai.huang@intel.com>
---
 arch/x86/include/asm/tdx.h       | 24 -----------
 arch/x86/virt/vmx/tdx/seamcall.h | 71 ++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.c      | 46 +--------------------
 3 files changed, 72 insertions(+), 69 deletions(-)
 create mode 100644 arch/x86/virt/vmx/tdx/seamcall.h

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 8b19294600c4..a45323118b7e 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -97,31 +97,7 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
 #endif /* CONFIG_INTEL_TDX_GUEST && CONFIG_KVM_GUEST */
 
 #ifdef CONFIG_INTEL_TDX_HOST
-u64 __seamcall(u64 fn, struct tdx_module_args *args);
-u64 __seamcall_ret(u64 fn, struct tdx_module_args *args);
-u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
 void tdx_init(void);
-
-#include <asm/archrandom.h>
-
-typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
-
-static inline u64 sc_retry(sc_func_t func, u64 fn,
-                          struct tdx_module_args *args)
-{
-       int retry = RDRAND_RETRY_LOOPS;
-       u64 ret;
-
-       do {
-               ret = func(fn, args);
-       } while (ret == TDX_RND_NO_ENTROPY && --retry);
-
-       return ret;
-}
-
-#define seamcall(_fn, _args)           sc_retry(__seamcall, (_fn), (_args))
-#define seamcall_ret(_fn, _args)       sc_retry(__seamcall_ret, (_fn), (_args))
-#define seamcall_saved_ret(_fn, _args) sc_retry(__seamcall_saved_ret, (_fn), (_args))
 int tdx_cpu_enable(void);
 int tdx_enable(void);
 const char *tdx_dump_mce_info(struct mce *m);
diff --git a/arch/x86/virt/vmx/tdx/seamcall.h b/arch/x86/virt/vmx/tdx/seamcall.h
new file mode 100644
index 000000000000..54922f7bda3a
--- /dev/null
+++ b/arch/x86/virt/vmx/tdx/seamcall.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2025 Intel Corporation */
+#include <asm/tdx.h>
+#include <asm/archrandom.h>
+
+u64 __seamcall(u64 fn, struct tdx_module_args *args);
+u64 __seamcall_ret(u64 fn, struct tdx_module_args *args);
+u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
+
+typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
+
+static inline u64 sc_retry(sc_func_t func, u64 fn,
+                          struct tdx_module_args *args)
+{
+       int retry = RDRAND_RETRY_LOOPS;
+       u64 ret;
+
+       do {
+               ret = func(fn, args);
+       } while (ret == TDX_RND_NO_ENTROPY && --retry); 
+ 
+       return ret;
+}
+
+#define seamcall(_fn, _args)           sc_retry(__seamcall, (_fn), (_args))
+#define seamcall_ret(_fn, _args)       sc_retry(__seamcall_ret, (_fn), (_args))
+#define seamcall_saved_ret(_fn, _args) sc_retry(__seamcall_saved_ret, (_fn), (_args))
+
+typedef void (*sc_err_func_t)(u64 fn, u64 err, struct tdx_module_args *args);
+
+static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
+{
+       pr_err("SEAMCALL (0x%016llx) failed: 0x%016llx\n", fn, err);
+}
+
+static inline void seamcall_err_ret(u64 fn, u64 err,
+                                   struct tdx_module_args *args)
+{
+       seamcall_err(fn, err, args);
+       pr_err("RCX 0x%016llx RDX 0x%016llx R08 0x%016llx\n",
+                       args->rcx, args->rdx, args->r8);
+       pr_err("R09 0x%016llx R10 0x%016llx R11 0x%016llx\n",
+                       args->r9, args->r10, args->r11);
+}
+
+static inline int sc_retry_prerr(sc_func_t func, sc_err_func_t err_func,
+                                u64 fn, struct tdx_module_args *args)
+{
+       u64 sret = sc_retry(func, fn, args);
+
+       if (sret == TDX_SUCCESS)
+               return 0;
+
+       if (sret == TDX_SEAMCALL_VMFAILINVALID)
+               return -ENODEV;
+
+       if (sret == TDX_SEAMCALL_GP)
+               return -EOPNOTSUPP;
+
+       if (sret == TDX_SEAMCALL_UD)
+               return -EACCES;
+
+       err_func(fn, sret, args);
+       return -EIO;
+}
+
+#define seamcall_prerr(__fn, __args)                                           \
+       sc_retry_prerr(__seamcall, seamcall_err, (__fn), (__args))
+
+#define seamcall_prerr_ret(__fn, __args)                                       \
+       sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 2457d13c3f9e..b963e2d75713 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -38,6 +38,7 @@
 #include <asm/cpu_device_id.h>
 #include <asm/processor.h>
 #include <asm/mce.h>
+#include "seamcall.h"
 #include "tdx.h"

 static u32 tdx_global_keyid __ro_after_init;
@@ -57,51 +58,6 @@ static DEFINE_MUTEX(tdx_module_lock);
 static LIST_HEAD(tdx_memlist);

 static struct tdx_sys_info tdx_sysinfo;
-
-typedef void (*sc_err_func_t)(u64 fn, u64 err, struct tdx_module_args *args);
-
-static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
-{
-       pr_err("SEAMCALL (0x%016llx) failed: 0x%016llx\n", fn, err);
-}
-
-static inline void seamcall_err_ret(u64 fn, u64 err,
-                                   struct tdx_module_args *args)
-{
-       seamcall_err(fn, err, args);
-       pr_err("RCX 0x%016llx RDX 0x%016llx R08 0x%016llx\n",
-                       args->rcx, args->rdx, args->r8);
-       pr_err("R09 0x%016llx R10 0x%016llx R11 0x%016llx\n",
-                       args->r9, args->r10, args->r11);
-}
-
-static inline int sc_retry_prerr(sc_func_t func, sc_err_func_t err_func,
-                                u64 fn, struct tdx_module_args *args)
-{
-       u64 sret = sc_retry(func, fn, args);
-
-       if (sret == TDX_SUCCESS)
-               return 0;
-
-       if (sret == TDX_SEAMCALL_VMFAILINVALID)
-               return -ENODEV;
-
-       if (sret == TDX_SEAMCALL_GP)
-               return -EOPNOTSUPP;
-
-       if (sret == TDX_SEAMCALL_UD)
-               return -EACCES;
-
-       err_func(fn, sret, args);
-       return -EIO;
-}
-
-#define seamcall_prerr(__fn, __args)                                           \
-       sc_retry_prerr(__seamcall, seamcall_err, (__fn), (__args))
-
-#define seamcall_prerr_ret(__fn, __args)                                       \
-       sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
-
 /*
  * Do the module global initialization once and return its result.
  * It can be done on any cpu.  It's always called with interrupts
-- 
2.49.0
Re: [RFC PATCH 02/20] x86/virt/tdx: Prepare to support P-SEAMLDR SEAMCALLs
Posted by Chao Gao 6 months, 2 weeks ago
>Given there will be a dedicated seamldr.c, I don't quite like having
>seamldr_prerr() in "tdx.h" and tdx.c.
>
>Now for all SEAMCALLs used by KVM, we have a dedicated wrapper implemented
>in tdx.c and exported for KVM to use.  I think we can move seamcall*() out
>of <asm/tdx.h> to TDX host local since no other kernel code except the TDX
>host core is supposed to use seamcall*().
>
>This also cleans up <asm/tdx.h> a little bit, which in general makes code
>cleaner IMHO.
>
>E.g., how about we do below patch, and then you can do changes to support
>P-SEAMLDR on top of it?

looks good to me. I'd like to incorporate this patch into my series if
Kirill and Dave have no objections to this cleanup. I assume
seamldr_prerr() can be added to the new seamcall.h

Thanks for this suggestion.

>diff --git a/arch/x86/virt/vmx/tdx/seamcall.h b/arch/x86/virt/vmx/tdx/seamcall.h
>new file mode 100644
>index 000000000000..54922f7bda3a
>--- /dev/null
>+++ b/arch/x86/virt/vmx/tdx/seamcall.h
>@@ -0,0 +1,71 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+/* Copyright (C) 2025 Intel Corporation */
>+#include <asm/tdx.h>

If seamcall.h is intended to provide low-level helpers, including
<asm/tdx.h>, which is meant to offer high-level APIs for other components
such as KVM, seems a bit odd to me. But I suppose we can live with this.
Re: [RFC PATCH 02/20] x86/virt/tdx: Prepare to support P-SEAMLDR SEAMCALLs
Posted by Huang, Kai 6 months, 2 weeks ago
On Wed, 2025-06-04 at 21:14 +0800, Gao, Chao wrote:
> > Given there will be a dedicated seamldr.c, I don't quite like having
> > seamldr_prerr() in "tdx.h" and tdx.c.
> > 
> > Now for all SEAMCALLs used by KVM, we have a dedicated wrapper implemented
> > in tdx.c and exported for KVM to use.  I think we can move seamcall*() out
> > of <asm/tdx.h> to TDX host local since no other kernel code except the TDX
> > host core is supposed to use seamcall*().
> > 
> > This also cleans up <asm/tdx.h> a little bit, which in general makes code
> > cleaner IMHO.
> > 
> > E.g., how about we do below patch, and then you can do changes to support
> > P-SEAMLDR on top of it?
> 
> looks good to me. I'd like to incorporate this patch into my series if
> Kirill and Dave have no objections to this cleanup. I assume
> seamldr_prerr() can be added to the new seamcall.h
> 
> Thanks for this suggestion.

Seems we both think this is a good cleanup.  My TDX host kexec series also
conflicts with this so I think I can send this patch out first to see how
things will go.  At the meantime, yeah please carry it in your series.

> 
> > diff --git a/arch/x86/virt/vmx/tdx/seamcall.h b/arch/x86/virt/vmx/tdx/seamcall.h
> > new file mode 100644
> > index 000000000000..54922f7bda3a
> > --- /dev/null
> > +++ b/arch/x86/virt/vmx/tdx/seamcall.h
> > @@ -0,0 +1,71 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (C) 2025 Intel Corporation */
> > +#include <asm/tdx.h>
> 
> If seamcall.h is intended to provide low-level helpers, including
> <asm/tdx.h>, which is meant to offer high-level APIs for other components
> such as KVM, seems a bit odd to me. But I suppose we can live with this.

Kinda agree, I can remove it and do:

struct tdx_module_args;

explicitly.

But we also need to include <asm/archrandom.h> etc, so I think I will just
leave it as-is until other people coming out to complain.