linux-kernel - Re: [PATCHv5 02/30] x86/tdx: Provide common base for SEAMCALL and TDCALL C wrappers

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]

Date:   Tue, 8 Mar 2022 11:56:52 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        luto@...nel.org, peterz@...radead.org
Cc:     sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
        ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
        hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
        joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
        pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        thomas.lendacky@....com, brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCHv5 02/30] x86/tdx: Provide common base for SEAMCALL and
 TDCALL C wrappers

On 3/2/22 06:27, Kirill A. Shutemov wrote:
> Secure Arbitration Mode (SEAM) is an extension of VMX architecture.  It
> defines a new VMX root operation (SEAM VMX root) and a new VMX non-root
> operation (SEAM VMX non-root) which are both isolated from the legacy
> VMX operation where the host kernel runs.
> 
> A CPU-attested software module (called 'TDX module') runs in SEAM VMX
> root to manage and protect VMs running in SEAM VMX non-root.  SEAM VMX
> root is also used to host another CPU-attested software module (called
> 'P-SEAMLDR') to load and update the TDX module.
> 
> Host kernel transits to either P-SEAMLDR or TDX module via the new
> SEAMCALL instruction, which is essentially a VMExit from VMX root mode
> to SEAM VMX root mode.  SEAMCALLs are leaf functions defined by
> P-SEAMLDR and TDX module around the new SEAMCALL instruction.
> 
> A guest kernel can also communicate with TDX module via TDCALL
> instruction.
> 
> TDCALLs and SEAMCALLs use an ABI different from the x86-64 system-v ABI.
> RAX is used to carry both the SEAMCALL leaf function number (input) and
> the completion status (output).  Additional GPRs (RCX, RDX, R8-R11) may
> be further used as both input and output operands in individual leaf.
> 
> TDCALL and SEAMCALL share the same ABI and require the largely same
> code to pass down arguments and retrieve results.
> 
> Define an assembly macro that can be used to implement C wrapper for
> both TDCALL and SEAMCALL.

It's probably also worth mentioning that the SEAMCALL half won't get
used in this series.

> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index ba8042ce61c2..e5ff8ed59adf 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -8,6 +8,33 @@
>  #define TDX_CPUID_LEAF_ID	0x21
>  #define TDX_IDENT		"IntelTDX    "
>  
> +/*
> + * SW-defined error codes.
> + *
> + * Bits 47:40 == 0xFF indicate Reserved status code class that never used by
> + * TDX module.
That's a bit clunky.  Perhaps replace it with this:

 * Bits 47:40 == 0xFF indicate a "Reserved" status code class that is
   never used by the TDX module.

> + */
> +#define TDX_ERROR			(1UL << 63)
> +#define TDX_SW_ERROR			(TDX_ERROR | GENMASK_ULL(40, 47))
> +#define TDX_SEAMCALL_VMFAILINVALID	(TDX_SW_ERROR | 0xFFFF0000ULL)
> +
> +#ifndef __ASSEMBLY__

The "UL" construct doesn't work in the assembler.  But, this won't shoIf
you use _BITUL, it will do the hard work for you.

> +/*
> + * Used to gather the output registers values of the TDCALL and SEAMCALL
> + * instructions when requesting services from the TDX module.
> + *
> + * This is a software only structure and not part of the TDX module/VMM ABI.
> + */
> +struct tdx_module_output {
> +	u64 rcx;
> +	u64 rdx;
> +	u64 r8;
> +	u64 r9;
> +	u64 r10;
> +	u64 r11;
> +};

With those fixed:

Reviewed-by: Dave Hansen <dave.hansen@...ux.intel.com>