linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Xing, Cedric" <cedric.xing@intel.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: "Christopherson, Sean J" <sean.j.christopherson@intel.com>,
	"Jarkko Sakkinen" <jarkko.sakkinen@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"nhorman@redhat.com" <nhorman@redhat.com>,
	"npmccallum@redhat.com" <npmccallum@redhat.com>,
	"Ayoun, Serge" <serge.ayoun@intel.com>,
	"Katz-zamir, Shay" <shay.katz-zamir@intel.com>,
	"Huang, Haitao" <haitao.huang@intel.com>,
	"andriy.shevchenko@linux.intel.com" 
	<andriy.shevchenko@linux.intel.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"Svahn, Kai" <kai.svahn@intel.com>, "bp@alien8.de" <bp@alien8.de>,
	"josh@joshtriplett.org" <josh@joshtriplett.org>,
	"Huang, Kai" <kai.huang@intel.com>,
	"rientjes@google.com" <rientjes@google.com>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	Haitao Huang <haitao.huang@linux.intel.com>,
	Jethro Beekman <jethro@fortanix.com>,
	"Dr . Greg Wettstein" <greg@enjellic.com>
Subject: RE: [PATCH v19,RESEND 24/27] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions
Date: Thu, 28 Mar 2019 04:23:37 +0000	[thread overview]
Message-ID: <960B34DE67B9E140824F1DCDEC400C0F4E860C69@ORSMSX116.amr.corp.intel.com> (raw)
In-Reply-To: <CALCETrVcR_etAHSVZxvCz7CcPnF4ajpcWiRCtFrOckVWVJByHA@mail.gmail.com>

Hi Andy,

> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Andy Lutomirski
> 
> I suppose the real question is: are there a significant number of
> users who will want to run enclaves created using an old SDK on Linux?
>  And will there actually be support for doing this in the software
> stack?

To your first question, I cannot share information of Intel customers or speak for them. But in general, people would like to stay with an old enclave usually because of: 1) attestation, because MRENCLAVE will change after rebuild; and/or 2) the need to support a mix of older and newer Linux kernels. So I'd say it'll be commonly desired, especially when this vDSO API is still "new" (so not available on every platform).

To your second question, Intel will support all "legacy" enclaves built with older SGX SDKs on newer kernels. And that's why we are so eager to find a migration path. I can't speak for other companies, but guess backward compatibility is always desirable.

> 
> If the answer to both questions is yes, then it seems like it could be
> reasonable to support it in the vDSO.  But I still think it should
> probably be a different vDSO entry point so that the normal case
> doesn't become more complicated.

I'll support whatever you think is more appropriate.

At the end, I'd like to give out the full version of my proposal, with your feedbacks (i.e. stack unwinder and Spectre variant 2) addressed. I'm a bit concerned by retpoline, which won't work (or be needed) when CET comes online. Are you looking to change it again then?

Here's the summary of the changes:
 - Added CFI directives for proper unwinding.
 - Defined sgx_ex_callback - the callback function on enclave exit/exception.
 - Aligned stack properly before calling sgx_ex_callback (per x86_64 ABI).
 - Used retpoline in place of indirect call.
 - The block starting at label "4:" captures all the code necessary to support sgx_ex_call. It has grown longer due to retpoline.

