Re: [PATCH v19 13/27] x86/sgx: Add wrappers for ENCLS leaf functions

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Thu, Mar 21, 2019 at 04:51:53PM +0200, Jarkko Sakkinen wrote:
> On Tue, Mar 19, 2019 at 12:59:07PM -0700, Sean Christopherson wrote:
> > On Sun, Mar 17, 2019 at 11:14:42PM +0200, Jarkko Sakkinen wrote:
> > > +/**
> > > + * ENCLS_FAULT_FLAG - flag signifying an ENCLS return code is a trapnr
> > > + *
> > > + * ENCLS has its own (positive value) error codes and also generates
> > > + * ENCLS specific #GP and #PF faults.  And the ENCLS values get munged
> > > + * with system error codes as everything percolates back up the stack.
> > > + * Unfortunately (for us), we need to precisely identify each unique
> > > + * error code, e.g. the action taken if EWB fails varies based on the
> > > + * type of fault and on the exact SGX error code, i.e. we can't simply
> > > + * convert all faults to -EFAULT.
> > > + *
> > > + * To make all three error types coexist, we set bit 30 to identify an
> > > + * ENCLS fault.  Bit 31 (technically bits N:31) is used to differentiate
> > > + * between positive (faults and SGX error codes) and negative (system
> > > + * error codes) values.
> > > + */
> > > +#define ENCLS_FAULT_FLAG 0x40000000
> > > +
> > > +/**
> > > + * Retrieve the encoded trapnr from the specified return code.
> > > + */
> > > +#define ENCLS_TRAPNR(r) ((r) & ~ENCLS_FAULT_FLAG)
> > 
> > I honestly can't remember, is there a reason ENCLS_TRAPNR is still a macro
> > and not an inline function?
> 
> Not at all. And also the value should be unsigned and fault flag
> should be 0x80* because we never should get negative values.

From ENCLS itself, but we do have scenarios where the return code can
hold a system error code (negative value), an SGX error code (positive
value) or fault inforation.  E.g. sgx_encl_init() sets ret to -ERESTARTSYS,
and __sgx_encl_ewb() returns system error codes if get_backing() fails
and also directly returns the result of __ewb().  We could dance around
those issues, but IMO it's too fragile as it essentially requires a full
audit of every possible error path when modifying code.