lkml.org 
[lkml]   [2020]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v28 21/22] x86/vdso: Implement a vDSO for Intel SGX enclave call
On Fri, Mar 13, 2020 at 11:48:54AM -0400, Nathaniel McCallum wrote:
> Thinking about this more carefully, I still think that at least part
> of my critique still stands.
>
> __vdso_sgx_enter_enclave() doesn't use the x86-64 ABI. This means that
> there will always be an assembly wrapper for
> __vdso_sgx_enter_enclave(). But because __vdso_sgx_enter_enclave()
> doesn't save %rbx, the wrapper is forced to in order to be called from
> C.
>
> A common pattern for the wrapper will be to do something like this:
>
> # void enter_enclave(rdi, rsi, rdx, unused, r8, r9, @tcs, @e,
> @handler, @leaf, @vdso)
> enter_enclave:
> push %rbx
> push $0 /* align */
> push 0x48(%rsp)
> push 0x48(%rsp)
> push 0x48(%rsp)
>
> mov 0x70(%rsp), %eax
> call *0x68(%rsp)
>
> add $0x20, %rsp
> pop %rbx
> ret
>
> Because __vdso_sgx_enter_enclave() doesn't preserve %rbx, the wrapper
> is forced to reposition stack parameters in a performance-critical
> path. On the other hand, if __vdso_sgx_enter_enclave() preserved %rbx,
> you could implement the above as:
>
> # void enter_enclave(rdi, rsi, rdx, unused, r8, r9, @tcs, @e,
> @handler, @leaf, @vdso)
> enter_enclave:
> mov 0x20(%rsp), %eax
> jmp *0x28(%rsp)
>
> This also implies that if __vdso_sgx_enter_enclave() took @leaf as a
> stack parameter and preserved %rbx, it would be x86-64 ABI compliant
> enough to call from C if the enclave preserves all callee-saved
> registers besides %rbx (Enarx does).
>
> What are the downsides of this approach? It also doesn't harm the more
> extended case when you need to use an assembly wrapper to setup
> additional registers. This can still be done. It does imply an extra
> push and mov instruction. But because there are currently an odd
> number of stack function parameters, the push also removes an
> alignment instruction where the stack is aligned before the call to
> __vdso_sgx_enter_enclave() (likely). Further, the push and mov are
> going to be performed by *someone* in order to call
> __vdso_sgx_enter_enclave() from C.
>
> Therefore, I'd like to propose that __vdso_sgx_enter_enclave():
> * Preserve %rbx.

At first glance, that looks sane. Being able to call __vdso... from C
would certainly be nice.

> * Take the leaf as an additional stack parameter instead of passing
> it in %rax.

Does the leaf even need to be a stack param? Wouldn't it be possible to
use %rcx as @leaf instead of @unusued? E.g.

int __vdso_sgx_enter_enclave(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)
{
push %rbp
mov %rsp, %rbp
push %rbx

mov %ecx, %eax
.Lenter_enclave
cmp $0x2, %eax
jb .Linvalid_leaf
cmp $0x3, %eax
ja .Linvalid_leaf

mov 0x0x10(%rbp), %rbx
lea .Lasync_exit_pointer(%rip), %rcx

.Lasync_exit_pointer:
.Lenclu_eenter_eresume:
enclu

xor %eax, %eax

.Lhandle_exit:
cmp $0, 0x20(%rbp)
jne .Linvoke_userspace_handler

.Lout
pop %rbx
leave
ret
}


> Then C can call it without additional overhead. And people who need to
> "extend" the C ABI can still do so.
>

\
 
 \ /
  Last update: 2020-03-13 17:46    [W:0.168 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site