Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-21 Thread Jarkko Sakkinen
On Fri, Dec 21, 2018 at 10:28:09AM -0800, Sean Christopherson wrote: > > Why would you want to pass EPC through user space to KVM rather than > > KVM allocating it through kernel interfaces? > > Delegating EPC management to userspace fits better with KVM's existing > memory ABI. KVM provides a

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-21 Thread Sean Christopherson
On Wed, Dec 19, 2018 at 07:00:47AM +0200, Jarkko Sakkinen wrote: > On Tue, Dec 18, 2018 at 10:53:49AM -0800, Sean Christopherson wrote: > > What if we re-organize the ioctls in such a way that we leave open the > > possibility of allocating raw EPC for KVM via /dev/sgx? I'm not 100% > > positive

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Wed, Dec 19, 2018 at 06:47:32AM +0200, Jarkko Sakkinen wrote: > On Tue, Dec 18, 2018 at 07:44:18AM -0800, Sean Christopherson wrote: > > My fd/inode knowledge is lacking, to say the least. Whatever works, so > > long as we have a way to uniquely identify enclaves. > > I will simply trial and

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 09:55:08PM -0800, Andy Lutomirski wrote: > On Thu, Nov 15, 2018 at 5:08 PM Jarkko Sakkinen > wrote: > > > > Intel Software Guard eXtensions (SGX) is a set of CPU instructions that > > can be used by applications to set aside private regions of code and > > data. The code

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Wed, Dec 19, 2018 at 07:00:47AM +0200, Jarkko Sakkinen wrote: > On Tue, Dec 18, 2018 at 10:53:49AM -0800, Sean Christopherson wrote: > > What if we re-organize the ioctls in such a way that we leave open the > > possibility of allocating raw EPC for KVM via /dev/sgx? I'm not 100% > > positive

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Tue, Dec 18, 2018 at 10:53:49AM -0800, Sean Christopherson wrote: > What if we re-organize the ioctls in such a way that we leave open the > possibility of allocating raw EPC for KVM via /dev/sgx? I'm not 100% > positive this approach will work[1], but conceptually it fits well with > KVM's

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Tue, Dec 18, 2018 at 07:44:18AM -0800, Sean Christopherson wrote: > My fd/inode knowledge is lacking, to say the least. Whatever works, so > long as we have a way to uniquely identify enclaves. I will simply trial and error :-) I think it should work since it does own an address space, but

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Sean Christopherson
On Tue, Dec 18, 2018 at 07:44:18AM -0800, Sean Christopherson wrote: > On Mon, Dec 17, 2018 at 08:59:54PM -0800, Andy Lutomirski wrote: > > On Mon, Dec 17, 2018 at 2:20 PM Sean Christopherson > > wrote: > > > > > > > > My brain is still sorting out the details, but I generally like the idea > >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Sean Christopherson
On Tue, Dec 18, 2018 at 03:13:11PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 10:21:49PM +0200, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 09:33:22PM +0200, Jarkko Sakkinen wrote: > > > On Mon, Dec 17, 2018 at 10:48:58AM -0800, Sean Christopherson wrote: > > > > On Mon, Dec

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Sean Christopherson
On Mon, Dec 17, 2018 at 08:59:54PM -0800, Andy Lutomirski wrote: > On Mon, Dec 17, 2018 at 2:20 PM Sean Christopherson > wrote: > > > > > My brain is still sorting out the details, but I generally like the idea > > of allocating an anon inode when creating an enclave, and exposing the > > other

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 09:02:03PM -0800, Andy Lutomirski wrote: > That's what unmap_mapping_range(), etc do for you, no? IOW make a > struct address_space that represents the logical enclave address > space, i.e. address 0 is the start and the pages count up from there. > You can unmap pages

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 08:55:02PM -0800, Andy Lutomirski wrote: > On Mon, Dec 17, 2018 at 5:39 PM Jarkko Sakkinen > wrote: > > > > On Mon, Dec 17, 2018 at 02:20:48PM -0800, Sean Christopherson wrote: > > > The only potential hiccup I can see is the build flow. Currently, > > > EADD+EEXTEND is

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 10:21:49PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 09:33:22PM +0200, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 10:48:58AM -0800, Sean Christopherson wrote: > > > On Mon, Dec 17, 2018 at 08:43:33PM +0200, Jarkko Sakkinen wrote: > > > > On Mon, Dec

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-18 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 08:59:54PM -0800, Andy Lutomirski wrote: > On Mon, Dec 17, 2018 at 2:20 PM Sean Christopherson > wrote: > > > > > My brain is still sorting out the details, but I generally like the idea > > of allocating an anon inode when creating an enclave, and exposing the > > other

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Andy Lutomirski
On Thu, Nov 15, 2018 at 5:08 PM Jarkko Sakkinen wrote: > > Intel Software Guard eXtensions (SGX) is a set of CPU instructions that > can be used by applications to set aside private regions of code and > data. The code outside the enclave is disallowed to access the memory > inside the enclave by

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Andy Lutomirski
On Mon, Dec 17, 2018 at 7:27 PM Jarkko Sakkinen wrote: > > On Tue, Dec 18, 2018 at 03:39:18AM +0200, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 02:20:48PM -0800, Sean Christopherson wrote: > > > The only potential hiccup I can see is the build flow. Currently, > > > EADD+EEXTEND is done

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Andy Lutomirski
On Mon, Dec 17, 2018 at 2:20 PM Sean Christopherson wrote: > > My brain is still sorting out the details, but I generally like the idea > of allocating an anon inode when creating an enclave, and exposing the > other ioctls() via the returned fd. This is essentially the approach > used by KVM

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Andy Lutomirski
On Mon, Dec 17, 2018 at 5:39 PM Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 02:20:48PM -0800, Sean Christopherson wrote: > > The only potential hiccup I can see is the build flow. Currently, > > EADD+EEXTEND is done via a work queue to avoid major performance issues > > (10x regression)

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Tue, Dec 18, 2018 at 03:39:18AM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 02:20:48PM -0800, Sean Christopherson wrote: > > The only potential hiccup I can see is the build flow. Currently, > > EADD+EEXTEND is done via a work queue to avoid major performance issues > > (10x

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 12:10:17PM -0800, Andy Lutomirski wrote: > On Mon, Dec 17, 2018 at 12:03 PM Dave Hansen wrote: > > > > On 12/17/18 11:55 AM, Andy Lutomirski wrote: > > >> You're effectively rebuilding reverse-mapping infrastructure here. It's > > >> a frequent thing for the core VM to

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 02:20:48PM -0800, Sean Christopherson wrote: > The only potential hiccup I can see is the build flow. Currently, > EADD+EEXTEND is done via a work queue to avoid major performance issues > (10x regression) when userspace is building multiple enclaves in parallel > using

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Tue, Dec 18, 2018 at 03:17:25AM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 11:12:21AM -0800, Andy Lutomirski wrote: > > I'm going to ask an obnoxious high-level question: why does an enclave > > even refer to a specific mm? > > The reason is that it has not been yet in focus in

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 11:12:21AM -0800, Andy Lutomirski wrote: > I'm going to ask an obnoxious high-level question: why does an enclave > even refer to a specific mm? The reason is that it has not been yet in focus in the review process and there has been other concerns. At least the code is

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Sean Christopherson
On Mon, Dec 17, 2018 at 12:15:47PM -0800, Dave Hansen wrote: > On 12/17/18 12:10 PM, Andy Lutomirski wrote: > >> There's no 'struct page' for enclave memory as it stands. That means no > >> page cache, and that means there's no 'struct address_space *mapping' in > >> the first place. > >> > >>

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Sean Christopherson
On Mon, Dec 17, 2018 at 11:12:21AM -0800, Andy Lutomirski wrote: > On Mon, Dec 17, 2018 at 10:47 AM Dave Hansen wrote: > > > > On 12/17/18 10:43 AM, Jarkko Sakkinen wrote: > > > On Mon, Dec 17, 2018 at 10:36:13AM -0800, Sean Christopherson wrote: > > >> I'm pretty sure doing mmget() would result

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 09:33:22PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 10:48:58AM -0800, Sean Christopherson wrote: > > On Mon, Dec 17, 2018 at 08:43:33PM +0200, Jarkko Sakkinen wrote: > > > On Mon, Dec 17, 2018 at 10:36:13AM -0800, Sean Christopherson wrote: > > > > I'm pretty

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
On 12/17/18 12:10 PM, Andy Lutomirski wrote: >> There's no 'struct page' for enclave memory as it stands. That means no >> page cache, and that means there's no 'struct address_space *mapping' in >> the first place. >> >> Basically, the choice was made a long time ago to have SGX's memory >>

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Andy Lutomirski
On Mon, Dec 17, 2018 at 12:03 PM Dave Hansen wrote: > > On 12/17/18 11:55 AM, Andy Lutomirski wrote: > >> You're effectively rebuilding reverse-mapping infrastructure here. It's > >> a frequent thing for the core VM to need to go from 'struct page' back > >> to the page tables mapping it. For

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
On 12/17/18 11:55 AM, Andy Lutomirski wrote: >> You're effectively rebuilding reverse-mapping infrastructure here. It's >> a frequent thing for the core VM to need to go from 'struct page' back >> to the page tables mapping it. For that we go (logically) >>

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Andy Lutomirski
On Mon, Dec 17, 2018 at 11:53 AM Dave Hansen wrote: > > On 12/17/18 11:49 AM, Jarkko Sakkinen wrote: > >> Yeah, the code is built to have one VMA and only one VMA per enclave. > >> You need to go over the origin of this restriction and what enforces this. > > It is before ECREATE but after that

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 11:25:47AM -0800, Andy Lutomirski wrote: > On Mon, Dec 17, 2018 at 11:17 AM Dave Hansen wrote: > > > > On 12/17/18 11:12 AM, Andy Lutomirski wrote: > > > So I'm not saying that you shouldn't do it the way you are now, but I > > > do think that the changelog or at least

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
On 12/17/18 11:49 AM, Jarkko Sakkinen wrote: >> Yeah, the code is built to have one VMA and only one VMA per enclave. >> You need to go over the origin of this restriction and what enforces this. > It is before ECREATE but after that you can split it with mprotect(). > > Lets take an example. I'm

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 11:17:49AM -0800, Dave Hansen wrote: > On 12/17/18 11:12 AM, Andy Lutomirski wrote: > > So I'm not saying that you shouldn't do it the way you are now, but I > > do think that the changelog or at least some emails should explain > > *why* the enclave needs to keep a pointer

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
On 12/17/18 11:37 AM, Jarkko Sakkinen wrote: >> Suggestion: >> >> It looks like you only expect one VMA per enclave. Things go bonkers if >> this is not true. So, instead of storing encl->mm, don't. You can get >> the mm from vma->vm_mm and you could just store encl->vma instead. > The code

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 11:09:33AM -0800, Dave Hansen wrote: > On 12/17/18 10:48 AM, Sean Christopherson wrote: > > We can't set mm to NULL as we need it to unregister the notifier, and > > I'm fairly certain attempting to unregister in the release callback > > will deadlock. > > Suggestion: > >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 10:46:25AM -0800, Sean Christopherson wrote: > On Mon, Dec 17, 2018 at 08:23:19PM +0200, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 10:09:57AM -0800, Sean Christopherson wrote: > > > No, EREMOVE should never fail if the enclave is being released, i.e. all > > >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 10:48:58AM -0800, Sean Christopherson wrote: > On Mon, Dec 17, 2018 at 08:43:33PM +0200, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 10:36:13AM -0800, Sean Christopherson wrote: > > > I'm pretty sure doing mmget() would result in circular dependencies and > > > a

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Andy Lutomirski
On Mon, Dec 17, 2018 at 11:17 AM Dave Hansen wrote: > > On 12/17/18 11:12 AM, Andy Lutomirski wrote: > > So I'm not saying that you shouldn't do it the way you are now, but I > > do think that the changelog or at least some emails should explain > > *why* the enclave needs to keep a pointer to

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
On 12/17/18 11:12 AM, Andy Lutomirski wrote: > So I'm not saying that you shouldn't do it the way you are now, but I > do think that the changelog or at least some emails should explain > *why* the enclave needs to keep a pointer to the creating process's > mm. And, if you do keep the current

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Andy Lutomirski
On Mon, Dec 17, 2018 at 10:47 AM Dave Hansen wrote: > > On 12/17/18 10:43 AM, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 10:36:13AM -0800, Sean Christopherson wrote: > >> I'm pretty sure doing mmget() would result in circular dependencies and > >> a zombie enclave. In the do_exit() case

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
On 12/17/18 10:48 AM, Sean Christopherson wrote: > We can't set mm to NULL as we need it to unregister the notifier, and > I'm fairly certain attempting to unregister in the release callback > will deadlock. Suggestion: It looks like you only expect one VMA per enclave. Things go bonkers if

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Sean Christopherson
On Mon, Dec 17, 2018 at 08:43:33PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 10:36:13AM -0800, Sean Christopherson wrote: > > I'm pretty sure doing mmget() would result in circular dependencies and > > a zombie enclave. In the do_exit() case where a task is abruptly killed: > > >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
On 12/17/18 10:43 AM, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 10:36:13AM -0800, Sean Christopherson wrote: >> I'm pretty sure doing mmget() would result in circular dependencies and >> a zombie enclave. In the do_exit() case where a task is abruptly killed: >> >> - __mmput() is never

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Sean Christopherson
On Mon, Dec 17, 2018 at 08:23:19PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 10:09:57AM -0800, Sean Christopherson wrote: > > No, EREMOVE should never fail if the enclave is being released, i.e. all > > references to the enclave are gone. And failure during sgx_encl_release() > >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 10:36:13AM -0800, Sean Christopherson wrote: > I'm pretty sure doing mmget() would result in circular dependencies and > a zombie enclave. In the do_exit() case where a task is abruptly killed: > > - __mmput() is never called because the enclave holds a ref > -

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Sean Christopherson
On Mon, Dec 17, 2018 at 08:01:02PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 09:45:40AM -0800, Dave Hansen wrote: > > > +struct sgx_encl *sgx_encl_alloc(struct sgx_secs *secs) > > > +{ > > ... > > > + kref_init(>refcount); > > > + INIT_LIST_HEAD(>add_page_reqs); > > > +

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 10:07:08AM -0800, Dave Hansen wrote: > On 12/17/18 10:01 AM, Jarkko Sakkinen wrote: > >>> + encl->mm = current->mm; <-> + > >>> encl->base = secs->base; > >>> + encl->size = secs->size; > >>> + encl->ssaframesize = secs->ssa_frame_size; >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 10:09:57AM -0800, Sean Christopherson wrote: > No, EREMOVE should never fail if the enclave is being released, i.e. all > references to the enclave are gone. And failure during sgx_encl_release() > means we leaked an EPC page, which warrants a WARN. Right that what I was

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Sean Christopherson
On Mon, Dec 17, 2018 at 07:49:35PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 09:31:06AM -0800, Sean Christopherson wrote: > > This doesn't work as-is. sgx_encl_release() needs to use sgx_free_page() > > and not __sgx_free_page() so that we get a WARN() if the page can't be > >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
On 12/17/18 10:01 AM, Jarkko Sakkinen wrote: >>> + encl->mm = current->mm; <-> + >>> encl->base = secs->base; >>> + encl->size = secs->size; >>> + encl->ssaframesize = secs->ssa_frame_size; >>> + encl->backing = backing; >>> + >>> + return encl; >>> +}

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 09:45:40AM -0800, Dave Hansen wrote: > > +struct sgx_encl *sgx_encl_alloc(struct sgx_secs *secs) > > +{ > ... > > + kref_init(>refcount); > > + INIT_LIST_HEAD(>add_page_reqs); > > + INIT_RADIX_TREE(>page_tree, GFP_KERNEL); > > + mutex_init(>lock); > > +

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 09:31:06AM -0800, Sean Christopherson wrote: > This doesn't work as-is. sgx_encl_release() needs to use sgx_free_page() > and not __sgx_free_page() so that we get a WARN() if the page can't be > freed. sgx_invalidate() needs to use __sgx_free_page() as freeing a page >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dave Hansen
> +struct sgx_encl *sgx_encl_alloc(struct sgx_secs *secs) > +{ ... > + kref_init(>refcount); > + INIT_LIST_HEAD(>add_page_reqs); > + INIT_RADIX_TREE(>page_tree, GFP_KERNEL); > + mutex_init(>lock); > + INIT_WORK(>add_page_work, sgx_add_page_worker); > + > + encl->mm =

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Sean Christopherson
On Mon, Dec 17, 2018 at 04:08:11PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 03:39:28PM +0200, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 03:28:59PM +0200, Jarkko Sakkinen wrote: > > > On Fri, Dec 14, 2018 at 04:06:27PM -0800, Sean Christopherson wrote: > > > > [ 504.149548]

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Dr. Greg
On Mon, Dec 17, 2018 at 04:13:15PM +0200, Jarkko Sakkinen wrote: Good morning to everyone. > On Mon, Dec 17, 2018 at 04:08:11PM +0200, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 03:39:28PM +0200, Jarkko Sakkinen wrote: > > > On Mon, Dec 17, 2018 at 03:28:59PM +0200, Jarkko Sakkinen

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Sean Christopherson
On Sat, Dec 15, 2018 at 05:22:31PM -0600, Dr. Greg wrote: > On Fri, Dec 14, 2018 at 04:06:27PM -0800, Sean Christopherson wrote: > > Good afternoon, I hope the weekend is going well for everyone. > > > On Fri, Dec 14, 2018 at 05:59:17PM -0600, Dr. Greg wrote: > > > On Wed, Dec 12, 2018 at

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 04:08:11PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 03:39:28PM +0200, Jarkko Sakkinen wrote: > > On Mon, Dec 17, 2018 at 03:28:59PM +0200, Jarkko Sakkinen wrote: > > > On Fri, Dec 14, 2018 at 04:06:27PM -0800, Sean Christopherson wrote: > > > > [ 504.149548]

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 03:39:28PM +0200, Jarkko Sakkinen wrote: > On Mon, Dec 17, 2018 at 03:28:59PM +0200, Jarkko Sakkinen wrote: > > On Fri, Dec 14, 2018 at 04:06:27PM -0800, Sean Christopherson wrote: > > > [ 504.149548] [ cut here ] > > > [ 504.149550] kernel BUG at

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Mon, Dec 17, 2018 at 03:28:59PM +0200, Jarkko Sakkinen wrote: > On Fri, Dec 14, 2018 at 04:06:27PM -0800, Sean Christopherson wrote: > > [ 504.149548] [ cut here ] > > [ 504.149550] kernel BUG at > > /home/sean/go/src/kernel.org/linux/mm/mmap.c:669! > > [ 504.150288]

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-17 Thread Jarkko Sakkinen
On Fri, Dec 14, 2018 at 04:06:27PM -0800, Sean Christopherson wrote: > [ 504.149548] [ cut here ] > [ 504.149550] kernel BUG at /home/sean/go/src/kernel.org/linux/mm/mmap.c:669! > [ 504.150288] invalid opcode: [#1] SMP > [ 504.150614] CPU: 2 PID: 237 Comm:

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-15 Thread Dr. Greg
On Fri, Dec 14, 2018 at 04:06:27PM -0800, Sean Christopherson wrote: Good afternoon, I hope the weekend is going well for everyone. > On Fri, Dec 14, 2018 at 05:59:17PM -0600, Dr. Greg wrote: > > On Wed, Dec 12, 2018 at 08:00:36PM +0200, Jarkko Sakkinen wrote: > > > > Good evening, I hope the

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-14 Thread Sean Christopherson
On Fri, Dec 14, 2018 at 05:59:17PM -0600, Dr. Greg wrote: > On Wed, Dec 12, 2018 at 08:00:36PM +0200, Jarkko Sakkinen wrote: > > Good evening, I hope the week has gone well for everyone. > > > On Mon, Dec 10, 2018 at 04:49:08AM -0600, Dr. Greg wrote: > > > In the meantime, I wanted to confirm

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-14 Thread Dr. Greg
On Wed, Dec 12, 2018 at 08:00:36PM +0200, Jarkko Sakkinen wrote: Good evening, I hope the week has gone well for everyone. > On Mon, Dec 10, 2018 at 04:49:08AM -0600, Dr. Greg wrote: > > In the meantime, I wanted to confirm that your jarkko-sgx/master > > branch contains the proposed driver that

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-12 Thread Jarkko Sakkinen
On Mon, Dec 10, 2018 at 04:49:08AM -0600, Dr. Greg wrote: > In the meantime, I wanted to confirm that your jarkko-sgx/master > branch contains the proposed driver that is headed upstream. Before > adding the SFLC patches we thought it best to run the driver through > some testing in order to

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-10 Thread Dr. Greg
On Sun, Dec 09, 2018 at 06:01:32PM +0100, Pavel Machek wrote: > Hi! Good morning to everyone. > > On Thu, Nov 15, 2018 at 5:08 PM Jarkko Sakkinen > > wrote: > > > > > > Intel Software Guard eXtensions (SGX) is a set of CPU instructions that > > > can be used by applications to set aside

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-10 Thread Dr. Greg
On Wed, Nov 28, 2018 at 11:22:28AM -0800, Jarkko Sakkinen wrote: Good morning, I hope everyone had a pleasant weekend. > On Wed, Nov 28, 2018 at 04:49:41AM -0600, Dr. Greg wrote: > > We've been carrying a patch, that drops in on top of the proposed > > kernel driver, that implements the needed

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-09 Thread Pavel Machek
Hi! > On Thu, Nov 15, 2018 at 5:08 PM Jarkko Sakkinen > wrote: > > > > Intel Software Guard eXtensions (SGX) is a set of CPU instructions that > > can be used by applications to set aside private regions of code and > > data. The code outside the enclave is disallowed to access the memory > >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-12-09 Thread Pavel Machek
Hi! > > There would be three types of users: > > > > 1. Ones that have access to neither of the devices. > > 2. Ones that have access to unprivileged. Who are these? > > Either 0666 (world) or an sgx group. Sgx group, please. Or even better, what is generic term for sgx? We probably want to use

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-28 Thread Andy Lutomirski
On Tue, Nov 27, 2018 at 12:55 AM Dr. Greg wrote: > Since the thread has become a bit divergent I wanted to note that we > have offered a proposal for a general policy management framework > based on MRSIGNER values. This framework is consistent with the SGX > security model, ie. cryptographic

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-28 Thread Andy Lutomirski
On Tue, Nov 27, 2018 at 12:55 AM Dr. Greg wrote: > Since the thread has become a bit divergent I wanted to note that we > have offered a proposal for a general policy management framework > based on MRSIGNER values. This framework is consistent with the SGX > security model, ie. cryptographic

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-28 Thread Jarkko Sakkinen
On Wed, Nov 28, 2018 at 04:49:41AM -0600, Dr. Greg wrote: > We've been carrying a patch, that drops in on top of the proposed > kernel driver, that implements the needed policy management framework > for DAC fragile (FLC) platforms. After a meeting yesterday with the > client that is funding the

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-28 Thread Jarkko Sakkinen
On Wed, Nov 28, 2018 at 04:49:41AM -0600, Dr. Greg wrote: > We've been carrying a patch, that drops in on top of the proposed > kernel driver, that implements the needed policy management framework > for DAC fragile (FLC) platforms. After a meeting yesterday with the > client that is funding the

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-28 Thread Dr. Greg
On Tue, Nov 27, 2018 at 09:55:45AM -0800, Andy Lutomirski wrote: > > On Nov 27, 2018, at 8:41 AM, Jarkko Sakkinen > > wrote: > > > >> On Tue, Nov 27, 2018 at 02:55:33AM -0600, Dr. Greg wrote: > >> Since the thread has become a bit divergent I wanted to note that we > >> have offered a proposal

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-28 Thread Dr. Greg
On Tue, Nov 27, 2018 at 09:55:45AM -0800, Andy Lutomirski wrote: > > On Nov 27, 2018, at 8:41 AM, Jarkko Sakkinen > > wrote: > > > >> On Tue, Nov 27, 2018 at 02:55:33AM -0600, Dr. Greg wrote: > >> Since the thread has become a bit divergent I wanted to note that we > >> have offered a proposal

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Jarkko Sakkinen
On Sat, Nov 24, 2018 at 08:45:34AM -0800, Jarkko Sakkinen wrote: > On Fri, Nov 23, 2018 at 04:39:23AM -0600, Dr. Greg wrote: > > Jarkko, when this driver lands it will set the SGX ABI in stone for > > Linux. It would be very, very helpful to the development community if > > there was some

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Jarkko Sakkinen
On Sat, Nov 24, 2018 at 08:45:34AM -0800, Jarkko Sakkinen wrote: > On Fri, Nov 23, 2018 at 04:39:23AM -0600, Dr. Greg wrote: > > Jarkko, when this driver lands it will set the SGX ABI in stone for > > Linux. It would be very, very helpful to the development community if > > there was some

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Andy Lutomirski
> On Nov 27, 2018, at 8:41 AM, Jarkko Sakkinen > wrote: > >> On Tue, Nov 27, 2018 at 02:55:33AM -0600, Dr. Greg wrote: >> Since the thread has become a bit divergent I wanted to note that we >> have offered a proposal for a general policy management framework >> based on MRSIGNER values.

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Andy Lutomirski
> On Nov 27, 2018, at 8:41 AM, Jarkko Sakkinen > wrote: > >> On Tue, Nov 27, 2018 at 02:55:33AM -0600, Dr. Greg wrote: >> Since the thread has become a bit divergent I wanted to note that we >> have offered a proposal for a general policy management framework >> based on MRSIGNER values.

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Jarkko Sakkinen
On Tue, Nov 27, 2018 at 02:55:33AM -0600, Dr. Greg wrote: > 3.) Enclaves with the SGX_FLAGS_LICENSE_KEY attribute set - i.e., 'Launch > Enclaves'. Kernel does not have to manage this. If the MSRs are read-only, they should match your LE. If the MSRs writable, you don't need an LE. This whole

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Jarkko Sakkinen
On Tue, Nov 27, 2018 at 02:55:33AM -0600, Dr. Greg wrote: > 3.) Enclaves with the SGX_FLAGS_LICENSE_KEY attribute set - i.e., 'Launch > Enclaves'. Kernel does not have to manage this. If the MSRs are read-only, they should match your LE. If the MSRs writable, you don't need an LE. This whole

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Jarkko Sakkinen
On Tue, Nov 27, 2018 at 02:55:33AM -0600, Dr. Greg wrote: > Since the thread has become a bit divergent I wanted to note that we > have offered a proposal for a general policy management framework > based on MRSIGNER values. This framework is consistent with the SGX > security model, ie.

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Jarkko Sakkinen
On Tue, Nov 27, 2018 at 02:55:33AM -0600, Dr. Greg wrote: > Since the thread has become a bit divergent I wanted to note that we > have offered a proposal for a general policy management framework > based on MRSIGNER values. This framework is consistent with the SGX > security model, ie.

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Dr. Greg
On Mon, Nov 26, 2018 at 03:04:36PM -0800, Jarkko Sakkinen wrote: Good morning to everyone. > On Mon, Nov 26, 2018 at 01:51:45PM -0800, Jarkko Sakkinen wrote: > > > ioctl(sgx, SGX_IOC_ADD_RIGHT, sgx_provisioning); > > > > > > This requires extra syscalls, but it doesn???t have the combinatorial

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-27 Thread Dr. Greg
On Mon, Nov 26, 2018 at 03:04:36PM -0800, Jarkko Sakkinen wrote: Good morning to everyone. > On Mon, Nov 26, 2018 at 01:51:45PM -0800, Jarkko Sakkinen wrote: > > > ioctl(sgx, SGX_IOC_ADD_RIGHT, sgx_provisioning); > > > > > > This requires extra syscalls, but it doesn???t have the combinatorial

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Mon, Nov 26, 2018 at 01:51:45PM -0800, Jarkko Sakkinen wrote: > > ioctl(sgx, SGX_IOC_ADD_RIGHT, sgx_provisioning); > > > > This requires extra syscalls, but it doesn’t have the combinatorial > > explosion problem. > > I like this design because it is extendable. I'm now also in the same >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Mon, Nov 26, 2018 at 01:51:45PM -0800, Jarkko Sakkinen wrote: > > ioctl(sgx, SGX_IOC_ADD_RIGHT, sgx_provisioning); > > > > This requires extra syscalls, but it doesn’t have the combinatorial > > explosion problem. > > I like this design because it is extendable. I'm now also in the same >

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Mon, Nov 26, 2018 at 05:00:39AM -0600, Dr. Greg wrote: > We will be interested in your comments as to why the proposal is > insufficient in the real world of FLC. > > I believe the proposed architecture can be defended as being effective > in the real world, as it allows the root user to use

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Mon, Nov 26, 2018 at 05:00:39AM -0600, Dr. Greg wrote: > We will be interested in your comments as to why the proposal is > insufficient in the real world of FLC. > > I believe the proposed architecture can be defended as being effective > in the real world, as it allows the root user to use

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Sun, Nov 25, 2018 at 08:22:35AM -0800, Andy Lutomirski wrote: > Agreed. What I’m proposing adds additional security if the kernel is > *not* compromised. And even if the kernel is compromised evil use will detected quicker i.e. compromissed kernel is "better" than a kernel that allows to use

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Sun, Nov 25, 2018 at 08:22:35AM -0800, Andy Lutomirski wrote: > Agreed. What I’m proposing adds additional security if the kernel is > *not* compromised. And even if the kernel is compromised evil use will detected quicker i.e. compromissed kernel is "better" than a kernel that allows to use

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Sat, Nov 24, 2018 at 02:13:18PM -0600, Dr. Greg wrote: > This isn't about an enclave being able to tell that it is really an > enclave. As I noted in my previous reply, access to the provisioning > bit allows an enclave author to create a perpetual hardware identifier > for a platform based on

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Sat, Nov 24, 2018 at 02:13:18PM -0600, Dr. Greg wrote: > This isn't about an enclave being able to tell that it is really an > enclave. As I noted in my previous reply, access to the provisioning > bit allows an enclave author to create a perpetual hardware identifier > for a platform based on

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Sat, Nov 24, 2018 at 01:24:54PM -0600, Dr. Greg wrote: > On Sat, Nov 24, 2018 at 08:15:21AM -0800, Jarkko Sakkinen wrote: > > > On Tue, Nov 20, 2018 at 05:15:08AM -0600, Dr. Greg wrote: > > > Malware would not necessarily need the Intel attestation service. > > > Once access to the PROVISION

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Jarkko Sakkinen
On Sat, Nov 24, 2018 at 01:24:54PM -0600, Dr. Greg wrote: > On Sat, Nov 24, 2018 at 08:15:21AM -0800, Jarkko Sakkinen wrote: > > > On Tue, Nov 20, 2018 at 05:15:08AM -0600, Dr. Greg wrote: > > > Malware would not necessarily need the Intel attestation service. > > > Once access to the PROVISION

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Andy Lutomirski
On Mon, Nov 26, 2018 at 3:00 AM Dr. Greg wrote: > > On Sun, Nov 25, 2018 at 04:37:00PM -0800, Andy Lutomirski wrote: > > > Bah, I hit send on a partially written draft. I???ll try again soon. > > > > --Andy > > Your first issue seems to be complete so I will respond to that in > order to make

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Andy Lutomirski
On Mon, Nov 26, 2018 at 3:00 AM Dr. Greg wrote: > > On Sun, Nov 25, 2018 at 04:37:00PM -0800, Andy Lutomirski wrote: > > > Bah, I hit send on a partially written draft. I???ll try again soon. > > > > --Andy > > Your first issue seems to be complete so I will respond to that in > order to make

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Dr. Greg
On Sun, Nov 25, 2018 at 04:37:00PM -0800, Andy Lutomirski wrote: > Bah, I hit send on a partially written draft. I???ll try again soon. > > --Andy Your first issue seems to be complete so I will respond to that in order to make sure we are not talking past one another. > > On Nov 25, 2018, at

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-26 Thread Dr. Greg
On Sun, Nov 25, 2018 at 04:37:00PM -0800, Andy Lutomirski wrote: > Bah, I hit send on a partially written draft. I???ll try again soon. > > --Andy Your first issue seems to be complete so I will respond to that in order to make sure we are not talking past one another. > > On Nov 25, 2018, at

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-25 Thread Andy Lutomirski
Bah, I hit send on a partially written draft. I’ll try again soon. --Andy > On Nov 25, 2018, at 1:59 PM, Andy Lutomirski wrote: > > > >> On Nov 25, 2018, at 10:55 AM, Dr. Greg wrote: >> > >> >> >> The notion of a launch enclave is critical to establishing these >> guarantees. As soon

Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

2018-11-25 Thread Andy Lutomirski
Bah, I hit send on a partially written draft. I’ll try again soon. --Andy > On Nov 25, 2018, at 1:59 PM, Andy Lutomirski wrote: > > > >> On Nov 25, 2018, at 10:55 AM, Dr. Greg wrote: >> > >> >> >> The notion of a launch enclave is critical to establishing these >> guarantees. As soon

  1   2   >