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

From: Kai Huang
Date: Sun Apr 03 2022 - 23:19:49 EST


On Mon, 2022-03-21 at 19:02 +0300, Kirill A. Shutemov wrote:
> +/*
> + * SW-defined error codes.
> + *
> + * Bits 47:40 == 0xFF indicate Reserved status code class that never used by
> + * TDX module.
> + */
> +#define TDX_ERROR _BITUL(63)
> +#define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(40, 47))
> +#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000))

Hi Kirill,

GENMASK_ULL(40, 47) should be GENMASK_ULL(47, 40), otherwise I am getting error
while building TDX host code:

arch/x86/virt/vmx/tdx/tdx.c: In function ‘seamcall’:
./include/linux/build_bug.h:16:51: error: negative width in bit-field
‘<anonymous>’
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))

Btw, perhaps you should also explicitly include <linux/bits.h> where
GENMASK_ULL() is defined in this header file.

Btw, I previous suggested perhaps we can just use -1ULL instead of above value
for TDX_SEAMCALL_VMFAILINVALID, but didn't get response. The reason is this
value will only be used when detecting P-SEAMLDR using P-SEAMLDR's SEAMLDR.INFO
SEAMCALL. Note your above SW-defined error codes is based on error code
definition for TDX module, but actually P-SEAMLDR has different error code
definition:

"The Intel P-SEAMLDR module returns error codes in the format
0x80000000_cccceeee, where the value cccc specifies the error class, and the
value eeee specifies the error code within that class"

The above value happens to work for P-SEAMLDR but it doesn't follow P-SEAMLDR's
error code definition.

More information:
https://lore.kernel.org/lkml/20220224155630.52734-1-kirill.shutemov@xxxxxxxxxxxxxxx/T/#m5242e88e58a52a7e1da69dde3b63f19a717d3118


--
Thanks,
-Kai