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

["Huang, Kai" <kai.huang@intel.com>]

> 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