diff mbox series

[v14,06/23] x86/virt/tdx: Add SEAMCALL error printing for module initialization

Message ID 58c44258cb5b1009f0ddfe6b07ac986b9614b8b3.1697532085.git.kai.huang@intel.com (mailing list archive)
State New, archived
Headers show
Series TDX host kernel support | expand

Commit Message

Huang, Kai Oct. 17, 2023, 10:14 a.m. UTC 
The SEAMCALLs involved during the TDX module initialization are not
expected to fail.  In fact, they are not expected to return any non-zero
code (except the "running out of entropy error", which can be handled
internally already).

Add yet another set of SEAMCALL wrappers, which treats all non-zero
return code as error, to support printing SEAMCALL error upon failure
for module initialization.  Note the TDX module initialization doesn't
use the _saved_ret() variant thus no wrapper is added for it.

SEAMCALL assembly can also return kernel-defined error codes for three
special cases: 1) TDX isn't enabled by the BIOS; 2) TDX module isn't
loaded; 3) CPU isn't in VMX operation.  Whether they can legally happen
depends on the caller, so leave to the caller to print error message
when desired.

Also convert the SEAMCALL error codes to the kernel error codes in the
new wrappers so that each SEAMCALL caller doesn't have to repeat the
conversion.

Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---

v13 -> v14:
 - Use real functions to replace macros. (Dave)
 - Moved printing error message for special error code to the caller
   (internal)
 - Added Kirill's tag

v12 -> v13:
 - New implementation due to TDCALL assembly series.

---
 arch/x86/include/asm/tdx.h  |  1 +
 arch/x86/virt/vmx/tdx/tdx.c | 52 +++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

Comments

Kuppuswamy Sathyanarayanan Oct. 17, 2023, 1:37 p.m. UTC | #1 
On 10/17/2023 3:14 AM, Kai Huang wrote:
> The SEAMCALLs involved during the TDX module initialization are not
> expected to fail.  In fact, they are not expected to return any non-zero
> code (except the "running out of entropy error", which can be handled
> internally already).
> 
> Add yet another set of SEAMCALL wrappers, which treats all non-zero
> return code as error, to support printing SEAMCALL error upon failure
> for module initialization.  Note the TDX module initialization doesn't
> use the _saved_ret() variant thus no wrapper is added for it.
> 
> SEAMCALL assembly can also return kernel-defined error codes for three
> special cases: 1) TDX isn't enabled by the BIOS; 2) TDX module isn't
> loaded; 3) CPU isn't in VMX operation.  Whether they can legally happen
> depends on the caller, so leave to the caller to print error message
> when desired.
> 
> Also convert the SEAMCALL error codes to the kernel error codes in the
> new wrappers so that each SEAMCALL caller doesn't have to repeat the
> conversion.
> 
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
> 
> v13 -> v14:
>  - Use real functions to replace macros. (Dave)
>  - Moved printing error message for special error code to the caller
>    (internal)
>  - Added Kirill's tag
> 
> v12 -> v13:
>  - New implementation due to TDCALL assembly series.
> 
> ---
>  arch/x86/include/asm/tdx.h  |  1 +
>  arch/x86/virt/vmx/tdx/tdx.c | 52 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index d624aa25aab0..984efd3114ed 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -27,6 +27,7 @@
>  /*
>   * TDX module SEAMCALL leaf function error codes
>   */
> +#define TDX_SUCCESS		0ULL
>  #define TDX_RND_NO_ENTROPY	0x8000020300000000ULL
>  
>  #ifndef __ASSEMBLY__
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 13d22ea2e2d9..52fb14e0195f 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -20,6 +20,58 @@ static u32 tdx_global_keyid __ro_after_init;
>  static u32 tdx_guest_keyid_start __ro_after_init;
>  static u32 tdx_nr_guest_keyids __ro_after_init;
>  
> +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%llx) failed: 0x%llx\n", fn, err);
> +}
> +

Why pass args here?

