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

[Dave Hansen]

On 4/4/22 17:23, Kai Huang wrote:
>> The vague "some reserved bits" magic works for exactly *one* error code.
>>  If you ever want to make it two, then you subject some some poor future
>> dope to a trip to TDX documentation purgatory.
>>
>> All I'm asking is that this code stand on its own.  Just declare what
>> the *ACTUAL* reserved bits are.  Not some vague reference to "some
>> bits".  There are *REAL* documented bits.  Use them, please.
> OK.  How about below:
> 
> 	/*
> 	 * VMfailInvalid happens before any actual internal logic of the
> 	 * P-SEAMLDR and the TDX module is reached.  Use a unique value which
> 	 * will never conflict with any actual error code returned by both the
> 	 * P-SEAMLDR and the TDX module to represent this case.
> 	 *
> 	 * Both error code definitions of the P-SEAMLDR and the TDX module have
> 	 * some reserved bits (bits 61:48 for the TDX module and bits 62:32 for
> 	 * the P-SEAMLDR) which will never be set to 1.  Choose -1ULL as it
> will
> 	 * never be returned as error code by both of them.
> 	 */
> 	#define TDX_SEAMCALL_VMFAILINVALID	GENMASK_ULL(63, 0)

Kai, I think I've failed horribly in explaining myself.

Here's what I want:

#define TDX_ERROR_RSVD_P_SEAMLDR	GENMASK_ULL(62, 32)
#define TDX_ERROR_RSVD_MODULE		GENMASK_ULL(61, 48)

The P-SEAMLDR and the TDX module itself each define reserved bits in
their error codes.  These bits will never be set after a SEAMCALL to
those two entities.  Define a error code which can be returned by
software which uses those reserved bits and can be used for either the
P-SEAMLDR or the TDX module:

#define	TDX_SEAMCALL_VMFAILINVALID	(TDX_ERROR_RSVD_P_SEAMLDR &
					 TDX_ERROR_RSVD_MODULE)

See what that does?  It does what you said in english, but does it in
code.  Take two things from the specs (the reserved masks), and find a
value that satisfies both entities.

Now, I'm not sure why you *need* TDX_SEAMCALL_VMFAILINVALID, but this is
how I'd like it to be defined.  Sure, -1 *works*, but it's basically an
opaque value.  It's also not *obviously* correct.