Re: [RFC PATCH v4 5/5] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions

[Andy Lutomirski]

> On Dec 14, 2018, at 9:03 AM, Sean Christopherson <sean.j.christopherson@xxxxxxxxx> wrote:
> 
>> On Fri, Dec 14, 2018 at 07:38:30AM -0800, Sean Christopherson wrote:
>>> On Fri, Dec 14, 2018 at 07:12:04AM -0800, Sean Christopherson wrote:
>>>> On Fri, Dec 14, 2018 at 09:55:49AM +0000, Jethro Beekman wrote:
>>>>> On 2018-12-14 03:01, Sean Christopherson wrote:
>>>>> +2:    pop    %rbx
>>>>> +    pop    %r12
>>>>> +    pop    %r13
>>>>> +    pop    %r14
>>>>> +    pop    %r15
>>>>> +    pop    %rbp
>>>>> +    ret
>>>> 
>>>> x86-64 ABI requires that you call CLD here (enclave may set it).
>>> 
>>> Ugh.  Technically MXCSR and the x87 CW also need to be preserved.
>>> 
>>> What if rather than treating the enclave as hostile we require it to be
>>> compliant with the x86-64 ABI like any other function?  That would solve
>>> the EFLAGS.DF, MXCSR and x87 issues without adding unnecessary overhead.
>>> And we wouldn't have to save/restore R12-R15.  It'd mean we couldn't use
>>> the stack's red zone to hold @regs and @e, but that's poor form anyways.
>> 
>> Grr, except the processor crushes R12-R15, FCW and MXCSR on asynchronous
>> exits.  But not EFLAGS.DF, that's real helpful.
> 
> I can think of three options that are at least somewhat reasonable:
> 
>  1) Save/restore MXCSR and FCW
> 
>     + 100% compliant with the x86-64 ABI
>     + Callable from any code
>     + Minimal documentation required
>     - Restoring MXCSR/FCW is likely unnecessary 99% of the time
>     - Slow
> 
>  2) Clear EFLAGS.DF but not save/restore MXCSR and FCW
> 
>     + Mostly compliant with the x86-64 ABI
>     + Callable from any code that doesn't use SIMD registers
>     - Need to document deviations from x86-64 ABI
> 
>  3) Require the caller to save/restore everything.
> 
>     + Fast
>     + Userspace can pass all GPRs to the enclave (minus EAX, RBX and RCX)
>     - Completely custom ABI
>     - For all intents and purposes must be called from an assembly wrapper
> 
> Option (3) actually isn't all that awful.  RCX can be used to pass an
> optional pointer to a 'struct sgx_enclave_exception' and we can still
> return standard error codes, e.g. -EFAULT.

I like 3, but:

> 
> E.g.:
> 
> /**
> * __vdso_sgx_enter_enclave() - Enter an SGX enclave
> *
> * %eax:    ENCLU leaf, must be EENTER or ERESUME
> * %rbx:    TCS, must be non-NULL
> * %rcx:    Optional pointer to 'struct sgx_enclave_exception'
> *
> * Return:
> *  0 on a clean entry/exit to/from the enclave
> *  -EINVAL if ENCLU leaf is not allowed or if TCS is NULL
> *  -EFAULT if ENCLU or the enclave faults
> */
> ENTRY(__vdso_sgx_enter_enclave)
>    /* EENTER <= leaf <= ERESUME */
>    cmp    $0x2, %eax
>    jb    bad_input
> 
>    cmp    $0x3, %eax
>    ja    bad_input
> 
>    /* TCS must be non-NULL */
>    test    %rbx, %rbx
>    je    bad_input
> 
>    /* save @exception pointer */
>    push    %rcx
> 
>    /* load leaf, TCS and AEP for ENCLU */
>    lea    1f(%rip),  %rcx
> 1:    enclu
> 
>    add    0x8, %rsp
>    xor    %eax, %eax
>    ret
> 
> bad_input:
>    mov     $(-EINVAL), %rax
>    ret
> 
> .pushsection .fixup, "ax"
> 2:    pop    %rcx    
>    test    %rcx, %rcx
>    je    3f
> 
>    mov    %eax, EX_LEAF(%rcx)
>    mov    %di,  EX_TRAPNR(%rcx)
>    mov    %si,  EX_ERROR_CODE(%rcx)
>    mov    %rdx, EX_ADDRESS(%rcx)
> 3:    mov     $(-EFAULT), %rax
>    ret

Iâm not totally sold on -EFAULT as the error code.  That usually indicates a bad pointer.  Iâm not sure I have a better suggestion.

> .popsection
> 
> _ASM_VDSO_EXTABLE_HANDLE(1b, 3b)
> 
> ENDPROC(__vdso_sgx_enter_enclave)