Re: [RFC PATCH 2/4] x86/vdso: x86/sgx: Rework __vdso_sgx_enter_enclave() API

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Mon, Aug 24, 2020 at 03:36:11PM +0200, Jethro Beekman wrote:
> On 2020-08-18 06:24, Sean Christopherson wrote:
> >  /**
> >   * __vdso_sgx_enter_enclave() - Enter an SGX enclave
> > @@ -119,16 +153,14 @@ typedef int (*sgx_enclave_exit_handler_t)(long rdi, long rsi, long rdx,
> >   * @leaf:	ENCLU leaf, must be EENTER or ERESUME
> >   * @r8:		Pass-through value for R8
> >   * @r9:		Pass-through value for R9
> > - * @tcs:	TCS, must be non-NULL
> > - * @e:		Optional struct sgx_enclave_exception instance
> > - * @handler:	Optional enclave exit handler
> > + * @r:		struct sgx_enclave_run, must be non-NULL
> >   *
> >   * NOTE: __vdso_sgx_enter_enclave() does not ensure full compliance with the
> > - * x86-64 ABI, e.g. doesn't explicitly clear EFLAGS.DF after EEXIT.  Except for
> > - * non-volatile general purpose registers, preserving/setting state in
> > - * accordance with the x86-64 ABI is the responsibility of the enclave and its
> > - * runtime, i.e. __vdso_sgx_enter_enclave() cannot be called from C code
> > - * without careful consideration by both the enclave and its runtime.
> > + * x86-64 ABI, e.g. doesn't handle XSAVE state.  Except for non-volatile
> > + * general purpose registers, EFLAGS.DF, and RSP alignment, preserving/setting
> > + * state in accordance with the x86-64 ABI is the responsibility of the enclave
> > + * and its runtime, i.e. __vdso_sgx_enter_enclave() cannot be called from C
> > + * code without careful consideration by both the enclave and its runtime.
> >   *
> >   * All general purpose registers except RAX, RBX and RCX are passed as-is to
> >   * the enclave.  RAX, RBX and RCX are consumed by EENTER and ERESUME and are
> > @@ -160,16 +192,12 @@ typedef int (*sgx_enclave_exit_handler_t)(long rdi, long rsi, long rdx,
> >   * without returning to __vdso_sgx_enter_enclave().
> >   *
> >   * Return:
> > - *  0 on success,
> > + *  0 on success (ENCLU reached),
> >   *  -EINVAL if ENCLU leaf is not allowed,
> > - *  -EFAULT if an exception occurs on ENCLU or within the enclave
> > - *  -errno for all other negative values returned by the userspace exit handler
> >   */
> >  typedef int (*vdso_sgx_enter_enclave_t)(unsigned long rdi, unsigned long rsi,
> >  					unsigned long rdx, unsigned int leaf,
> >  					unsigned long r8,  unsigned long r9,
> > -					void *tcs,
> > -					struct sgx_enclave_exception *e,
> > -					sgx_enclave_exit_handler_t handler);
> > +					struct sgx_enclave_run *r);
> >  
> >  #endif /* _UAPI_ASM_X86_SGX_H */
> > 
> 
> I think this should return void now, not int? Then, the “return”
> section of the documentation is also no longer correct.

This documentation should be moved to Documentation/x86/sgx.rst.

It is easier to read from there and then it will be included by kdoc
to the kernel documentation. In here it is not addressed by kdoc and
it is unnecessarily hard to read.

> --
> Jethro Beekman | Fortanix

/Jarkko