On 10/7/2019 9:46 PM, Sean Christopherson wrote:
Rework __vdso_sgx_enter_enclave() to prioritize the flow where userspace
is not providing a callback, which is the preferred method of operation.
Processors have branch predictors so your "prioritizing" may not get
what your want.
But if you still insist, a simple ht/hnt prefixing the original branch
instruction would have sufficed. Rearrangement of code blocks is indeed
unnecessary.
A caveat though, for any given process, whether it supplies a callback
or not is usually hard-coded. So either it always takes the callback
path, or it always takes the other. And the branch predictor will do
well in both cases. It's usually unwise to apply ht/hnt prefixes.
Using a callback requires a retpoline, and the only known motivation for
employing a callback is to allow the enclave to muck with the stack of
the untrusted runtime.
Opportunistically replace the majority of the local labels with local
symbol names to improve the readability of the code.
Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
---
arch/x86/entry/vdso/vsgx_enter_enclave.S | 120 ++++++++++++++---------
1 file changed, 71 insertions(+), 49 deletions(-)
diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
index de54e47c83f4..fc5622dcd2fa 100644
--- a/arch/x86/entry/vdso/vsgx_enter_enclave.S
+++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
@@ -85,75 +85,97 @@ ENTRY(__vdso_sgx_enter_enclave)
mov %rsp, %rbp
.cfi_def_cfa_register %rbp
-1: /* EENTER <= leaf <= ERESUME */
+.Lenter_enclave:
+ /* EENTER <= leaf <= ERESUME */
cmp $0x2, %eax
- jb 6f
+ jb .Linvalid_leaf
cmp $0x3, %eax
- ja 6f
+ ja .Linvalid_leaf
/* Load TCS and AEP */
mov 0x10(%rbp), %rbx
- lea 2f(%rip), %rcx
+ lea .Lasync_exit_pointer(%rip), %rcx
/* Single ENCLU serving as both EENTER and AEP (ERESUME) */
-2: enclu
+.Lasync_exit_pointer:
+.Lenclu_eenter_eresume:
+ enclu
- /* EEXIT path */
+ /* EEXIT jumps here unless the enclave is doing something fancy. */
xor %eax, %eax
-3: mov %eax, %ecx
-
- /* Call the exit handler if supplied */
- mov 0x20(%rbp), %rax
- test %rax, %rax
- jz 7f
- /* Align stack per x86_64 ABI. The original %rsp is saved in %rbx to be
- * restored after the exit handler returns. */
+
+ /* Invoke userspace's exit handler if one was provided. */
+.Lhandle_exit:
+ cmp $0, 0x20(%rbp)
+ jne .Linvoke_userspace_handler
+
+.Lout:
+ leave
+ .cfi_def_cfa %rsp, 8
+ ret
+
+.Linvalid_leaf:
Please set frame pointer back to %rbp here, or stack unwinding will fail.
+ mov $(-EINVAL), %eax
+ jmp .Lout
+
+.Lhandle_exception:
+ mov 0x18(%rbp), %rcx
+ test %rcx, %rcx
+ je .Lskip_exception_info
A single "jrcxz .Lskip_exception_info" is equivalent to the above 2
instructions combined.
+
+ /* Fill optional exception info. */
+ mov %eax, EX_LEAF(%rcx)
+ mov %di, EX_TRAPNR(%rcx)
+ mov %si, EX_ERROR_CODE(%rcx)
+ mov %rdx, EX_ADDRESS(%rcx)
+.Lskip_exception_info:
+ mov $(-EFAULT), %eax
+ jmp .Lhandle_exit
+
+.Linvoke_userspace_handler:
+ /*
+ * Align stack per x86_64 ABI. Save the original %rsp in %rbx to be
+ * restored after the callback returns.
+ */
mov %rsp, %rbx
and $-0x10, %rsp
- /* Clear RFLAGS.DF per x86_64 ABI */
- cld
- /* Parameters for the exit handler */
+
+ /* Push @e, u_rsp and @tcs as parameters to the callback. */
push 0x18(%rbp)
push %rbx
push 0x10(%rbp)
- /* Call *%rax via retpoline */
- call 40f
- /* Restore %rsp to its original value left off by the enclave from last
- * exit */
+
+ /* Pass the "return" value to the callback via %rcx. */
+ mov %eax, %ecx
@e (ex_info) is almost always needed by every callback as it also serves
as the "context pointer". The return value on the other hand is
insignificant because it could be deduced from @e->EX_LEAF anyway. So
I'd retain %rcx and push %rax to the stack instead, given the purpose of
this patch is to squeeze out a bit performance.
+
+ /* Clear RFLAGS.DF per x86_64 ABI */
+ cld
+
+ /* Load the callback pointer to %rax and invoke it via retpoline. */
+ mov 0x20(%rbp), %rax
Per X86_64 ABI, %rsp shall be 16 bytes aligned before "call". But %rsp
here doesn't look aligned properly.
+ call .Lretpoline
+
+ /* Restore %rsp to its post-exit value. */
mov %rbx, %rsp
- /* Positive return value from the exit handler will be interpreted as
- * an ENCLU leaf, while a non-positive value will be interpreted as the
- * return value to be passed back to the caller. */
- jmp 1b
-40: /* retpoline */
- call 42f
-41: pause
- lfence
- jmp 41b
-42: mov %rax, (%rsp)
- ret
-5: /* Exception path */
- mov 0x18(%rbp), %rcx
- jrcxz 52f
- mov %eax, EX_LEAF(%rcx)
- mov %di, EX_TRAPNR(%rcx)
- mov %si, EX_ERROR_CODE(%rcx)
- mov %rdx, EX_ADDRESS(%rcx)
-52: mov $-EFAULT, %eax
- jmp 3b
-
-6: /* Unsupported ENCLU leaf */
+ /*
+ * If the return from callback is zero or negative, return immediately,
+ * else re-execute ENCLU with the postive return value interpreted as
+ * the requested ENCLU leaf.
+ */
cmp $0, %eax
- jle 7f
- mov $-EINVAL, %eax
+ jle .Lout
+ jmp .Lenter_enclave
-7: /* Epilog */
- leave
- .cfi_def_cfa %rsp, 8
+.Lretpoline:
+ call 2f
+1: pause
+ lfence
+ jmp 1b
+2: mov %rax, (%rsp)
ret
.cfi_endproc
-_ASM_VDSO_EXTABLE_HANDLE(2b, 5b)
+_ASM_VDSO_EXTABLE_HANDLE(.Lenclu_eenter_eresume, .Lhandle_exception)
ENDPROC(__vdso_sgx_enter_enclave)