/**
 * __vdso_sgx_enter_enclave() - Enter an SGX enclave
 *
 * %eax:        ENCLU leaf, must be either EENTER or ERESUME
 * 0x08(%rsp):  TCS
 * 0x10(%rsp):  Optional pointer to 'struct sgx_enclave_exception'
 * 0x18(%rsp):  Optional function pointer to 'sgx_ex_callback', whose
 *              definition will be given below. Note that this function, if
 *              present, shall follow x86_64 ABI.
 * return:      0 (zero) on success, or a negative error code on failure.
 *
 * Note that __vdso_sgx_enter_enclave() is not compatible with x86_64 ABI.
 * All registers except RBP must be treated as volatile from the caller's
 * perspective, including but not limited to GPRs, EFLAGS.DF, MXCSR, FCW, etc...
 * Enclave may decrement RSP, but must not increment it - i.e. existing content
 * of the stack shall be preserved.
 *
 * sgx_ex_callback - A callback function to be invoked by
 * __vdso_sgx_enter_enclave() upon exception or after the enclave exits.
 *
 * typedef int (*sgx_ex_callback)(long rdi, long rsi, long rdx,
 *      struct sgx_enclave_exception *ex_info, long r8, long r9,
 *      long rsp, void *tcs);
 *
 * Note that sgx_ex_callback shall be x86_64 ABI compliant.
 *
 * Note that other GPRs (except %rax, %rbx and %rcx) are also passed through to
 * sgx_ex_callback, even though accessing them requires assembly code.
 */
__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

1:      /* EENTER <= leaf <= ERESUME */
        cmp     $0x2, %eax
        jb      5f
        cmp     $0x3, %eax
        ja      5f

        /* Load TCS and AEP */
        mov     0x10(%rbp), %rbx
        lea     2f(%rip), %rcx

2:      enclu

        /* EEXIT path */
        mov     0x18(%rbp), %rcx
        jrcxz   3f
        mov     %eax, EX_LEAF(%rcx)
        /* normalize return value */
3:      xor     %eax, %eax

4:      /* call sgx_ex_callback if supplied */
        cmpq    $0, 0x20(%rbp)
        jz      6f
        /* align stack per x86_64 ABI */
        mov     %rsp, %rbx
        and     $-0x10, %rsp
        /* parameters */
        push    0x10(%rbp)
        push    %rbx
        /* call *0x20(%rbp) using retpoline */
        mov     0x20(%rbp), %rax
        call    41f
        /* stack cleanup */
        mov     %rbx, %rsp
        jmp     1b
41:     call    43f
42:     pause
        lfence
        jmp     42b
43:     mov     %rax, (%rsp)
        ret

5:      /* bad leaf */
        cmp     $0, %eax
        jle     6f
        mov     $(-EINVAL), %eax

6:      /* epilog */
        leave
        .cfi_def_cfa            %rsp, 8
        ret
        .cfi_endproc

.pushsection    .fixup, "ax"
7:      mov     0x18(%rbp), %rcx
        jrcxz   8f
        /* fill in ex_info */
        mov     %eax, EX_LEAF(%rcx)
        mov     %di, EX_TRAPNR(%rcx)
        mov     %si, EX_ERROR_CODE(%rcx)
        mov     %rdx, EX_ADDRESS(%rcx)
8:      mov     $(-EFAULT), %eax
        jmp     4b
.popsection

_ASM_VDSO_EXTABLE_HANDLE(2b, 7b)


-Cedric

  reply	other threads:[~2019-03-28  4:23 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-20 16:20 [PATCH v19,RESEND 00/27] Intel SGX1 support Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 01/27] x86/cpufeatures: Add Intel-defined SGX feature bit Jarkko Sakkinen
