linux-kernel - Re: [PATCH v14 06/23] x86/virt/tdx: Add SEAMCALL error printing for module initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <28569cea-4291-4d2c-9662-da19a6f53308@linux.intel.com>
Date:   Tue, 17 Oct 2023 06:37:50 -0700
From:   Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Kai Huang <kai.huang@...el.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     x86@...nel.org, dave.hansen@...el.com,
        kirill.shutemov@...ux.intel.com, peterz@...radead.org,
        tony.luck@...el.com, tglx@...utronix.de, bp@...en8.de,
        mingo@...hat.com, hpa@...or.com, seanjc@...gle.com,
        pbonzini@...hat.com, rafael@...nel.org, david@...hat.com,
        dan.j.williams@...el.com, len.brown@...el.com, ak@...ux.intel.com,
        isaku.yamahata@...el.com, ying.huang@...el.com, chao.gao@...el.com,
        nik.borisov@...e.com, bagasdotme@...il.com, sagis@...gle.com,
        imammedo@...hat.com
Subject: Re: [PATCH v14 06/23] x86/virt/tdx: Add SEAMCALL error printing for
 module initialization



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@...el.com>
> Reviewed-by: Kirill A. Shutemov <kirill.shutemov@...ux.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)
>  {

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer