* [PATCH for_v29 v2 1/5] x86/sgx: vdso: Make __vdso_sgx_enter_enclave() callable from C code
2020-03-30 18:08 [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C Sean Christopherson
@ 2020-03-30 18:08 ` Sean Christopherson
2020-03-30 21:04 ` Jarkko Sakkinen
2020-03-30 18:08 ` [PATCH for_v29 v2 2/5] x86/sgx: vdso: Define a typedef for __vdso_sgx_enter_enclave Sean Christopherson
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Sean Christopherson @ 2020-03-30 18:08 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
Make __vdso_sgx_enter_enclave() callable from C by preserving %rbx
and taking @leaf in %rcx instead of %rax. Being able to invoke the vDSO
from C reduces the overhead of runtimes that are tightly coupled with
their enclaves, e.g. that can rely on the enclave to save and restore
non-volatile registers, as the runtime doesn't need an assembly wrapper
to preserve non-volatile registers and/or shuffle stack arguments.
Note, both %rcx and %rbx are consumed by EENTER/ERESUME, i.e. consuming
them doesn't violate the primary tenet of __vdso_sgx_enter_enclave()
that "thou shalt not restrict how information is exchanged between an
enclave and its host process".
Suggested-by: Nathaniel McCallum <npmccallum@redhat.com>
Cc: Cedric Xing <cedric.xing@intel.com>
Cc: Jethro Beekman <jethro@fortanix.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: linux-sgx@vger.kernel.org
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
arch/x86/entry/vdso/vsgx_enter_enclave.S | 30 ++++++++++++++----------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
index 34cee2b0ef09..c56064fb36bc 100644
--- a/arch/x86/entry/vdso/vsgx_enter_enclave.S
+++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
@@ -17,22 +17,22 @@
/**
* __vdso_sgx_enter_enclave() - 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
* @tcs: TCS, must be non-NULL
* @e: Optional struct sgx_enclave_exception instance
* @handler: Optional enclave exit handler
*
- * **Important!** __vdso_sgx_enter_enclave() is **NOT** compliant with the
- * x86-64 ABI, i.e. cannot be called from standard C code.
- *
- * Input ABI:
- * @leaf %eax
- * @tcs 8(%rsp)
- * @e 0x10(%rsp)
- * @handler 0x18(%rsp)
- *
- * Output ABI:
- * @ret %eax
+ * **Important!** __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.
*
* 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
@@ -71,7 +71,9 @@
*/
#ifdef SGX_KERNEL_DOC
/* C-style function prototype to coerce kernel-doc into parsing the comment. */
-int __vdso_sgx_enter_enclave(int leaf, void *tcs,
+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);
#endif
@@ -83,7 +85,10 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
.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
@@ -109,6 +114,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
jne .Linvoke_userspace_handler
.Lout:
+ pop %rbx
leave
.cfi_def_cfa %rsp, 8
ret
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 1/5] x86/sgx: vdso: Make __vdso_sgx_enter_enclave() callable from C code
2020-03-30 18:08 ` [PATCH for_v29 v2 1/5] x86/sgx: vdso: Make __vdso_sgx_enter_enclave() callable from C code Sean Christopherson
@ 2020-03-30 21:04 ` Jarkko Sakkinen
2020-04-17 15:05 ` Sean Christopherson
0 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-03-30 21:04 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
On Mon, Mar 30, 2020 at 11:08:07AM -0700, Sean Christopherson wrote:
> Make __vdso_sgx_enter_enclave() callable from C by preserving %rbx
> and taking @leaf in %rcx instead of %rax. Being able to invoke the vDSO
> from C reduces the overhead of runtimes that are tightly coupled with
> their enclaves, e.g. that can rely on the enclave to save and restore
> non-volatile registers, as the runtime doesn't need an assembly wrapper
> to preserve non-volatile registers and/or shuffle stack arguments.
>
> Note, both %rcx and %rbx are consumed by EENTER/ERESUME, i.e. consuming
> them doesn't violate the primary tenet of __vdso_sgx_enter_enclave()
> that "thou shalt not restrict how information is exchanged between an
> enclave and its host process".
>
> Suggested-by: Nathaniel McCallum <npmccallum@redhat.com>
> Cc: Cedric Xing <cedric.xing@intel.com>
> Cc: Jethro Beekman <jethro@fortanix.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: linux-sgx@vger.kernel.org
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
> arch/x86/entry/vdso/vsgx_enter_enclave.S | 30 ++++++++++++++----------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> index 34cee2b0ef09..c56064fb36bc 100644
> --- a/arch/x86/entry/vdso/vsgx_enter_enclave.S
> +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> @@ -17,22 +17,22 @@
>
> /**
> * __vdso_sgx_enter_enclave() - 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
> * @tcs: TCS, must be non-NULL
> * @e: Optional struct sgx_enclave_exception instance
> * @handler: Optional enclave exit handler
> *
> - * **Important!** __vdso_sgx_enter_enclave() is **NOT** compliant with the
> - * x86-64 ABI, i.e. cannot be called from standard C code.
> - *
> - * Input ABI:
> - * @leaf %eax
> - * @tcs 8(%rsp)
> - * @e 0x10(%rsp)
> - * @handler 0x18(%rsp)
> - *
> - * Output ABI:
> - * @ret %eax
> + * **Important!** __vdso_sgx_enter_enclave() does not ensure full compliance
I'd simply put **NOTE** here instead of **Important!** as it is more
common.
> + * 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.
Instead "e.g. doesn't explcitly clear EFLAGS.DF after EEXIT" (which is
somewhat confusing statement) paragraph should be replaced with a simple
enumerated list of differences.
Something might be left out but that's cool. Just do your best and it
can refined over time to be more exact.
> *
> * 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
> @@ -71,7 +71,9 @@
> */
> #ifdef SGX_KERNEL_DOC
> /* C-style function prototype to coerce kernel-doc into parsing the comment. */
> -int __vdso_sgx_enter_enclave(int leaf, void *tcs,
> +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);
> #endif
> @@ -83,7 +85,10 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
> .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
> @@ -109,6 +114,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
> jne .Linvoke_userspace_handler
>
> .Lout:
> + pop %rbx
> leave
> .cfi_def_cfa %rsp, 8
> ret
> --
> 2.24.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 1/5] x86/sgx: vdso: Make __vdso_sgx_enter_enclave() callable from C code
2020-03-30 21:04 ` Jarkko Sakkinen
@ 2020-04-17 15:05 ` Sean Christopherson
2020-04-17 18:57 ` Jarkko Sakkinen
0 siblings, 1 reply; 21+ messages in thread
From: Sean Christopherson @ 2020-04-17 15:05 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
On Tue, Mar 31, 2020 at 12:04:46AM +0300, Jarkko Sakkinen wrote:
> On Mon, Mar 30, 2020 at 11:08:07AM -0700, Sean Christopherson wrote:
> > Make __vdso_sgx_enter_enclave() callable from C by preserving %rbx
> > and taking @leaf in %rcx instead of %rax. Being able to invoke the vDSO
> > from C reduces the overhead of runtimes that are tightly coupled with
> > their enclaves, e.g. that can rely on the enclave to save and restore
> > non-volatile registers, as the runtime doesn't need an assembly wrapper
> > to preserve non-volatile registers and/or shuffle stack arguments.
> >
> > Note, both %rcx and %rbx are consumed by EENTER/ERESUME, i.e. consuming
> > them doesn't violate the primary tenet of __vdso_sgx_enter_enclave()
> > that "thou shalt not restrict how information is exchanged between an
> > enclave and its host process".
> >
> > Suggested-by: Nathaniel McCallum <npmccallum@redhat.com>
> > Cc: Cedric Xing <cedric.xing@intel.com>
> > Cc: Jethro Beekman <jethro@fortanix.com>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: linux-sgx@vger.kernel.org
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> > arch/x86/entry/vdso/vsgx_enter_enclave.S | 30 ++++++++++++++----------
> > 1 file changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > index 34cee2b0ef09..c56064fb36bc 100644
> > --- a/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > @@ -17,22 +17,22 @@
> >
> > /**
> > * __vdso_sgx_enter_enclave() - 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
> > * @tcs: TCS, must be non-NULL
> > * @e: Optional struct sgx_enclave_exception instance
> > * @handler: Optional enclave exit handler
> > *
> > - * **Important!** __vdso_sgx_enter_enclave() is **NOT** compliant with the
> > - * x86-64 ABI, i.e. cannot be called from standard C code.
> > - *
> > - * Input ABI:
> > - * @leaf %eax
> > - * @tcs 8(%rsp)
> > - * @e 0x10(%rsp)
> > - * @handler 0x18(%rsp)
> > - *
> > - * Output ABI:
> > - * @ret %eax
> > + * **Important!** __vdso_sgx_enter_enclave() does not ensure full compliance
>
> I'd simply put **NOTE** here instead of **Important!** as it is more
> common.
>
> > + * 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.
>
> Instead "e.g. doesn't explcitly clear EFLAGS.DF after EEXIT" (which is
> somewhat confusing statement) paragraph should be replaced with a simple
> enumerated list of differences.
I don't think the list is that simple, e.g. there is a lot of state that
is defined by the ABI that isn't touched, IMO talking about that state will
add confusion.
I also don't understand what's confusing about stating EFLAGS.DF isn't
cleared.
> Something might be left out but that's cool. Just do your best and it
> can refined over time to be more exact.
>
> > *
> > * 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
> > @@ -71,7 +71,9 @@
> > */
> > #ifdef SGX_KERNEL_DOC
> > /* C-style function prototype to coerce kernel-doc into parsing the comment. */
> > -int __vdso_sgx_enter_enclave(int leaf, void *tcs,
> > +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);
> > #endif
> > @@ -83,7 +85,10 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
> > .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
> > @@ -109,6 +114,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
> > jne .Linvoke_userspace_handler
> >
> > .Lout:
> > + pop %rbx
> > leave
> > .cfi_def_cfa %rsp, 8
> > ret
> > --
> > 2.24.1
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 1/5] x86/sgx: vdso: Make __vdso_sgx_enter_enclave() callable from C code
2020-04-17 15:05 ` Sean Christopherson
@ 2020-04-17 18:57 ` Jarkko Sakkinen
0 siblings, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-04-17 18:57 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
On Fri, Apr 17, 2020 at 08:05:01AM -0700, Sean Christopherson wrote:
> On Tue, Mar 31, 2020 at 12:04:46AM +0300, Jarkko Sakkinen wrote:
> > On Mon, Mar 30, 2020 at 11:08:07AM -0700, Sean Christopherson wrote:
> > > Make __vdso_sgx_enter_enclave() callable from C by preserving %rbx
> > > and taking @leaf in %rcx instead of %rax. Being able to invoke the vDSO
> > > from C reduces the overhead of runtimes that are tightly coupled with
> > > their enclaves, e.g. that can rely on the enclave to save and restore
> > > non-volatile registers, as the runtime doesn't need an assembly wrapper
> > > to preserve non-volatile registers and/or shuffle stack arguments.
> > >
> > > Note, both %rcx and %rbx are consumed by EENTER/ERESUME, i.e. consuming
> > > them doesn't violate the primary tenet of __vdso_sgx_enter_enclave()
> > > that "thou shalt not restrict how information is exchanged between an
> > > enclave and its host process".
> > >
> > > Suggested-by: Nathaniel McCallum <npmccallum@redhat.com>
> > > Cc: Cedric Xing <cedric.xing@intel.com>
> > > Cc: Jethro Beekman <jethro@fortanix.com>
> > > Cc: Andy Lutomirski <luto@amacapital.net>
> > > Cc: linux-sgx@vger.kernel.org
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > > ---
> > > arch/x86/entry/vdso/vsgx_enter_enclave.S | 30 ++++++++++++++----------
> > > 1 file changed, 18 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > > index 34cee2b0ef09..c56064fb36bc 100644
> > > --- a/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > > +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > > @@ -17,22 +17,22 @@
> > >
> > > /**
> > > * __vdso_sgx_enter_enclave() - 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
> > > * @tcs: TCS, must be non-NULL
> > > * @e: Optional struct sgx_enclave_exception instance
> > > * @handler: Optional enclave exit handler
> > > *
> > > - * **Important!** __vdso_sgx_enter_enclave() is **NOT** compliant with the
> > > - * x86-64 ABI, i.e. cannot be called from standard C code.
> > > - *
> > > - * Input ABI:
> > > - * @leaf %eax
> > > - * @tcs 8(%rsp)
> > > - * @e 0x10(%rsp)
> > > - * @handler 0x18(%rsp)
> > > - *
> > > - * Output ABI:
> > > - * @ret %eax
> > > + * **Important!** __vdso_sgx_enter_enclave() does not ensure full compliance
> >
> > I'd simply put **NOTE** here instead of **Important!** as it is more
> > common.
> >
> > > + * 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.
> >
> > Instead "e.g. doesn't explcitly clear EFLAGS.DF after EEXIT" (which is
> > somewhat confusing statement) paragraph should be replaced with a simple
> > enumerated list of differences.
>
> I don't think the list is that simple, e.g. there is a lot of state that
> is defined by the ABI that isn't touched, IMO talking about that state will
> add confusion.
>
> I also don't understand what's confusing about stating EFLAGS.DF isn't
> cleared.
Too much time was has passed since the last version, and too many
patches have been reviewed since.
Please just refresh the way you feel best and I will make conclusions
based on that. It's faster that way in the end.
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH for_v29 v2 2/5] x86/sgx: vdso: Define a typedef for __vdso_sgx_enter_enclave
2020-03-30 18:08 [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C Sean Christopherson
2020-03-30 18:08 ` [PATCH for_v29 v2 1/5] x86/sgx: vdso: Make __vdso_sgx_enter_enclave() callable from C code Sean Christopherson
@ 2020-03-30 18:08 ` Sean Christopherson
2020-03-30 21:10 ` Jarkko Sakkinen
2020-03-30 18:08 ` [PATCH for_v29 v2 3/5] selftests/sgx: Pass EENTER to vDSO wrapper instead of hardcoding Sean Christopherson
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Sean Christopherson @ 2020-03-30 18:08 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
Define a typedef for the __vdso_sgx_enter_enclave() prototype and move
the entire function comment from the assembly code to the uAPI header,
dropping the kernel doc hack along the way.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
arch/x86/entry/vdso/vsgx_enter_enclave.S | 62 ------------------------
arch/x86/include/uapi/asm/sgx.h | 61 +++++++++++++++++++++++
2 files changed, 61 insertions(+), 62 deletions(-)
diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
index c56064fb36bc..be7e467e1efb 100644
--- a/arch/x86/entry/vdso/vsgx_enter_enclave.S
+++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
@@ -15,68 +15,6 @@
.code64
.section .text, "ax"
-/**
- * __vdso_sgx_enter_enclave() - 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
- * @tcs: TCS, must be non-NULL
- * @e: Optional struct sgx_enclave_exception instance
- * @handler: Optional enclave exit handler
- *
- * **Important!** __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.
- *
- * 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,
- * -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
- */
-#ifdef SGX_KERNEL_DOC
-/* C-style function prototype to coerce kernel-doc into parsing the comment. */
-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);
-#endif
SYM_FUNC_START(__vdso_sgx_enter_enclave)
/* Prolog */
.cfi_startproc
diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index e196cfd44b70..8ca9ef7ea50a 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -111,4 +111,65 @@ typedef int (*sgx_enclave_exit_handler_t)(long rdi, long rsi, long rdx,
void *tcs, int ret,
struct sgx_enclave_exception *e);
+/**
+ * __vdso_sgx_enter_enclave() - 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
+ * @tcs: TCS, must be non-NULL
+ * @e: Optional struct sgx_enclave_exception instance
+ * @handler: Optional enclave exit handler
+ *
+ * **Important!** __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.
+ *
+ * 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,
+ * -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);
+
#endif /* _UAPI_ASM_X86_SGX_H */
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 2/5] x86/sgx: vdso: Define a typedef for __vdso_sgx_enter_enclave
2020-03-30 18:08 ` [PATCH for_v29 v2 2/5] x86/sgx: vdso: Define a typedef for __vdso_sgx_enter_enclave Sean Christopherson
@ 2020-03-30 21:10 ` Jarkko Sakkinen
0 siblings, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-03-30 21:10 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
On Mon, Mar 30, 2020 at 11:08:08AM -0700, Sean Christopherson wrote:
> Define a typedef for the __vdso_sgx_enter_enclave() prototype and move
> the entire function comment from the assembly code to the uAPI header,
> dropping the kernel doc hack along the way.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
> arch/x86/entry/vdso/vsgx_enter_enclave.S | 62 ------------------------
> arch/x86/include/uapi/asm/sgx.h | 61 +++++++++++++++++++++++
> 2 files changed, 61 insertions(+), 62 deletions(-)
>
> diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> index c56064fb36bc..be7e467e1efb 100644
> --- a/arch/x86/entry/vdso/vsgx_enter_enclave.S
> +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> @@ -15,68 +15,6 @@
> .code64
> .section .text, "ax"
>
> -/**
> - * __vdso_sgx_enter_enclave() - 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
> - * @tcs: TCS, must be non-NULL
> - * @e: Optional struct sgx_enclave_exception instance
> - * @handler: Optional enclave exit handler
> - *
> - * **Important!** __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.
> - *
> - * 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,
> - * -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
> - */
> -#ifdef SGX_KERNEL_DOC
> -/* C-style function prototype to coerce kernel-doc into parsing the comment. */
> -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);
> -#endif
> SYM_FUNC_START(__vdso_sgx_enter_enclave)
> /* Prolog */
> .cfi_startproc
> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> index e196cfd44b70..8ca9ef7ea50a 100644
> --- a/arch/x86/include/uapi/asm/sgx.h
> +++ b/arch/x86/include/uapi/asm/sgx.h
> @@ -111,4 +111,65 @@ typedef int (*sgx_enclave_exit_handler_t)(long rdi, long rsi, long rdx,
> void *tcs, int ret,
> struct sgx_enclave_exception *e);
>
> +/**
> + * __vdso_sgx_enter_enclave() - 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
> + * @tcs: TCS, must be non-NULL
> + * @e: Optional struct sgx_enclave_exception instance
> + * @handler: Optional enclave exit handler
> + *
> + * **Important!** __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.
> + *
> + * 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,
> + * -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);
> +
> #endif /* _UAPI_ASM_X86_SGX_H */
> --
> 2.24.1
>
Most probaby agree with this.
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH for_v29 v2 3/5] selftests/sgx: Pass EENTER to vDSO wrapper instead of hardcoding
2020-03-30 18:08 [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C Sean Christopherson
2020-03-30 18:08 ` [PATCH for_v29 v2 1/5] x86/sgx: vdso: Make __vdso_sgx_enter_enclave() callable from C code Sean Christopherson
2020-03-30 18:08 ` [PATCH for_v29 v2 2/5] x86/sgx: vdso: Define a typedef for __vdso_sgx_enter_enclave Sean Christopherson
@ 2020-03-30 18:08 ` Sean Christopherson
2020-03-30 21:07 ` Jarkko Sakkinen
2020-03-30 18:08 ` [PATCH for_v29 v2 4/5] selftests/sgx: Stop clobbering non-volatile registers Sean Christopherson
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Sean Christopherson @ 2020-03-30 18:08 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
Pass EENTER to the vDSO wrapper via %ecx instead of hardcoding it to
make the wrapper compatible with the new vDSO calling convention.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
tools/testing/selftests/sgx/call.S | 1 -
tools/testing/selftests/sgx/defines.h | 1 +
tools/testing/selftests/sgx/main.c | 2 +-
tools/testing/selftests/sgx/main.h | 2 +-
4 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/sgx/call.S b/tools/testing/selftests/sgx/call.S
index c37f55a607c8..77131e83db42 100644
--- a/tools/testing/selftests/sgx/call.S
+++ b/tools/testing/selftests/sgx/call.S
@@ -37,7 +37,6 @@ sgx_call_vdso:
.cfi_adjust_cfa_offset 8
push 0x48(%rsp)
.cfi_adjust_cfa_offset 8
- mov $2, %eax
call *eenter(%rip)
add $0x20, %rsp
.cfi_adjust_cfa_offset -0x20
diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h
index 1802cace7527..be8969922804 100644
--- a/tools/testing/selftests/sgx/defines.h
+++ b/tools/testing/selftests/sgx/defines.h
@@ -15,6 +15,7 @@
#define __packed __attribute__((packed))
#include "../../../../arch/x86/kernel/cpu/sgx/arch.h"
+#include "../../../../arch/x86/include/asm/enclu.h"
#include "../../../../arch/x86/include/uapi/asm/sgx.h"
#endif /* DEFINES_H */
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index 9238cce47f77..f6bb40f22884 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -171,7 +171,7 @@ int main(int argc, char *argv[], char *envp[])
eenter = addr + eenter_sym->st_value;
- sgx_call_vdso((void *)&MAGIC, &result, 0, NULL, NULL, NULL,
+ sgx_call_vdso((void *)&MAGIC, &result, 0, EENTER, NULL, NULL,
(void *)encl.encl_base, &exception, NULL);
if (result != MAGIC)
goto err;
diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h
index 6e1ae292bd25..999422cc7343 100644
--- a/tools/testing/selftests/sgx/main.h
+++ b/tools/testing/selftests/sgx/main.h
@@ -32,7 +32,7 @@ bool encl_load(const char *path, struct encl *encl);
bool encl_measure(struct encl *encl);
bool encl_build(struct encl *encl);
-int sgx_call_vdso(void *rdi, void *rsi, long rdx, void *rcx, void *r8, void *r9,
+int sgx_call_vdso(void *rdi, void *rsi, long rdx, u32 leaf, void *r8, void *r9,
void *tcs, struct sgx_enclave_exception *ei, void *cb);
#endif /* MAIN_H */
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 3/5] selftests/sgx: Pass EENTER to vDSO wrapper instead of hardcoding
2020-03-30 18:08 ` [PATCH for_v29 v2 3/5] selftests/sgx: Pass EENTER to vDSO wrapper instead of hardcoding Sean Christopherson
@ 2020-03-30 21:07 ` Jarkko Sakkinen
2020-03-30 21:11 ` Jarkko Sakkinen
0 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-03-30 21:07 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
On Mon, Mar 30, 2020 at 11:08:09AM -0700, Sean Christopherson wrote:
> Pass EENTER to the vDSO wrapper via %ecx instead of hardcoding it to
> make the wrapper compatible with the new vDSO calling convention.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
I think it'd be good if you merge 1/5-3/5 and 4/5-55 to have two patches.
Easier to review given the strong bounds, easier for you to update
and easier for me to merge later on.
/Jarko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 3/5] selftests/sgx: Pass EENTER to vDSO wrapper instead of hardcoding
2020-03-30 21:07 ` Jarkko Sakkinen
@ 2020-03-30 21:11 ` Jarkko Sakkinen
0 siblings, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-03-30 21:11 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
On Tue, Mar 31, 2020 at 12:07:16AM +0300, Jarkko Sakkinen wrote:
> On Mon, Mar 30, 2020 at 11:08:09AM -0700, Sean Christopherson wrote:
> > Pass EENTER to the vDSO wrapper via %ecx instead of hardcoding it to
> > make the wrapper compatible with the new vDSO calling convention.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>
> I think it'd be good if you merge 1/5-3/5 and 4/5-55 to have two patches.
>
> Easier to review given the strong bounds, easier for you to update
> and easier for me to merge later on.
I mean {1, 2, 3} and {4, 5}
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH for_v29 v2 4/5] selftests/sgx: Stop clobbering non-volatile registers
2020-03-30 18:08 [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C Sean Christopherson
` (2 preceding siblings ...)
2020-03-30 18:08 ` [PATCH for_v29 v2 3/5] selftests/sgx: Pass EENTER to vDSO wrapper instead of hardcoding Sean Christopherson
@ 2020-03-30 18:08 ` Sean Christopherson
2020-03-30 18:08 ` [PATCH for_v29 v2 5/5] selftests/sgx: Add selftest to invoke __vsgx_enter_enclave() from C Sean Christopherson
2020-03-30 20:48 ` [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable " Jarkko Sakkinen
5 siblings, 0 replies; 21+ messages in thread
From: Sean Christopherson @ 2020-03-30 18:08 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
Stop clearing non-volatile registers in the enclave's trampoline code,
there are no secrets to preserve, and not clobbering the registers makes
the enclave compatible with calling the vDSO from C.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
tools/testing/selftests/sgx/test_encl_bootstrap.S | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S
index 6a5d734cbf16..6836ea86126e 100644
--- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
+++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
@@ -54,7 +54,7 @@ encl_entry:
pop %rbx # pop the enclave base address
- # Clear GPRs.
+ /* Clear volatile GPRs, except RAX (EEXIT leaf). */
xor %rcx, %rcx
xor %rdx, %rdx
xor %rdi, %rdi
@@ -63,10 +63,6 @@ encl_entry:
xor %r9, %r9
xor %r10, %r10
xor %r11, %r11
- xor %r12, %r12
- xor %r13, %r13
- xor %r14, %r14
- xor %r15, %r15
# Reset status flags.
add %rdx, %rdx # OF = SF = AF = CF = 0; ZF = PF = 1
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH for_v29 v2 5/5] selftests/sgx: Add selftest to invoke __vsgx_enter_enclave() from C
2020-03-30 18:08 [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C Sean Christopherson
` (3 preceding siblings ...)
2020-03-30 18:08 ` [PATCH for_v29 v2 4/5] selftests/sgx: Stop clobbering non-volatile registers Sean Christopherson
@ 2020-03-30 18:08 ` Sean Christopherson
2020-03-30 20:48 ` [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable " Jarkko Sakkinen
5 siblings, 0 replies; 21+ messages in thread
From: Sean Christopherson @ 2020-03-30 18:08 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
Add a selftest to call __vsgx_enter_enclave() from C.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
tools/testing/selftests/sgx/main.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index f6bb40f22884..5394b2f6af8e 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -19,7 +19,7 @@
#include "main.h"
static const uint64_t MAGIC = 0x1122334455667788ULL;
-void *eenter;
+vdso_sgx_enter_enclave_t eenter;
struct vdso_symtab {
Elf64_Sym *elf_symtab;
@@ -173,15 +173,27 @@ int main(int argc, char *argv[], char *envp[])
sgx_call_vdso((void *)&MAGIC, &result, 0, EENTER, NULL, NULL,
(void *)encl.encl_base, &exception, NULL);
- if (result != MAGIC)
+ if (result != MAGIC) {
+ printf("FAIL: sgx_call_vdso(), expected: 0x%lx, got: 0x%lx\n",
+ MAGIC, result);
goto err;
+ }
+
+ /* Invoke the vDSO directly. */
+ result = 0;
+ eenter((unsigned long)&MAGIC, (unsigned long)&result, 0, EENTER, 0, 0,
+ (void *)encl.encl_base, &exception, NULL);
+ if (result != MAGIC) {
+ printf("FAIL: eenter(), expected: 0x%lx, got: 0x%lx\n",
+ MAGIC, result);
+ goto err;
+ }
printf("SUCCESS\n");
encl_delete(&encl);
exit(0);
err:
- printf("FAILURE\n");
encl_delete(&encl);
exit(1);
}
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-03-30 18:08 [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C Sean Christopherson
` (4 preceding siblings ...)
2020-03-30 18:08 ` [PATCH for_v29 v2 5/5] selftests/sgx: Add selftest to invoke __vsgx_enter_enclave() from C Sean Christopherson
@ 2020-03-30 20:48 ` Jarkko Sakkinen
2020-03-30 21:42 ` Nathaniel McCallum
5 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-03-30 20:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> close to being callable from C (with caveats and a cooperative enclave).
> The missing pieces are preserving %rbx and taking @leaf as a standard
> parameter.
>
> v2:
> - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> - Add CFI directive for RBX. [Cedric]
I'm sorry for throwing stick's constantly but I think having a real
ELF loader is for better.
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-03-30 20:48 ` [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable " Jarkko Sakkinen
@ 2020-03-30 21:42 ` Nathaniel McCallum
2020-03-31 11:58 ` Jarkko Sakkinen
0 siblings, 1 reply; 21+ messages in thread
From: Nathaniel McCallum @ 2020-03-30 21:42 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sean Christopherson, Cedric Xing, Jethro Beekman,
Andy Lutomirski, linux-sgx
On Mon, Mar 30, 2020 at 4:48 PM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> > Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> > close to being callable from C (with caveats and a cooperative enclave).
> > The missing pieces are preserving %rbx and taking @leaf as a standard
> > parameter.
> >
> > v2:
> > - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> > - Add CFI directive for RBX. [Cedric]
>
> I'm sorry for throwing stick's constantly but I think having a real
> ELF loader is for better.
Why is it one or the other?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-03-30 21:42 ` Nathaniel McCallum
@ 2020-03-31 11:58 ` Jarkko Sakkinen
2020-03-31 13:40 ` Nathaniel McCallum
0 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-03-31 11:58 UTC (permalink / raw)
To: Nathaniel McCallum
Cc: Sean Christopherson, Cedric Xing, Jethro Beekman,
Andy Lutomirski, linux-sgx
On Mon, Mar 30, 2020 at 05:42:29PM -0400, Nathaniel McCallum wrote:
> On Mon, Mar 30, 2020 at 4:48 PM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> > > Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> > > close to being callable from C (with caveats and a cooperative enclave).
> > > The missing pieces are preserving %rbx and taking @leaf as a standard
> > > parameter.
> > >
> > > v2:
> > > - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> > > - Add CFI directive for RBX. [Cedric]
> >
> > I'm sorry for throwing stick's constantly but I think having a real
> > ELF loader is for better.
>
> Why is it one or the other?
Hmm... Not sure I understand what you want to know.
A raw binary requires a hardcoded way to interpret it. Obviously ELF
is for better.
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-03-31 11:58 ` Jarkko Sakkinen
@ 2020-03-31 13:40 ` Nathaniel McCallum
2020-04-01 8:17 ` Jarkko Sakkinen
0 siblings, 1 reply; 21+ messages in thread
From: Nathaniel McCallum @ 2020-03-31 13:40 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sean Christopherson, Cedric Xing, Jethro Beekman,
Andy Lutomirski, linux-sgx
On Tue, Mar 31, 2020 at 7:58 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Mon, Mar 30, 2020 at 05:42:29PM -0400, Nathaniel McCallum wrote:
> > On Mon, Mar 30, 2020 at 4:48 PM Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> > > > Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> > > > close to being callable from C (with caveats and a cooperative enclave).
> > > > The missing pieces are preserving %rbx and taking @leaf as a standard
> > > > parameter.
> > > >
> > > > v2:
> > > > - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> > > > - Add CFI directive for RBX. [Cedric]
> > >
> > > I'm sorry for throwing stick's constantly but I think having a real
> > > ELF loader is for better.
This statement seems like you are juxtaposing having
__vdso_sgx_enter_enclave() be potentially C-compatible with having an
ELF-loader. These are not incompabile. __vdso_sgx_enter_enclave() can
be C-callable *and* you can have an ELF loader.
> > Why is it one or the other?
>
> Hmm... Not sure I understand what you want to know.
>
> A raw binary requires a hardcoded way to interpret it. Obviously ELF
> is for better.
We (Enarx) implement an ELF loader.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-03-31 13:40 ` Nathaniel McCallum
@ 2020-04-01 8:17 ` Jarkko Sakkinen
2020-04-01 13:06 ` Nathaniel McCallum
0 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-04-01 8:17 UTC (permalink / raw)
To: Nathaniel McCallum
Cc: Sean Christopherson, Cedric Xing, Jethro Beekman,
Andy Lutomirski, linux-sgx
On Tue, Mar 31, 2020 at 09:40:24AM -0400, Nathaniel McCallum wrote:
> On Tue, Mar 31, 2020 at 7:58 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Mon, Mar 30, 2020 at 05:42:29PM -0400, Nathaniel McCallum wrote:
> > > On Mon, Mar 30, 2020 at 4:48 PM Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > >
> > > > On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> > > > > Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> > > > > close to being callable from C (with caveats and a cooperative enclave).
> > > > > The missing pieces are preserving %rbx and taking @leaf as a standard
> > > > > parameter.
> > > > >
> > > > > v2:
> > > > > - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> > > > > - Add CFI directive for RBX. [Cedric]
> > > >
> > > > I'm sorry for throwing stick's constantly but I think having a real
> > > > ELF loader is for better.
>
> This statement seems like you are juxtaposing having
> __vdso_sgx_enter_enclave() be potentially C-compatible with having an
> ELF-loader. These are not incompabile. __vdso_sgx_enter_enclave() can
> be C-callable *and* you can have an ELF loader.
I'm not honestly sure what this is about but my comment was about heavy
rebasing of the GIT tree as I rewrote the selftest last week.
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-04-01 8:17 ` Jarkko Sakkinen
@ 2020-04-01 13:06 ` Nathaniel McCallum
2020-04-01 14:49 ` Sean Christopherson
2020-04-02 19:49 ` Jarkko Sakkinen
0 siblings, 2 replies; 21+ messages in thread
From: Nathaniel McCallum @ 2020-04-01 13:06 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sean Christopherson, Cedric Xing, Jethro Beekman,
Andy Lutomirski, linux-sgx
On Wed, Apr 1, 2020 at 4:18 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Tue, Mar 31, 2020 at 09:40:24AM -0400, Nathaniel McCallum wrote:
> > On Tue, Mar 31, 2020 at 7:58 AM Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Mon, Mar 30, 2020 at 05:42:29PM -0400, Nathaniel McCallum wrote:
> > > > On Mon, Mar 30, 2020 at 4:48 PM Jarkko Sakkinen
> > > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > >
> > > > > On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> > > > > > Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> > > > > > close to being callable from C (with caveats and a cooperative enclave).
> > > > > > The missing pieces are preserving %rbx and taking @leaf as a standard
> > > > > > parameter.
> > > > > >
> > > > > > v2:
> > > > > > - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> > > > > > - Add CFI directive for RBX. [Cedric]
> > > > >
> > > > > I'm sorry for throwing stick's constantly but I think having a real
> > > > > ELF loader is for better.
> >
> > This statement seems like you are juxtaposing having
> > __vdso_sgx_enter_enclave() be potentially C-compatible with having an
> > ELF-loader. These are not incompabile. __vdso_sgx_enter_enclave() can
> > be C-callable *and* you can have an ELF loader.
>
> I'm not honestly sure what this is about but my comment was about heavy
> rebasing of the GIT tree as I rewrote the selftest last week.
Okay. Let's chalk it up to miscommunication then. :)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-04-01 13:06 ` Nathaniel McCallum
@ 2020-04-01 14:49 ` Sean Christopherson
2020-04-02 20:01 ` Jarkko Sakkinen
2020-04-02 19:49 ` Jarkko Sakkinen
1 sibling, 1 reply; 21+ messages in thread
From: Sean Christopherson @ 2020-04-01 14:49 UTC (permalink / raw)
To: Nathaniel McCallum
Cc: Jarkko Sakkinen, Cedric Xing, Jethro Beekman, Andy Lutomirski, linux-sgx
On Wed, Apr 01, 2020 at 09:06:38AM -0400, Nathaniel McCallum wrote:
> On Wed, Apr 1, 2020 at 4:18 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Tue, Mar 31, 2020 at 09:40:24AM -0400, Nathaniel McCallum wrote:
> > > On Tue, Mar 31, 2020 at 7:58 AM Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > >
> > > > On Mon, Mar 30, 2020 at 05:42:29PM -0400, Nathaniel McCallum wrote:
> > > > > On Mon, Mar 30, 2020 at 4:48 PM Jarkko Sakkinen
> > > > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> > > > > > > Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> > > > > > > close to being callable from C (with caveats and a cooperative enclave).
> > > > > > > The missing pieces are preserving %rbx and taking @leaf as a standard
> > > > > > > parameter.
> > > > > > >
> > > > > > > v2:
> > > > > > > - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> > > > > > > - Add CFI directive for RBX. [Cedric]
> > > > > >
> > > > > > I'm sorry for throwing stick's constantly but I think having a real
> > > > > > ELF loader is for better.
> > >
> > > This statement seems like you are juxtaposing having
> > > __vdso_sgx_enter_enclave() be potentially C-compatible with having an
> > > ELF-loader. These are not incompabile. __vdso_sgx_enter_enclave() can
> > > be C-callable *and* you can have an ELF loader.
> >
> > I'm not honestly sure what this is about but my comment was about heavy
> > rebasing of the GIT tree as I rewrote the selftest last week.
>
> Okay. Let's chalk it up to miscommunication then. :)
Ha, I was in the same boat as Nathaniel. We thought the "having a real
ELF loader comment" was a comment on the patch itself, i.e. that you
disagreed with it in some way because it didn't support an ELF loader,
hence our confusion.
Now I realize you were refering to the rebase needed due to rewriting the
selftest to use an ELF loader. Crisis aborted :-)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-04-01 14:49 ` Sean Christopherson
@ 2020-04-02 20:01 ` Jarkko Sakkinen
0 siblings, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-04-02 20:01 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nathaniel McCallum, Cedric Xing, Jethro Beekman, Andy Lutomirski,
linux-sgx
On Wed, Apr 01, 2020 at 07:49:38AM -0700, Sean Christopherson wrote:
> On Wed, Apr 01, 2020 at 09:06:38AM -0400, Nathaniel McCallum wrote:
> > On Wed, Apr 1, 2020 at 4:18 AM Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Tue, Mar 31, 2020 at 09:40:24AM -0400, Nathaniel McCallum wrote:
> > > > On Tue, Mar 31, 2020 at 7:58 AM Jarkko Sakkinen
> > > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > >
> > > > > On Mon, Mar 30, 2020 at 05:42:29PM -0400, Nathaniel McCallum wrote:
> > > > > > On Mon, Mar 30, 2020 at 4:48 PM Jarkko Sakkinen
> > > > > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > > > >
> > > > > > > On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> > > > > > > > Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> > > > > > > > close to being callable from C (with caveats and a cooperative enclave).
> > > > > > > > The missing pieces are preserving %rbx and taking @leaf as a standard
> > > > > > > > parameter.
> > > > > > > >
> > > > > > > > v2:
> > > > > > > > - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> > > > > > > > - Add CFI directive for RBX. [Cedric]
> > > > > > >
> > > > > > > I'm sorry for throwing stick's constantly but I think having a real
> > > > > > > ELF loader is for better.
> > > >
> > > > This statement seems like you are juxtaposing having
> > > > __vdso_sgx_enter_enclave() be potentially C-compatible with having an
> > > > ELF-loader. These are not incompabile. __vdso_sgx_enter_enclave() can
> > > > be C-callable *and* you can have an ELF loader.
> > >
> > > I'm not honestly sure what this is about but my comment was about heavy
> > > rebasing of the GIT tree as I rewrote the selftest last week.
> >
> > Okay. Let's chalk it up to miscommunication then. :)
>
> Ha, I was in the same boat as Nathaniel. We thought the "having a real
> ELF loader comment" was a comment on the patch itself, i.e. that you
> disagreed with it in some way because it didn't support an ELF loader,
> hence our confusion.
>
> Now I realize you were refering to the rebase needed due to rewriting the
> selftest to use an ELF loader. Crisis aborted :-)
Awesome :-)
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for_v29 v2 0/5] x86/sgx: Make vDSO callable from C
2020-04-01 13:06 ` Nathaniel McCallum
2020-04-01 14:49 ` Sean Christopherson
@ 2020-04-02 19:49 ` Jarkko Sakkinen
1 sibling, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2020-04-02 19:49 UTC (permalink / raw)
To: Nathaniel McCallum
Cc: Sean Christopherson, Cedric Xing, Jethro Beekman,
Andy Lutomirski, linux-sgx
On Wed, Apr 01, 2020 at 09:06:38AM -0400, Nathaniel McCallum wrote:
> On Wed, Apr 1, 2020 at 4:18 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Tue, Mar 31, 2020 at 09:40:24AM -0400, Nathaniel McCallum wrote:
> > > On Tue, Mar 31, 2020 at 7:58 AM Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > >
> > > > On Mon, Mar 30, 2020 at 05:42:29PM -0400, Nathaniel McCallum wrote:
> > > > > On Mon, Mar 30, 2020 at 4:48 PM Jarkko Sakkinen
> > > > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 30, 2020 at 11:08:06AM -0700, Sean Christopherson wrote:
> > > > > > > Nathaniel pointed out that __vdso_sgx_enter_enclave() is tantalizingly
> > > > > > > close to being callable from C (with caveats and a cooperative enclave).
> > > > > > > The missing pieces are preserving %rbx and taking @leaf as a standard
> > > > > > > parameter.
> > > > > > >
> > > > > > > v2:
> > > > > > > - Rebase to Jarkko's latest master, commit 402fb35a477a, "docs: ...")
> > > > > > > - Add CFI directive for RBX. [Cedric]
> > > > > >
> > > > > > I'm sorry for throwing stick's constantly but I think having a real
> > > > > > ELF loader is for better.
> > >
> > > This statement seems like you are juxtaposing having
> > > __vdso_sgx_enter_enclave() be potentially C-compatible with having an
> > > ELF-loader. These are not incompabile. __vdso_sgx_enter_enclave() can
> > > be C-callable *and* you can have an ELF loader.
> >
> > I'm not honestly sure what this is about but my comment was about heavy
> > rebasing of the GIT tree as I rewrote the selftest last week.
>
> Okay. Let's chalk it up to miscommunication then. :)
Yeah, lets bring a background :-)
Last week basically rewrote the self-test. It now directly loads ELF and
dynamically signs that. Thus, the codebase has been kind of moving
target for a while. I'm absolutely for these changes as soon as the
form fits to the existing codebase.
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread