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

[Kai Huang]

On Thu, 2022-02-24 at 16:41 -0800, Dave Hansen wrote:
> On 2/24/22 15:10, 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_SEAMCALL_VMFAILINVALID     0x8000FF00FFFF0000ULL
> 
> That's OK-ish.  But, it would be nice to make this a bit less magic.
> While I'm sure plenty of us can do the bits 47:40 => hex math in our
> heads, it might be nice to do it with a macro.  Maybe:
> 
> /*
>  * Bits 47:40 being set represent a reserved status class.
>  * The TDX module will never set these so they are safe to
>  * use for software error codes.
>  */
> #define TDX_SW_ERR(code) ((code) | GENMASK_ULL(40, 47))
> 
> #define TDX_SEAMCALL_VMFAILINVALID	TDX_SW_ERR(0xFFFF0000ULL)
> 
> By the way, is the entire "0xFFFF0000ULL" thing up for grabs?  Or do the
> the "0xFFFF...." bits _need_ to be set to represent an error somehow?
> 
> Would this work if it were:
> 
> #define TDX_SEAMCALL_VMFAILINVALID	TDX_SW_ERR(0ULL)
> 
> or
> 
> #define TDX_SEAMCALL_VMFAILINVALID	TDX_SW_ERR(1ULL)
> 
> or
> 
> #define TDX_SEAMCALL_VMFAILINVALID	TDX_SW_ERR(0x12345678ULL)
> 
> ?

Perhaps we can just use -1 (0xFFFFFFFFFFFFFFFFULL) instead of above value for
TDX_SEAMCALL_VMFAILINVALID.

Actually this value will mainly be used when calling P-SEAMLDR's SEAMLDR.INFO
SEAMCALL to detect whether P-SEAMLDR is loaded.  A success of this SEAMCALL
returns the P-SEAMLDR information which further tells whether the TDX module
is loaded or not (please refer to SEAMLDR sepc[1], chapter 4.1 SEAMLDR.INFO).

And P-SEAMLDR actually uses a different error code definition from TDX module
(SEAMLDR spec, chapter 4.4 ERROR HANDLING"):

"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"

It doesn't make a lot sense to use TDX module's error code definition to
define a value that is also supposed to cover P-SEAMLDR, although the chosen
value happens to work.

Instead, -1 works for both, as both error code definitions of P-SEAMLDR and
TDX module have couple of bits reserved and will never be set to 1.

[1]
https://www.intel.com/content/dam/develop/external/us/en/documents-tps/intel-tdx-seamldr-interface-specification.pdf