linux-kernel - Re: [PATCHv4 03/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]

Message-ID: <20220225154650.2yfx3gtyrem3to7v@black.fi.intel.com>
Date:   Fri, 25 Feb 2022 18:46:50 +0300
From:   "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:     Dave Hansen <dave.hansen@...el.com>
Cc:     tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        luto@...nel.org, peterz@...radead.org,
        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: [PATCHv4 03/30] x86/tdx: Provide common base for SEAMCALL and
 TDCALL C wrappers

On Thu, Feb 24, 2022 at 04:41:12PM -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))

Bit 63 also has to be set as it represents error (0 is success with
possible warning).

Bit 62 indicates if the error is recoverable. (0 is recoverable)

Bits 61:48 are reserved and must be 0. For this reason -1UL is not right.

Bits 47:40 are class.

Bits below that are up to grub.

See Table 17.6 of TDX module 1.0 spec.

So we can use 

#define TDX_SW_CLASS(code)		((code) | GENMASK_ULL(40, 47))
#define TDX_ERROR(code)			((code) | (1UL << 63))
#define TDX_SEAMCALL_VMFAILINVALID	TDX_ERROR(TDX_SW_CLASS(0xFFFF0000ULL))

But it looks silly to me. It brings more confusion than solves.

Hm?

> #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)
> 
> ?

Yes, it should work with any code.

-- 
 Kirill A. Shutemov