> +static inline void seamcall_err_ret(u64 fn, u64 err,
> +				    struct tdx_module_args *args)
> +{
> +	seamcall_err(fn, err, args);
> +	pr_err("RCX 0x%llx RDX 0x%llx R8 0x%llx R9 0x%llx R10 0x%llx R11 0x%llx\n",
> +			args->rcx, args->rdx, args->r8, args->r9,
> +			args->r10, args->r11);
> +}
> +
> +static inline void seamcall_err_saved_ret(u64 fn, u64 err,
> +					  struct tdx_module_args *args)
> +{
> +	seamcall_err_ret(fn, err, args);
> +	pr_err("RBX 0x%llx RDI 0x%llx RSI 0x%llx R12 0x%llx R13 0x%llx R14 0x%llx R15 0x%llx\n",
> +			args->rbx, args->rdi, args->rsi, args->r12,
> +			args->r13, args->r14, args->r15);
> +}
> +
> +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))
> +
>  static int __init record_keyid_partitioning(u32 *tdx_keyid_start,
>  					    u32 *nr_tdx_keyids)
>  {
Huang, Kai Oct. 18, 2023, 6:27 a.m. UTC | #2 
> >  
> > +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%llx) failed: 0x%llx\n", fn, err);
> > +}
> > +
> 
> Why pass args here?

It needs to be sc_err_func_t so that it can be used by the common code
sc_retry_prerr() below.

> 
> > +static inline void seamcall_err_ret(u64 fn, u64 err,
> > +				    struct tdx_module_args *args)
> > +{
> > +	seamcall_err(fn, err, args);
> > +	pr_err("RCX 0x%llx RDX 0x%llx R8 0x%llx R9 0x%llx R10 0x%llx R11 0x%llx\n",
> > +			args->rcx, args->rdx, args->r8, args->r9,
> > +			args->r10, args->r11);
> > +}
> > +
> > +static inline void seamcall_err_saved_ret(u64 fn, u64 err,
> > +					  struct tdx_module_args *args)
> > +{
> > +	seamcall_err_ret(fn, err, args);
> > +	pr_err("RBX 0x%llx RDI 0x%llx RSI 0x%llx R12 0x%llx R13 0x%llx R14 0x%llx R15 0x%llx\n",
> > +			args->rbx, args->rdi, args->rsi, args->r12,
> > +			args->r13, args->r14, args->r15);
> > +}
> > +
> > +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))
> > +
> >  static int __init record_keyid_partitioning(u32 *tdx_keyid_start,
> >  					    u32 *nr_tdx_keyids)
> >  {
>
Nikolay Borisov Oct. 18, 2023, 7:40 a.m. UTC | #3 
On 17.10.23 г. 13:14 ч., Kai Huang wrote:
> The SEAMCALLs involved during the TDX module initialization are not
> expected to fail.  In fact, they are not expected to return any non-zero
> code (except the "running out of entropy error", which can be handled
> internally already).
> 
> Add yet another set of SEAMCALL wrappers, which treats all non-zero
> return code as error, to support printing SEAMCALL error upon failure
> for module initialization.  Note the TDX module initialization doesn't
> use the _saved_ret() variant thus no wrapper is added for it.
> 
> SEAMCALL assembly can also return kernel-defined error codes for three
> special cases: 1) TDX isn't enabled by the BIOS; 2) TDX module isn't
> loaded; 3) CPU isn't in VMX operation.  Whether they can legally happen
> depends on the caller, so leave to the caller to print error message
> when desired.
> 
> Also convert the SEAMCALL error codes to the kernel error codes in the
> new wrappers so that each SEAMCALL caller doesn't have to repeat the
> conversion.
> 
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
> 
> v13 -> v14:
>   - Use real functions to replace macros. (Dave)
>   - Moved printing error message for special error code to the caller
>     (internal)
>   - Added Kirill's tag
> 
> v12 -> v13:
>   - New implementation due to TDCALL assembly series.
> 
> ---
>   arch/x86/include/asm/tdx.h  |  1 +
>   arch/x86/virt/vmx/tdx/tdx.c | 52 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 53 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index d624aa25aab0..984efd3114ed 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -27,6 +27,7 @@
>   /*
>    * TDX module SEAMCALL leaf function error codes
>    */
> +#define TDX_SUCCESS		0ULL
>   #define TDX_RND_NO_ENTROPY	0x8000020300000000ULL
>   
>   #ifndef __ASSEMBLY__
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 13d22ea2e2d9..52fb14e0195f 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -20,6 +20,58 @@ static u32 tdx_global_keyid __ro_after_init;
>   static u32 tdx_guest_keyid_start __ro_after_init;
>   static u32 tdx_nr_guest_keyids __ro_after_init;
>   
> +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%llx) failed: 0x%llx\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%llx RDX 0x%llx R8 0x%llx R9 0x%llx R10 0x%llx R11 0x%llx\n",
> +			args->rcx, args->rdx, args->r8, args->r9,
> +			args->r10, args->r11);
> +}
> +
> +static inline void seamcall_err_saved_ret(u64 fn, u64 err,
> +					  struct tdx_module_args *args)