2019-03-20 19:41   ` Neil Horman
2019-03-21 14:16     ` Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 02/27] x86/cpufeatures: Add SGX sub-features (as Linux-defined bits) Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 03/27] x86/msr: Add IA32_FEATURE_CONTROL.SGX_ENABLE definition Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 04/27] x86/cpufeatures: Add Intel-defined SGX_LC feature bit Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 05/27] x86/msr: Add SGX Launch Control MSR definitions Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 06/27] x86/mm: x86/sgx: Add new 'PF_SGX' page fault error code bit Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 07/27] x86/mm: x86/sgx: Signal SIGSEGV for userspace #PFs w/ PF_SGX Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 08/27] x86/cpu/intel: Detect SGX support and update caps appropriately Jarkko Sakkinen
2019-03-26 12:17   ` Huang, Kai
2019-03-26 14:27     ` Sean Christopherson
2019-03-26 21:25       ` Huang, Kai
2019-03-26 21:57         ` Sean Christopherson
2019-03-26 23:19           ` Huang, Kai
2019-03-20 16:21 ` [PATCH v19,RESEND 09/27] x86/sgx: Add ENCLS architectural error codes Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 10/27] x86/sgx: Add SGX1 and SGX2 architectural data structures Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 11/27] x86/sgx: Add definitions for SGX's CPUID leaf and variable sub-leafs Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 12/27] x86/sgx: Enumerate and track EPC sections Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 13/27] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 14/27] x86/sgx: Add functions to allocate and free EPC pages Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 15/27] x86/sgx: Add sgx_einit() for initializing enclaves Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 16/27] x86/sgx: Add the Linux SGX Enclave Driver Jarkko Sakkinen
2019-03-26 12:01   ` Huang, Kai
2019-03-26 12:40     ` Thomas Gleixner
2019-03-26 14:54       ` Sean Christopherson
2019-03-26 21:11         ` Huang, Kai
2019-03-27  5:02     ` Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 17/27] x86/sgx: Add provisioning Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 18/27] x86/sgx: Add swapping code to the core and SGX driver Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 19/27] x86/sgx: ptrace() support for the " Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 20/27] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 21/27] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 22/27] x86/fault: Attempt to fixup unhandled #PF in vDSO before signaling Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 23/27] x86/traps: Attempt to fixup exceptions " Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 24/27] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions Jarkko Sakkinen
2019-03-20 18:30   ` Xing, Cedric
2019-03-20 18:52     ` Andy Lutomirski
2019-03-20 19:57       ` Xing, Cedric
2019-03-20 21:03         ` Sean Christopherson
2019-03-21  0:17           ` Xing, Cedric
2019-03-22 21:20             ` Sean Christopherson
2019-03-21 17:17         ` Andy Lutomirski
2019-03-22 20:31           ` Xing, Cedric
2019-03-20 19:02     ` Jethro Beekman
2019-03-20 20:10       ` Xing, Cedric
2019-03-20 19:13     ` Sean Christopherson
2019-03-20 20:38       ` Xing, Cedric
2019-03-22 21:59         ` Sean Christopherson
2019-03-23 17:36           ` Xing, Cedric
2019-03-23 21:38             ` Andy Lutomirski
2019-03-24  8:59               ` Xing, Cedric
2019-03-25 18:03                 ` Sean Christopherson
2019-03-25 23:59                   ` Andy Lutomirski
2019-03-26  4:53                     ` Xing, Cedric
2019-03-26 17:08                       ` Andy Lutomirski
2019-03-28  4:23                         ` Xing, Cedric [this message]
2019-03-28 19:18                           ` Andy Lutomirski
2019-03-28 23:19                             ` Xing, Cedric
2019-03-29  9:48                               ` Jarkko Sakkinen
2019-03-31  8:43                                 ` Dr. Greg
2019-04-03 23:03                             ` Sean Christopherson
2019-03-25 23:54                 ` Andy Lutomirski
2019-03-26  4:16                   ` Xing, Cedric
2019-03-20 16:21 ` [PATCH v19,RESEND 25/27] x86/sgx: SGX documentation Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 26/27] selftests/x86: Add a selftest for SGX Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 27/27] x86/sgx: Update MAINTAINERS Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=960B34DE67B9E140824F1DCDEC400C0F4E860C69@ORSMSX116.amr.corp.intel.com \
    --to=cedric.xing@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=greg@enjellic.com \
    --cc=haitao.huang@intel.com \
    --cc=haitao.huang@linux.intel.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jethro@fortanix.com \
    --cc=josh@joshtriplett.org \
    --cc=kai.huang@intel.com \
    --cc=kai.svahn@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=nhorman@redhat.com \
    --cc=npmccallum@redhat.com \
    --cc=rientjes@google.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=serge.ayoun@intel.com \
    --cc=shay.katz-zamir@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).