From: Sean Christopherson <[email protected]>
An SGX runtime must be aware of the exceptions, which happen inside an
enclave. Introduce a vDSO call that wraps EENTER/ERESUME cycle and returns
the CPU exception back to the caller exactly when it happens.
Kernel fixups the exception information to RDI, RSI and RDX. The SGX call
vDSO handler fills this information to the user provided buffer or
alternatively trigger user provided callback at the time of the exception.
The calling convention is custom and does not follow System V x86-64 ABI.
Suggested-by: Andy Lutomirski <[email protected]>
Acked-by: Jethro Beekman <[email protected]>
Tested-by: Jethro Beekman <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Co-developed-by: Cedric Xing <[email protected]>
Signed-off-by: Cedric Xing <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
arch/x86/entry/vdso/Makefile | 2 +
arch/x86/entry/vdso/vdso.lds.S | 1 +
arch/x86/entry/vdso/vsgx_enter_enclave.S | 157 +++++++++++++++++++++++
arch/x86/include/asm/enclu.h | 8 ++
arch/x86/include/uapi/asm/sgx.h | 128 ++++++++++++++++++
5 files changed, 296 insertions(+)
create mode 100644 arch/x86/entry/vdso/vsgx_enter_enclave.S
create mode 100644 arch/x86/include/asm/enclu.h
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 3f183d0b8826..416f9432269d 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -29,6 +29,7 @@ VDSO32-$(CONFIG_IA32_EMULATION) := y
vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
vobjs32-y := vdso32/note.o vdso32/system_call.o vdso32/sigreturn.o
vobjs32-y += vdso32/vclock_gettime.o
+vobjs-$(VDSO64-y) += vsgx_enter_enclave.o
# files to link into kernel
obj-y += vma.o extable.o
@@ -100,6 +101,7 @@ $(vobjs): KBUILD_CFLAGS := $(filter-out $(GCC_PLUGINS_CFLAGS) $(RETPOLINE_CFLAGS
CFLAGS_REMOVE_vclock_gettime.o = -pg
CFLAGS_REMOVE_vdso32/vclock_gettime.o = -pg
CFLAGS_REMOVE_vgetcpu.o = -pg
+CFLAGS_REMOVE_vsgx_enter_enclave.o = -pg
#
# X32 processes use x32 vDSO to access 64bit kernel data.
diff --git a/arch/x86/entry/vdso/vdso.lds.S b/arch/x86/entry/vdso/vdso.lds.S
index 36b644e16272..4bf48462fca7 100644
--- a/arch/x86/entry/vdso/vdso.lds.S
+++ b/arch/x86/entry/vdso/vdso.lds.S
@@ -27,6 +27,7 @@ VERSION {
__vdso_time;
clock_getres;
__vdso_clock_getres;
+ __vdso_sgx_enter_enclave;
local: *;
};
}
diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
new file mode 100644
index 000000000000..adbd59d41517
--- /dev/null
+++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
@@ -0,0 +1,157 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/linkage.h>
+#include <asm/export.h>
+#include <asm/errno.h>
+#include <asm/enclu.h>
+
+#include "extable.h"
+
+/* Offset of 'struct sgx_enclave_run' relative to %rbp. */
+#define SGX_ENCLAVE_RUN_PTR 2*8
+
+/* Offsets into 'struct sgx_enclave_run'. */
+#define SGX_ENCLAVE_RUN_TSC 0*8
+#define SGX_ENCLAVE_RUN_FLAGS 1*8
+#define SGX_ENCLAVE_RUN_EXIT_REASON 1*8 + 4
+#define SGX_ENCLAVE_RUN_USER_HANDLER 2*8
+/* #define SGX_ENCLAVE_RUN_USER_DATA 3*8 */
+#define SGX_ENCLAVE_RUN_EXCEPTION 4*8
+
+#define SGX_SYNCHRONOUS_EXIT 0
+#define SGX_EXCEPTION_EXIT 1
+
+/* Offsets into sgx_enter_enclave.exception. */
+#define SGX_EX_LEAF 0*8
+#define SGX_EX_TRAPNR 0*8+4
+#define SGX_EX_ERROR_CODE 0*8+6
+#define SGX_EX_ADDRESS 1*8
+
+.code64
+.section .text, "ax"
+
+SYM_FUNC_START(__vdso_sgx_enter_enclave)
+ /* Prolog */
+ .cfi_startproc
+ push %rbp
+ .cfi_adjust_cfa_offset 8
+ .cfi_rel_offset %rbp, 0
+ mov %rsp, %rbp
+ .cfi_def_cfa_register %rbp
+ push %rbx
+ .cfi_rel_offset %rbx, -8
+
+ mov %ecx, %eax
+.Lenter_enclave:
+ /* EENTER <= leaf <= ERESUME */
+ cmp $EENTER, %eax
+ jb .Linvalid_input
+ cmp $ERESUME, %eax
+ ja .Linvalid_input
+
+ mov SGX_ENCLAVE_RUN_PTR(%rbp), %rcx
+
+ /* No flags are currently defined/supported. */
+ cmpl $0, SGX_ENCLAVE_RUN_FLAGS(%rcx)
+ jne .Linvalid_input
+
+ /* Load TCS and AEP */
+ mov SGX_ENCLAVE_RUN_TSC(%rcx), %rbx
+ lea .Lasync_exit_pointer(%rip), %rcx
+
+ /* Single ENCLU serving as both EENTER and AEP (ERESUME) */
+.Lasync_exit_pointer:
+.Lenclu_eenter_eresume:
+ enclu
+
+ /* EEXIT jumps here unless the enclave is doing something fancy. */
+ mov SGX_ENCLAVE_RUN_PTR(%rbp), %rbx
+
+ /* Set exit_reason. */
+ movl $SGX_SYNCHRONOUS_EXIT, SGX_ENCLAVE_RUN_EXIT_REASON(%rbx)
+
+ /* Invoke userspace's exit handler if one was provided. */
+.Lhandle_exit:
+ cmpq $0, SGX_ENCLAVE_RUN_USER_HANDLER(%rbx)
+ jne .Linvoke_userspace_handler
+
+ /* Success, in the sense that ENCLU was attempted. */
+ xor %eax, %eax
+
+.Lout:
+ pop %rbx
+ leave
+ .cfi_def_cfa %rsp, 8
+ ret
+
+ /* The out-of-line code runs with the pre-leave stack frame. */
+ .cfi_def_cfa %rbp, 16
+
+.Linvalid_input:
+ mov $(-EINVAL), %eax
+ jmp .Lout
+
+.Lhandle_exception:
+ mov SGX_ENCLAVE_RUN_PTR(%rbp), %rbx
+
+ /* Set the exit_reason and exception info. */
+ movl $SGX_EXCEPTION_EXIT, SGX_ENCLAVE_RUN_EXIT_REASON(%rbx)
+
+ mov %eax, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_LEAF)(%rbx)
+ mov %di, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_TRAPNR)(%rbx)
+ mov %si, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_ERROR_CODE)(%rbx)
+ mov %rdx, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_ADDRESS)(%rbx)
+ jmp .Lhandle_exit
+
+.Linvoke_userspace_handler:
+ /* Pass the untrusted RSP (at exit) to the callback via %rcx. */
+ mov %rsp, %rcx
+
+ /* Save @e, %rbx is about to be clobbered. */
+ mov %rbx, %rax
+
+ /* Save the untrusted RSP offset in %rbx (non-volatile register). */
+ mov %rsp, %rbx
+ and $0xf, %rbx
+
+ /*
+ * Align stack per x86_64 ABI. Note, %rsp needs to be 16-byte aligned
+ * _after_ pushing the parameters on the stack, hence the bonus push.
+ */
+ and $-0x10, %rsp
+ push %rax
+
+ /* Push @e as a param to the callback. */
+ push %rax
+
+ /* Clear RFLAGS.DF per x86_64 ABI */
+ cld
+
+ /* Load the callback pointer to %rax and invoke it via retpoline. */
+ mov SGX_ENCLAVE_RUN_USER_HANDLER(%rax), %rax
+ call .Lretpoline
+
+ /* Undo the post-exit %rsp adjustment. */
+ lea 0x10(%rsp, %rbx), %rsp
+
+ /*
+ * 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 .Lout
+ jmp .Lenter_enclave
+
+.Lretpoline:
+ call 2f
+1: pause
+ lfence
+ jmp 1b
+2: mov %rax, (%rsp)
+ ret
+ .cfi_endproc
+
+_ASM_VDSO_EXTABLE_HANDLE(.Lenclu_eenter_eresume, .Lhandle_exception)
+
+SYM_FUNC_END(__vdso_sgx_enter_enclave)
diff --git a/arch/x86/include/asm/enclu.h b/arch/x86/include/asm/enclu.h
new file mode 100644
index 000000000000..06157b3e9ede
--- /dev/null
+++ b/arch/x86/include/asm/enclu.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_ENCLU_H
+#define _ASM_X86_ENCLU_H
+
+#define EENTER 0x02
+#define ERESUME 0x03
+
+#endif /* _ASM_X86_ENCLU_H */
diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index d0916fb9629e..1564d7f88597 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -72,4 +72,132 @@ struct sgx_enclave_provision {
__u64 attribute_fd;
};
+#define SGX_SYNCHRONOUS_EXIT 0
+#define SGX_EXCEPTION_EXIT 1
+
+struct sgx_enclave_run;
+
+/**
+ * typedef sgx_enclave_exit_handler_t - Exit handler function accepted by
+ * __vdso_sgx_enter_enclave()
+ *
+ * @rdi: RDI at the time of enclave exit
+ * @rsi: RSI at the time of enclave exit
+ * @rdx: RDX at the time of enclave exit
+ * @ursp: RSP at the time of enclave exit (untrusted stack)
+ * @r8: R8 at the time of enclave exit
+ * @r9: R9 at the time of enclave exit
+ * @r: Pointer to struct sgx_enclave_run (as provided by caller)
+ *
+ * Return:
+ * 0 or negative to exit vDSO
+ * positive to re-enter enclave (must be EENTER or ERESUME leaf)
+ */
+typedef int (*sgx_enclave_exit_handler_t)(long rdi, long rsi, long rdx,
+ long ursp, long r8, long r9,
+ struct sgx_enclave_run *r);
+
+/**
+ * struct sgx_enclave_exception - structure to report exceptions encountered in
+ * __vdso_sgx_enter_enclave()
+ *
+ * @leaf: ENCLU leaf from \%eax at time of exception
+ * @trapnr: exception trap number, a.k.a. fault vector
+ * @error_code: exception error code
+ * @address: exception address, e.g. CR2 on a #PF
+ */
+struct sgx_enclave_exception {
+ __u32 leaf;
+ __u16 trapnr;
+ __u16 error_code;
+ __u64 address;
+};
+
+/**
+ * struct sgx_enclave_run - Control structure for __vdso_sgx_enter_enclave()
+ *
+ * @tcs: Thread Control Structure used to enter enclave
+ * @flags: Control flags
+ * @exit_reason: Cause of exit from enclave, e.g. EEXIT vs. exception
+ * @user_handler: User provided exit handler (optional)
+ * @user_data: User provided opaque value (optional)
+ * @exception: Valid on exit due to exception
+ */
+struct sgx_enclave_run {
+ __u64 tcs;
+ __u32 flags;
+ __u32 exit_reason;
+
+ union {
+ sgx_enclave_exit_handler_t user_handler;
+ __u64 __user_handler;
+ };
+ __u64 user_data;
+
+ union {
+ struct sgx_enclave_exception exception;
+
+ /* Pad the entire struct to 256 bytes. */
+ __u8 pad[256 - 32];
+ };
+};
+
+/**
+ * typedef vdso_sgx_enter_enclave_t - Prototype for __vdso_sgx_enter_enclave(),
+ * a vDSO function to enter an SGX enclave.
+ *
+ * @rdi: Pass-through value for RDI
+ * @rsi: Pass-through value for RSI
+ * @rdx: Pass-through value for RDX
+ * @leaf: ENCLU leaf, must be EENTER or ERESUME
+ * @r8: Pass-through value for R8
+ * @r9: Pass-through value for R9
+ * @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 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
+ * loaded with @leaf, asynchronous exit pointer, and @tcs respectively.
+ *
+ * RBP and the stack are used to anchor __vdso_sgx_enter_enclave() to the
+ * pre-enclave state, e.g. to retrieve @e and @handler after an enclave exit.
+ * All other registers are available for use by the enclave and its runtime,
+ * e.g. an enclave can push additional data onto the stack (and modify RSP) to
+ * pass information to the optional exit handler (see below).
+ *
+ * Most exceptions reported on ENCLU, including those that occur within the
+ * enclave, are fixed up and reported synchronously instead of being delivered
+ * via a standard signal. Debug Exceptions (#DB) and Breakpoints (#BP) are
+ * never fixed up and are always delivered via standard signals. On synchrously
+ * reported exceptions, -EFAULT is returned and details about the exception are
+ * recorded in @e, the optional sgx_enclave_exception struct.
+ *
+ * If an exit handler is provided, the handler will be invoked on synchronous
+ * exits from the enclave and for all synchronously reported exceptions. In
+ * latter case, @e is filled prior to invoking the handler.
+ *
+ * The exit handler's return value is interpreted as follows:
+ * >0: continue, restart __vdso_sgx_enter_enclave() with @ret as @leaf
+ * 0: success, return @ret to the caller
+ * <0: error, return @ret to the caller
+ *
+ * The exit handler may transfer control, e.g. via longjmp() or C++ exception,
+ * without returning to __vdso_sgx_enter_enclave().
+ *
+ * Return:
+ * 0 on success (ENCLU reached),
+ * -EINVAL if ENCLU leaf is not allowed,
+ * -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,
+ struct sgx_enclave_run *r);
+
#endif /* _UAPI_ASM_X86_SGX_H */
--
2.25.1
On Tue, Sep 15, 2020 at 02:28:39PM +0300, Jarkko Sakkinen wrote:
> diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> new file mode 100644
> index 000000000000..adbd59d41517
> --- /dev/null
> +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
Why not simply
arch/x86/entry/vdso/sgx.S
?
> @@ -0,0 +1,157 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <linux/linkage.h>
> +#include <asm/export.h>
> +#include <asm/errno.h>
> +#include <asm/enclu.h>
> +
> +#include "extable.h"
> +
> +/* Offset of 'struct sgx_enclave_run' relative to %rbp. */
> +#define SGX_ENCLAVE_RUN_PTR 2*8
> +
> +/* Offsets into 'struct sgx_enclave_run'. */
> +#define SGX_ENCLAVE_RUN_TSC 0*8
> +#define SGX_ENCLAVE_RUN_FLAGS 1*8
> +#define SGX_ENCLAVE_RUN_EXIT_REASON 1*8 + 4
> +#define SGX_ENCLAVE_RUN_USER_HANDLER 2*8
> +/* #define SGX_ENCLAVE_RUN_USER_DATA 3*8 */
What's with that guy? Left here as documentation showing what's at 3*8
offset?
> +#define SGX_ENCLAVE_RUN_EXCEPTION 4*8
> +
> +#define SGX_SYNCHRONOUS_EXIT 0
> +#define SGX_EXCEPTION_EXIT 1
Those are in ...uapi/asm/sgx.h too. Unify?
> +
> +/* Offsets into sgx_enter_enclave.exception. */
> +#define SGX_EX_LEAF 0*8
> +#define SGX_EX_TRAPNR 0*8+4
> +#define SGX_EX_ERROR_CODE 0*8+6
> +#define SGX_EX_ADDRESS 1*8
> +
> +.code64
> +.section .text, "ax"
> +
> +SYM_FUNC_START(__vdso_sgx_enter_enclave)
> + /* Prolog */
> + .cfi_startproc
Are the CFI annotations for userspace?
> + push %rbp
> + .cfi_adjust_cfa_offset 8
> + .cfi_rel_offset %rbp, 0
> + mov %rsp, %rbp
> + .cfi_def_cfa_register %rbp
> + push %rbx
> + .cfi_rel_offset %rbx, -8
> +
> + mov %ecx, %eax
> +.Lenter_enclave:
> + /* EENTER <= leaf <= ERESUME */
> + cmp $EENTER, %eax
> + jb .Linvalid_input
> + cmp $ERESUME, %eax
> + ja .Linvalid_input
> +
> + mov SGX_ENCLAVE_RUN_PTR(%rbp), %rcx
> +
> + /* No flags are currently defined/supported. */
> + cmpl $0, SGX_ENCLAVE_RUN_FLAGS(%rcx)
> + jne .Linvalid_input
> +
> + /* Load TCS and AEP */
TSC
I can see why you would write "TCS" though - there's a thread control
structure thing too in that patch.
> + mov SGX_ENCLAVE_RUN_TSC(%rcx), %rbx
> + lea .Lasync_exit_pointer(%rip), %rcx
> +
> + /* Single ENCLU serving as both EENTER and AEP (ERESUME) */
> +.Lasync_exit_pointer:
> +.Lenclu_eenter_eresume:
> + enclu
> +
> + /* EEXIT jumps here unless the enclave is doing something fancy. */
> + mov SGX_ENCLAVE_RUN_PTR(%rbp), %rbx
> +
> + /* Set exit_reason. */
> + movl $SGX_SYNCHRONOUS_EXIT, SGX_ENCLAVE_RUN_EXIT_REASON(%rbx)
> +
> + /* Invoke userspace's exit handler if one was provided. */
> +.Lhandle_exit:
> + cmpq $0, SGX_ENCLAVE_RUN_USER_HANDLER(%rbx)
> + jne .Linvoke_userspace_handler
> +
> + /* Success, in the sense that ENCLU was attempted. */
> + xor %eax, %eax
> +
> +.Lout:
> + pop %rbx
> + leave
> + .cfi_def_cfa %rsp, 8
> + ret
> +
> + /* The out-of-line code runs with the pre-leave stack frame. */
> + .cfi_def_cfa %rbp, 16
> +
> +.Linvalid_input:
> + mov $(-EINVAL), %eax
> + jmp .Lout
> +
> +.Lhandle_exception:
> + mov SGX_ENCLAVE_RUN_PTR(%rbp), %rbx
> +
> + /* Set the exit_reason and exception info. */
> + movl $SGX_EXCEPTION_EXIT, SGX_ENCLAVE_RUN_EXIT_REASON(%rbx)
> +
> + mov %eax, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_LEAF)(%rbx)
> + mov %di, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_TRAPNR)(%rbx)
> + mov %si, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_ERROR_CODE)(%rbx)
> + mov %rdx, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_ADDRESS)(%rbx)
> + jmp .Lhandle_exit
> +
> +.Linvoke_userspace_handler:
> + /* Pass the untrusted RSP (at exit) to the callback via %rcx. */
> + mov %rsp, %rcx
> +
> + /* Save @e, %rbx is about to be clobbered. */
I've been wondering what that "@e" thing is and I believe I've found it at the
end: "... @e, the optional sgx_enclave_exception struct"
Yes?
Please rewrite what that "e" is here - I think you wanna say "struct
sgx_enclave_run.exception" instead to clarify.
...
> diff --git a/arch/x86/include/asm/enclu.h b/arch/x86/include/asm/enclu.h
> new file mode 100644
> index 000000000000..06157b3e9ede
> --- /dev/null
> +++ b/arch/x86/include/asm/enclu.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_ENCLU_H
> +#define _ASM_X86_ENCLU_H
> +
> +#define EENTER 0x02
> +#define ERESUME 0x03
> +
> +#endif /* _ASM_X86_ENCLU_H */
> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> index d0916fb9629e..1564d7f88597 100644
> --- a/arch/x86/include/uapi/asm/sgx.h
> +++ b/arch/x86/include/uapi/asm/sgx.h
Are all those defines being added to the uapi header, also actually
needed by userspace?
> @@ -72,4 +72,132 @@ struct sgx_enclave_provision {
> __u64 attribute_fd;
> };
>
> +#define SGX_SYNCHRONOUS_EXIT 0
> +#define SGX_EXCEPTION_EXIT 1
> +
> +struct sgx_enclave_run;
> +
> +/**
> + * typedef sgx_enclave_exit_handler_t - Exit handler function accepted by
> + * __vdso_sgx_enter_enclave()
> + *
> + * @rdi: RDI at the time of enclave exit
> + * @rsi: RSI at the time of enclave exit
> + * @rdx: RDX at the time of enclave exit
> + * @ursp: RSP at the time of enclave exit (untrusted stack)
> + * @r8: R8 at the time of enclave exit
> + * @r9: R9 at the time of enclave exit
I'm sure you can avoid that "at the time of enclave exit" repetition.
...
> +/**
> + * typedef vdso_sgx_enter_enclave_t - Prototype for __vdso_sgx_enter_enclave(),
> + * a vDSO function to enter an SGX enclave.
> + *
> + * @rdi: Pass-through value for RDI
> + * @rsi: Pass-through value for RSI
> + * @rdx: Pass-through value for RDX
> + * @leaf: ENCLU leaf, must be EENTER or ERESUME
> + * @r8: Pass-through value for R8
> + * @r9: Pass-through value for R9
> + * @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 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
> + * loaded with @leaf, asynchronous exit pointer, and @tcs respectively.
That tcs is probably struct sgx_enclave_run.tcs?
> + * RBP and the stack are used to anchor __vdso_sgx_enter_enclave() to the
> + * pre-enclave state, e.g. to retrieve @e and @handler after an enclave exit.
Expand @e and do you mean "@user_handler" with "@handler"?
> + * All other registers are available for use by the enclave and its runtime,
> + * e.g. an enclave can push additional data onto the stack (and modify RSP) to
> + * pass information to the optional exit handler (see below).
> + *
> + * Most exceptions reported on ENCLU, including those that occur within the
> + * enclave, are fixed up and reported synchronously instead of being delivered
> + * via a standard signal. Debug Exceptions (#DB) and Breakpoints (#BP) are
> + * never fixed up and are always delivered via standard signals. On synchrously
> + * reported exceptions, -EFAULT is returned and details about the exception are
> + * recorded in @e, the optional sgx_enclave_exception struct.
> + *
> + * If an exit handler is provided, the handler will be invoked on synchronous
> + * exits from the enclave and for all synchronously reported exceptions. In
> + * latter case, @e is filled prior to invoking the handler.
> + *
> + * The exit handler's return value is interpreted as follows:
> + * >0: continue, restart __vdso_sgx_enter_enclave() with @ret as @leaf
> + * 0: success, return @ret to the caller
> + * <0: error, return @ret to the caller
> + *
> + * The exit handler may transfer control, e.g. via longjmp() or C++ exception,
> + * without returning to __vdso_sgx_enter_enclave().
> + *
> + * Return:
> + * 0 on success (ENCLU reached),
> + * -EINVAL if ENCLU leaf is not allowed,
> + * -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,
> + struct sgx_enclave_run *r);
> +
> #endif /* _UAPI_ASM_X86_SGX_H */
> --
> 2.25.1
>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Thu, Sep 24, 2020 at 08:04:07PM +0200, Borislav Petkov wrote:
> On Tue, Sep 15, 2020 at 02:28:39PM +0300, Jarkko Sakkinen wrote:
> > diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > new file mode 100644
> > index 000000000000..adbd59d41517
> > --- /dev/null
> > +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
>
> Why not simply
>
> arch/x86/entry/vdso/sgx.S
>
> ?
I renamed it as vsgx.S (for the sake of convention).
> > @@ -0,0 +1,157 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#include <linux/linkage.h>
> > +#include <asm/export.h>
> > +#include <asm/errno.h>
> > +#include <asm/enclu.h>
> > +
> > +#include "extable.h"
> > +
> > +/* Offset of 'struct sgx_enclave_run' relative to %rbp. */
> > +#define SGX_ENCLAVE_RUN_PTR 2*8
> > +
> > +/* Offsets into 'struct sgx_enclave_run'. */
> > +#define SGX_ENCLAVE_RUN_TSC 0*8
> > +#define SGX_ENCLAVE_RUN_FLAGS 1*8
> > +#define SGX_ENCLAVE_RUN_EXIT_REASON 1*8 + 4
> > +#define SGX_ENCLAVE_RUN_USER_HANDLER 2*8
> > +/* #define SGX_ENCLAVE_RUN_USER_DATA 3*8 */
>
> What's with that guy? Left here as documentation showing what's at 3*8
> offset?
Yes.
> > +#define SGX_ENCLAVE_RUN_EXCEPTION 4*8
> > +
> > +#define SGX_SYNCHRONOUS_EXIT 0
> > +#define SGX_EXCEPTION_EXIT 1
>
> Those are in ...uapi/asm/sgx.h too. Unify?
I have not authored this patch but what I would propose is to use just
raw value in the place of these constants. It is practially just a
boolean value.
I can also add sgx_vdso.h to uapi directory. I just don't see the point.
Holding on for feedback with this one.
> > +
> > +/* Offsets into sgx_enter_enclave.exception. */
> > +#define SGX_EX_LEAF 0*8
> > +#define SGX_EX_TRAPNR 0*8+4
> > +#define SGX_EX_ERROR_CODE 0*8+6
> > +#define SGX_EX_ADDRESS 1*8
> > +
> > +.code64
> > +.section .text, "ax"
> > +
> > +SYM_FUNC_START(__vdso_sgx_enter_enclave)
> > + /* Prolog */
> > + .cfi_startproc
>
> Are the CFI annotations for userspace?
Yes, for stack unwinding. However, I would leave them out. Holding on
for further feedback with this tho.
> > + push %rbp
> > + .cfi_adjust_cfa_offset 8
> > + .cfi_rel_offset %rbp, 0
> > + mov %rsp, %rbp
> > + .cfi_def_cfa_register %rbp
> > + push %rbx
> > + .cfi_rel_offset %rbx, -8
> > +
> > + mov %ecx, %eax
> > +.Lenter_enclave:
> > + /* EENTER <= leaf <= ERESUME */
> > + cmp $EENTER, %eax
> > + jb .Linvalid_input
> > + cmp $ERESUME, %eax
> > + ja .Linvalid_input
> > +
> > + mov SGX_ENCLAVE_RUN_PTR(%rbp), %rcx
> > +
> > + /* No flags are currently defined/supported. */
> > + cmpl $0, SGX_ENCLAVE_RUN_FLAGS(%rcx)
> > + jne .Linvalid_input
> > +
> > + /* Load TCS and AEP */
>
> TSC
>
> I can see why you would write "TCS" though - there's a thread control
> structure thing too in that patch.
Renamed.
> > + mov SGX_ENCLAVE_RUN_TSC(%rcx), %rbx
> > + lea .Lasync_exit_pointer(%rip), %rcx
> > +
> > + /* Single ENCLU serving as both EENTER and AEP (ERESUME) */
> > +.Lasync_exit_pointer:
> > +.Lenclu_eenter_eresume:
> > + enclu
> > +
> > + /* EEXIT jumps here unless the enclave is doing something fancy. */
> > + mov SGX_ENCLAVE_RUN_PTR(%rbp), %rbx
> > +
> > + /* Set exit_reason. */
> > + movl $SGX_SYNCHRONOUS_EXIT, SGX_ENCLAVE_RUN_EXIT_REASON(%rbx)
> > +
> > + /* Invoke userspace's exit handler if one was provided. */
> > +.Lhandle_exit:
> > + cmpq $0, SGX_ENCLAVE_RUN_USER_HANDLER(%rbx)
> > + jne .Linvoke_userspace_handler
> > +
> > + /* Success, in the sense that ENCLU was attempted. */
> > + xor %eax, %eax
> > +
> > +.Lout:
> > + pop %rbx
> > + leave
> > + .cfi_def_cfa %rsp, 8
> > + ret
> > +
> > + /* The out-of-line code runs with the pre-leave stack frame. */
> > + .cfi_def_cfa %rbp, 16
> > +
> > +.Linvalid_input:
> > + mov $(-EINVAL), %eax
> > + jmp .Lout
> > +
> > +.Lhandle_exception:
> > + mov SGX_ENCLAVE_RUN_PTR(%rbp), %rbx
> > +
> > + /* Set the exit_reason and exception info. */
> > + movl $SGX_EXCEPTION_EXIT, SGX_ENCLAVE_RUN_EXIT_REASON(%rbx)
> > +
> > + mov %eax, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_LEAF)(%rbx)
> > + mov %di, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_TRAPNR)(%rbx)
> > + mov %si, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_ERROR_CODE)(%rbx)
> > + mov %rdx, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_ADDRESS)(%rbx)
> > + jmp .Lhandle_exit
> > +
> > +.Linvoke_userspace_handler:
> > + /* Pass the untrusted RSP (at exit) to the callback via %rcx. */
> > + mov %rsp, %rcx
> > +
> > + /* Save @e, %rbx is about to be clobbered. */
>
> I've been wondering what that "@e" thing is and I believe I've found it at the
> end: "... @e, the optional sgx_enclave_exception struct"
>
> Yes?
>
> Please rewrite what that "e" is here - I think you wanna say "struct
> sgx_enclave_run.exception" instead to clarify.
I agree. Open coded them as "struct sgx_enclave_exception".
>
> ...
>
> > diff --git a/arch/x86/include/asm/enclu.h b/arch/x86/include/asm/enclu.h
> > new file mode 100644
> > index 000000000000..06157b3e9ede
> > --- /dev/null
> > +++ b/arch/x86/include/asm/enclu.h
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _ASM_X86_ENCLU_H
> > +#define _ASM_X86_ENCLU_H
> > +
> > +#define EENTER 0x02
> > +#define ERESUME 0x03
> > +
> > +#endif /* _ASM_X86_ENCLU_H */
> > diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> > index d0916fb9629e..1564d7f88597 100644
> > --- a/arch/x86/include/uapi/asm/sgx.h
> > +++ b/arch/x86/include/uapi/asm/sgx.h
>
> Are all those defines being added to the uapi header, also actually
> needed by userspace?
I'd remove the two constants, as it is clearly just a boolean value.
> > @@ -72,4 +72,132 @@ struct sgx_enclave_provision {
> > __u64 attribute_fd;
> > };
> >
> > +#define SGX_SYNCHRONOUS_EXIT 0
> > +#define SGX_EXCEPTION_EXIT 1
> > +
> > +struct sgx_enclave_run;
> > +
> > +/**
> > + * typedef sgx_enclave_exit_handler_t - Exit handler function accepted by
> > + * __vdso_sgx_enter_enclave()
> > + *
> > + * @rdi: RDI at the time of enclave exit
> > + * @rsi: RSI at the time of enclave exit
> > + * @rdx: RDX at the time of enclave exit
> > + * @ursp: RSP at the time of enclave exit (untrusted stack)
> > + * @r8: R8 at the time of enclave exit
> > + * @r9: R9 at the time of enclave exit
>
> I'm sure you can avoid that "at the time of enclave exit" repetition.
I edited this as
/**
* typedef sgx_enclave_exit_handler_t - Exit handler function accepted by
* __vdso_sgx_enter_enclave()
* @rdi: RDI snapshot
* @rsi: RSI snapshot
* @rdx: RDX snapshot
* @rsp: RSP snapshot (untrusted stack)
* @r8: R8 snapshot
* @r9: R9 snapshot
* @run: Pointer to the caller provided struct sgx_enclave_run
*
* Return:
* 0 or negative to exit vDSO
* positive to re-enter enclave (must be EENTER or ERESUME leaf)
*/
typedef int (*sgx_enclave_exit_handler_t)(long rdi, long rsi, long rdx,
long rsp, long r8, long r9,
struct sgx_enclave_run *run);
> > +/**
> > + * typedef vdso_sgx_enter_enclave_t - Prototype for __vdso_sgx_enter_enclave(),
> > + * a vDSO function to enter an SGX enclave.
> > + *
> > + * @rdi: Pass-through value for RDI
> > + * @rsi: Pass-through value for RSI
> > + * @rdx: Pass-through value for RDX
> > + * @leaf: ENCLU leaf, must be EENTER or ERESUME
> > + * @r8: Pass-through value for R8
> > + * @r9: Pass-through value for R9
> > + * @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 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
> > + * loaded with @leaf, asynchronous exit pointer, and @tcs respectively.
>
> That tcs is probably struct sgx_enclave_run.tcs?
Yes.
> > + * RBP and the stack are used to anchor __vdso_sgx_enter_enclave() to the
> > + * pre-enclave state, e.g. to retrieve @e and @handler after an enclave exit.
>
> Expand @e and do you mean "@user_handler" with "@handler"?
I replaced this and similar fields as @run.user_handler and so forth.
>
> > + * All other registers are available for use by the enclave and its runtime,
> > + * e.g. an enclave can push additional data onto the stack (and modify RSP) to
> > + * pass information to the optional exit handler (see below).
> > + *
> > + * Most exceptions reported on ENCLU, including those that occur within the
> > + * enclave, are fixed up and reported synchronously instead of being delivered
> > + * via a standard signal. Debug Exceptions (#DB) and Breakpoints (#BP) are
> > + * never fixed up and are always delivered via standard signals. On synchrously
> > + * reported exceptions, -EFAULT is returned and details about the exception are
> > + * recorded in @e, the optional sgx_enclave_exception struct.
> > + *
> > + * If an exit handler is provided, the handler will be invoked on synchronous
> > + * exits from the enclave and for all synchronously reported exceptions. In
> > + * latter case, @e is filled prior to invoking the handler.
> > + *
> > + * The exit handler's return value is interpreted as follows:
> > + * >0: continue, restart __vdso_sgx_enter_enclave() with @ret as @leaf
> > + * 0: success, return @ret to the caller
> > + * <0: error, return @ret to the caller
> > + *
> > + * The exit handler may transfer control, e.g. via longjmp() or C++ exception,
> > + * without returning to __vdso_sgx_enter_enclave().
> > + *
> > + * Return:
> > + * 0 on success (ENCLU reached),
> > + * -EINVAL if ENCLU leaf is not allowed,
> > + * -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,
> > + struct sgx_enclave_run *r);
> > +
> > #endif /* _UAPI_ASM_X86_SGX_H */
> > --
> > 2.25.1
> >
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
Also, I renamed 'r' as 'run' in some places.
End result:
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-sgx.git/tree/arch/x86/include/uapi/asm/sgx.h
I'm wondering this sentence:
"The calling convention is custom and does not follow System V x86-64 ABI."
AFAIK, now the vDSO is fully C-callable but waiting for feedback before
removing it.
/Jarkko
On Fri, Sep 25, 2020 at 04:00:40AM +0300, Jarkko Sakkinen wrote:
> I renamed it as vsgx.S (for the sake of convention).
Right.
> I have not authored this patch but what I would propose is to use just
> raw value in the place of these constants. It is practially just a
> boolean value.
>
> I can also add sgx_vdso.h to uapi directory. I just don't see the point.
Just be very cautious what you add to the uapi/ directory because it
becomes API and there's no changing it. That's why I point you guys to
it, to think hard what you expose there and that it becomes contract
with luserspace.
> > I can see why you would write "TCS" though - there's a thread control
> > structure thing too in that patch.
>
> Renamed.
See Sean's reply.
> /**
> * typedef sgx_enclave_exit_handler_t - Exit handler function accepted by
> * __vdso_sgx_enter_enclave()
> * @rdi: RDI snapshot
> * @rsi: RSI snapshot
> * @rdx: RDX snapshot
> * @rsp: RSP snapshot (untrusted stack)
> * @r8: R8 snapshot
> * @r9: R9 snapshot
I'd say here:
"The registers' content is the snapshot made at enclave exit."
> Also, I renamed 'r' as 'run' in some places.
>
> End result:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-sgx.git/tree/arch/x86/include/uapi/asm/sgx.h
>
> I'm wondering this sentence:
>
> "The calling convention is custom and does not follow System V x86-64 ABI."
Yeah, I was wondering what that meant too.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 2020-09-25 03:00, Jarkko Sakkinen wrote:
> End result:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-sgx.git/tree/arch/x86/include/uapi/asm/sgx.h
>
> I'm wondering this sentence:
>
> "The calling convention is custom and does not follow System V x86-64 ABI."
>
> AFAIK, now the vDSO is fully C-callable but waiting for feedback before
> removing it.
It's C-callable *iff your enclave follows the System V x86-64 ABI*. In addition, all registers not clobbered by the SGX ISA are passed to the enclave, not just those specified as parameter passing registers in the ABI. This is intentional to make the vDSO function usable in applications that use the current flexibility of ENCLU.
--
Jethro Beekman | Fortanix
On Fri, Sep 25, 2020 at 10:39:58AM +0200, Jethro Beekman wrote:
> On 2020-09-25 03:00, Jarkko Sakkinen wrote:
> > End result:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-sgx.git/tree/arch/x86/include/uapi/asm/sgx.h
> >
> > I'm wondering this sentence:
> >
> > "The calling convention is custom and does not follow System V x86-64 ABI."
> >
> > AFAIK, now the vDSO is fully C-callable but waiting for feedback before
> > removing it.
>
> It's C-callable *iff your enclave follows the System V x86-64 ABI*. In
> addition, all registers not clobbered by the SGX ISA are passed to the
> enclave, not just those specified as parameter passing registers in
> the ABI. This is intentional to make the vDSO function usable in
> applications that use the current flexibility of ENCLU.
Hold on, I really want to fix this bit of documentation before sending
any new version, so I'll explain how I understand it.
I think it is just SystemV ABI call with six parameters in the usual
GPRs (rdi, rsi, rdx, rcx, r8, r9).
I'm looking at vdso_sgx_enter_enclave.
What I'm not getting?
> --
> Jethro Beekman | Fortanix
/Jarkko
On 2020-09-25 13:17, Jarkko Sakkinen wrote:
> On Fri, Sep 25, 2020 at 10:39:58AM +0200, Jethro Beekman wrote:
>> On 2020-09-25 03:00, Jarkko Sakkinen wrote:
>>> End result:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-sgx.git/tree/arch/x86/include/uapi/asm/sgx.h
>>>
>>> I'm wondering this sentence:
>>>
>>> "The calling convention is custom and does not follow System V x86-64 ABI."
>>>
>>> AFAIK, now the vDSO is fully C-callable but waiting for feedback before
>>> removing it.
>>
>> It's C-callable *iff your enclave follows the System V x86-64 ABI*. In
>> addition, all registers not clobbered by the SGX ISA are passed to the
>> enclave, not just those specified as parameter passing registers in
>> the ABI. This is intentional to make the vDSO function usable in
>> applications that use the current flexibility of ENCLU.
>
> Hold on, I really want to fix this bit of documentation before sending
> any new version, so I'll explain how I understand it.
>
> I think it is just SystemV ABI call with six parameters in the usual
> GPRs (rdi, rsi, rdx, rcx, r8, r9).
>
> I'm looking at vdso_sgx_enter_enclave.
>
> What I'm not getting?
This can't be observed from looking at the code in vdso_sgx_enter_enclave, because what makes this "custom" is the absence of code or code in the enclave.
If you call vdso_sgx_enter_enclave() from C but your enclave doesn't respect the ABI (for example, it clobbers callee-saved registers), your code will break. But if you call vdso_sgx_enter_enclave from assembly, you can use enclaves with any ABI as long as your assembly and the enclave agree on that ABI.
Alternatively, when using assembly, I can pass stuff to the enclave in registers besides rdi, rsi, rdx, rcx, r8, r9, just as if I were manually calling ENCLU from assembly.
The vDSO assembly has been carefully written to support both scenarios by only using rax, rbx, rcx, rbp, rsp and passing rdi, rsi, rdx, r8, r9 (and everything else).
> + * NOTE: __vdso_sgx_enter_enclave() does not ensure full compliance with the
> + * 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
> + * loaded with @leaf, asynchronous exit pointer, and @tcs respectively.
Perhaps this should be updated to read "All general purpose registers except RAX, RBX, RCX, RBP and RSP ..."
--
Jethro Beekman | Fortanix
On Fri, Sep 25, 2020 at 10:28:07AM +0200, Borislav Petkov wrote:
> > > I can see why you would write "TCS" though - there's a thread control
> > > structure thing too in that patch.
> >
> > Renamed.
>
> See Sean's reply.
I did not get Sean's reply, and neither can find it from lore:
https://lore.kernel.org/linux-sgx/[email protected]/T/#t
> > * typedef sgx_enclave_exit_handler_t - Exit handler function accepted by
> > * __vdso_sgx_enter_enclave()
> > * @rdi: RDI snapshot
> > * @rsi: RSI snapshot
> > * @rdx: RDX snapshot
> > * @rsp: RSP snapshot (untrusted stack)
> > * @r8: R8 snapshot
> > * @r9: R9 snapshot
>
> I'd say here:
>
> "The registers' content is the snapshot made at enclave exit."
I'd make that a description and take away individual parameter
descriptions. Is that fine?
> > Also, I renamed 'r' as 'run' in some places.
> >
> > End result:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-sgx.git/tree/arch/x86/include/uapi/asm/sgx.h
> >
> > I'm wondering this sentence:
> >
> > "The calling convention is custom and does not follow System V x86-64 ABI."
>
> Yeah, I was wondering what that meant too.
I'll refine that one based on my own and Jethro's feedback.
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
/Jarkko
On Mon, Sep 28, 2020 at 02:37:00AM +0300, Jarkko Sakkinen wrote:
> I did not get Sean's reply, and neither can find it from lore:
>
> https://lore.kernel.org/linux-sgx/[email protected]/T/#t
Yah, your mail server upgrade broke a lot of stuff. And lore even says
it is not there:
2020-09-25 11:43 ` Jethro Beekman
[not found] ` <[email protected]> <---
2020-09-25 1:04 ` Jarkko Sakkinen
Lemme bounce it to you.
> I'd make that a description and take away individual parameter
> descriptions. Is that fine?
Sure.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Sep 28, 2020 at 10:30:32AM +0200, Borislav Petkov wrote:
> On Mon, Sep 28, 2020 at 02:37:00AM +0300, Jarkko Sakkinen wrote:
> > I did not get Sean's reply, and neither can find it from lore:
> >
> > https://lore.kernel.org/linux-sgx/[email protected]/T/#t
>
> Yah, your mail server upgrade broke a lot of stuff. And lore even says
> it is not there:
>
> 2020-09-25 11:43 ` Jethro Beekman
> [not found] ` <[email protected]> <---
> 2020-09-25 1:04 ` Jarkko Sakkinen
>
> Lemme bounce it to you.
Thank you. I think I have it correctly in my tree. And I actually
noticed that I had the original email stored in wrong archive folder on
my machine (sorry about that), so did I receive it after all, but it
does not exist in lore.
> > I'd make that a description and take away individual parameter
> > descriptions. Is that fine?
>
> Sure.
/**
* typedef sgx_enclave_exit_handler_t - Exit handler function accepted by
* __vdso_sgx_enter_enclave()
* @run: Pointer to the caller provided struct sgx_enclave_run
*
* The register parameters contain the snapshot of their values at enclave
* exit
*
* Return:
* 0 or negative to exit vDSO
* positive to re-enter enclave (must be EENTER or ERESUME leaf)
*/
typedef int (*sgx_enclave_exit_handler_t)(long rdi, long rsi, long rdx,
long rsp, long r8, long r9,
struct sgx_enclave_run *run);
I think this looks reasonable now.
Another minor clean up I made is:
struct sgx_enclave_run {
__u64 tcs;
__u32 flags;
__u32 exit_reason;
__u64 user_handler;
__u64 user_data;
I.e. got rid of the "user_handler union. Makes the struc less confusing
looking and is consistent with the other structs.
I've been thinking about this tail:
union {
struct sgx_enclave_exception exception;
/* Pad the entire struct to 256 bytes. */
__u8 pad[256 - 32];
};
};
I'd just replace this with
__u64 exception;
};
And do something like (just writing it to the email to show the idea,
have not even compiled this):
- mov %eax, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_LEAF)(%rbx)
- mov %di, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_TRAPNR)(%rbx)
- mov %si, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_ERROR_CODE)(%rbx)
- mov %rdx, (SGX_ENCLAVE_RUN_EXCEPTION + SGX_EX_ADDRESS)(%rbx)
+ mov SGX_ENCLAVE_RUN_EXCEPTION(%rbx), %rbx
+
+ mov %eax, (SGX_EX_LEAF)(%rbx)
+ mov %di, (SGX_EX_TRAPNR)(%rbx)
+ mov %si, (SGX_EX_ERROR_CODE)(%rbx)
+ mov %rdx, (SGX_EX_ADDRESS)(%rbx)
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
/Jarkko