This function remains unused throughout the whole series, remove it and 
add it later when it's actually going to be useful.

<snip>
Huang, Kai Oct. 18, 2023, 8:26 a.m. UTC | #4 
> > +
> > +static inline void seamcall_err_saved_ret(u64 fn, u64 err,
> > +					  struct tdx_module_args *args)
> 
> This function remains unused throughout the whole series, remove it and 
> add it later when it's actually going to be useful.
> 
> <snip>

OK fine to me.  Thanks.
Kuppuswamy Sathyanarayanan Oct. 18, 2023, 2:17 p.m. UTC | #5 
On 10/17/2023 3:14 AM, Kai Huang wrote:
> The SEAMCALLs involved during the TDX module initialization are not
> expected to fail.  In fact, they are not expected to return any non-zero
> code (except the "running out of entropy error", which can be handled
> internally already).
> 
> Add yet another set of SEAMCALL wrappers, which treats all non-zero
> return code as error, to support printing SEAMCALL error upon failure
> for module initialization.  Note the TDX module initialization doesn't
> use the _saved_ret() variant thus no wrapper is added for it.
> 
> SEAMCALL assembly can also return kernel-defined error codes for three
> special cases: 1) TDX isn't enabled by the BIOS; 2) TDX module isn't
> loaded; 3) CPU isn't in VMX operation.  Whether they can legally happen
> depends on the caller, so leave to the caller to print error message
> when desired.
> 
> Also convert the SEAMCALL error codes to the kernel error codes in the
> new wrappers so that each SEAMCALL caller doesn't have to repeat the
> conversion.
> 
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
> 
> v13 -> v14:
>  - Use real functions to replace macros. (Dave)
>  - Moved printing error message for special error code to the caller
>    (internal)
>  - Added Kirill's tag
> 
> v12 -> v13:
>  - New implementation due to TDCALL assembly series.
> 
> ---

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
diff mbox series

Patch

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index d624aa25aab0..984efd3114ed 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -27,6 +27,7 @@ 
 /*
  * TDX module SEAMCALL leaf function error codes
  */
+#define TDX_SUCCESS		0ULL
 #define TDX_RND_NO_ENTROPY	0x8000020300000000ULL
 
 #ifndef __ASSEMBLY__
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 13d22ea2e2d9..52fb14e0195f 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -20,6 +20,58 @@  static u32 tdx_global_keyid __ro_after_init;
 static u32 tdx_guest_keyid_start __ro_after_init;
 static u32 tdx_nr_guest_keyids __ro_after_init;
 
+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%llx) failed: 0x%llx\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%llx RDX 0x%llx R8 0x%llx R9 0x%llx R10 0x%llx R11 0x%llx\n",
+			args->rcx, args->rdx, args->r8, args->r9,
+			args->r10, args->r11);
+}
+
+static inline void seamcall_err_saved_ret(u64 fn, u64 err,
+					  struct tdx_module_args *args)
+{
+	seamcall_err_ret(fn, err, args);
+	pr_err("RBX 0x%llx RDI 0x%llx RSI 0x%llx R12 0x%llx R13 0x%llx R14 0x%llx R15 0x%llx\n",
+			args->rbx, args->rdi, args->rsi, args->r12,
+			args->r13, args->r14, args->r15);
+}
+
+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))
+
 static int __init record_keyid_partitioning(u32 *tdx_keyid_start,
 					    u32 *nr_tdx_keyids)
 {