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

[Dave Hansen]

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@xxxxxxxxxxxxxxx>