linux-sgx.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	x86@kernel.org, linux-sgx@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Jethro Beekman <jethro@fortanix.com>,
	Haitao Huang <haitao.huang@linux.intel.com>,
	Chunyang Hui <sanqian.hcy@antfin.com>,
	Jordan Hand <jorhand@linux.microsoft.com>,
	Nathaniel McCallum <npmccallum@redhat.com>,
	Seth Moore <sethmo@google.com>,
	Darren Kenny <darren.kenny@oracle.com>,
	Suresh Siddha <suresh.b.siddha@intel.com>,
	akpm@linux-foundation.org, andriy.shevchenko@linux.intel.com,
	asapek@google.com, bp@alien8.de, cedric.xing@intel.com,
	chenalexchen@google.com, conradparker@google.com,
	cyhanish@google.com, haitao.huang@intel.com, kai.huang@intel.com,
	kai.svahn@intel.com, kmoy@google.com, ludloff@google.com,
	luto@kernel.org, nhorman@redhat.com, puiterwijk@redhat.com,
	rientjes@google.com, tglx@linutronix.de, yaozhangx@google.com,
	mikko.ylinen@intel.com
Subject: Re: [PATCH v39 12/24] x86/sgx: Add SGX_IOC_ENCLAVE_CREATE
Date: Mon, 19 Oct 2020 13:48:50 -0700	[thread overview]
Message-ID: <20201019204848.GB23072@linux.intel.com> (raw)
In-Reply-To: <f6fe37ee-7b5c-15b6-6823-2500582e7921@intel.com>

On Mon, Oct 19, 2020 at 01:21:09PM -0700, Dave Hansen wrote:
> On 10/17/20 9:26 PM, Jarkko Sakkinen wrote:
> >>> +long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> >>> +{
> >>> +	struct sgx_encl *encl = filep->private_data;
> >>> +	int ret, encl_flags;
> >>> +
> >>> +	encl_flags = atomic_fetch_or(SGX_ENCL_IOCTL, &encl->flags);
> >>> +	if (encl_flags & SGX_ENCL_IOCTL)
> >>> +		return -EBUSY;
> >>
> >> Is the SGX_ENCL_IOCTL bit essentially just a lock to single-thread
> >> ioctl()s?  Should we name it as such?
> > 
> > Yes. It makes the concurrency overally easier if we can assume that
> > only a single ioctl is in progress. There is no good reason to do
> > them in parallel.
> > 
> > E.g. when you add pages you want to do that serially because the
> > order changes the MRENCLAVE.
> 
> There are also hardware concurrency requirements, right?  A bunch of the
> SGX functions seem not not even support being called in parallel.

Yes, and the driver, even when "holding" SGX_ENCL_IOCTL, takes encl->lock
when executing an ENCLS leaf.  The separate IOCTL flag avoids complications
with reclaim, specifically it allows the ioctls to initiate reclaim without
hitting a deadlock.

Reclaim needs to take encl->lock, e.g. to do ENCLS[EBLOCK], and reclaim is by
default initiated during allocation if there are no pages available.  I.e. if
an ioctl() simply held encl->lock, it would deadlock in the scenario where it
triggered reclaim on the current enclave.

In other words, the flag is necessary even if it weren't being used a lock
primitive, e.g. it'd still need to exist even if encl->lock were taken to set
and check the flag.  The atomic shenanigans were added as an optimization to
allow reclaim in parallel with the bulk of the ioctl flows, and partly because
using atomic_fetch_or() avoided having to drop encl->lock in an error flow,
i.e. yielded less code.

> > So should I rename it as SGX_ENCL_IOCTL_LOCKED?
> 
> I'd rather not see hand-rolled locking primitives frankly.

IOCTL_IN_PROGRESS would be my vote if we want a more descriptive name.

  reply	other threads:[~2020-10-19 20:48 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-03  4:50 [PATCH v39 00/24] Intel SGX foundations Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 01/24] x86/cpufeatures: x86/msr: Add Intel SGX hardware bits Jarkko Sakkinen
2020-10-19 14:10   ` Dave Hansen
2020-10-19 17:49     ` Sean Christopherson
2020-10-03  4:50 ` [PATCH v39 02/24] x86/cpufeatures: x86/msr: Add Intel SGX Launch Control " Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 03/24] x86/mm: x86/sgx: Signal SIGSEGV with PF_SGX Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 04/24] x86/sgx: Add SGX microarchitectural data structures Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 05/24] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen
2020-10-19 14:30   ` Dave Hansen
2020-10-19 17:38     ` Sean Christopherson
2020-10-19 17:48       ` Dave Hansen
2020-10-19 17:53         ` Sean Christopherson
2020-10-19 17:58           ` Dave Hansen
2020-10-03  4:50 ` [PATCH v39 06/24] x86/cpu/intel: Detect SGX support Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 07/24] x86/cpu/intel: Add nosgx kernel parameter Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 08/24] x86/sgx: Initialize metadata for Enclave Page Cache (EPC) sections Jarkko Sakkinen
2020-10-19  8:45   ` Jarkko Sakkinen
2020-10-19 12:39     ` Borislav Petkov
2020-10-23  9:01       ` Jarkko Sakkinen
2020-10-19 13:40     ` Dave Hansen
2020-10-23  9:03       ` Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 09/24] x86/sgx: Add __sgx_alloc_epc_page() and sgx_free_epc_page() Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 10/24] mm: Add 'mprotect' hook to struct vm_operations_struct Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 11/24] x86/sgx: Add SGX enclave driver Jarkko Sakkinen
2020-10-03 14:39   ` Greg KH
2020-10-04 14:32     ` Jarkko Sakkinen
2020-10-04 15:01       ` Jarkko Sakkinen
2020-10-05  9:42       ` Greg KH
2020-10-05 12:42         ` Jarkko Sakkinen
2020-10-07 18:09           ` Haitao Huang
2020-10-07 19:26             ` Greg KH
2020-10-09  6:44               ` Jarkko Sakkinen
2020-10-14 20:16                 ` Dave Hansen
2020-10-05  8:45     ` Christoph Hellwig
2020-10-05 11:42       ` Jarkko Sakkinen
2020-10-05 11:50         ` Greg KH
2020-10-05 14:23           ` Jarkko Sakkinen
2020-10-05 15:02             ` Greg KH
2020-10-05 16:40               ` Dave Hansen
2020-10-05 20:02                 ` Jarkko Sakkinen
2020-10-09  7:10     ` Pavel Machek
2020-10-09  7:21       ` Greg KH
2020-10-09  8:21         ` Pavel Machek
2020-10-03 19:54   ` Matthew Wilcox
2020-10-04 21:50     ` Jarkko Sakkinen
2020-10-04 22:02       ` Jarkko Sakkinen
2020-10-04 22:27       ` Matthew Wilcox
2020-10-04 23:41         ` Jarkko Sakkinen
2020-10-05  1:30           ` Matthew Wilcox
2020-10-05  3:06             ` Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 12/24] x86/sgx: Add SGX_IOC_ENCLAVE_CREATE Jarkko Sakkinen
2020-10-16 17:07   ` Dave Hansen
2020-10-18  4:26     ` Jarkko Sakkinen
2020-10-19 20:21       ` Dave Hansen
2020-10-19 20:48         ` Sean Christopherson [this message]
2020-10-03  4:50 ` [PATCH v39 13/24] x86/sgx: Add SGX_IOC_ENCLAVE_ADD_PAGES Jarkko Sakkinen
2020-10-16 21:25   ` Dave Hansen
2020-10-18  5:03     ` Jarkko Sakkinen
2020-10-19  7:03       ` Jarkko Sakkinen
2020-10-19 20:48       ` Dave Hansen
2020-10-19 21:15         ` Sean Christopherson
2020-10-19 21:44           ` Dave Hansen
2020-10-23 10:11             ` Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 14/24] x86/sgx: Add SGX_IOC_ENCLAVE_INIT Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 15/24] x86/sgx: Add SGX_IOC_ENCLAVE_PROVISION Jarkko Sakkinen
2020-10-20 15:48   ` Dave Hansen
2020-10-23 10:14     ` Jarkko Sakkinen
2020-10-20 21:19   ` Dave Hansen
2020-10-23 10:17     ` Jarkko Sakkinen
2020-10-23 14:19       ` Dave Hansen
2020-10-24 11:34         ` Jarkko Sakkinen
2020-10-24 15:47           ` Andy Lutomirski
2020-10-24 20:23             ` Jarkko Sakkinen
2020-10-27 10:38               ` Dr. Greg
2020-10-23 14:23       ` Jethro Beekman
2020-10-24 11:40         ` Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 16/24] x86/sgx: Add a page reclaimer Jarkko Sakkinen
2020-10-03  5:22   ` Haitao Huang
2020-10-03 13:32     ` Jarkko Sakkinen
2020-10-03 18:23       ` Haitao Huang
2020-10-04 22:39         ` Jarkko Sakkinen
2020-10-07 17:25           ` Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 17/24] x86/sgx: Add ptrace() support for the SGX driver Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 18/24] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 19/24] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 20/24] x86/traps: Attempt to fixup exceptions in vDSO before signaling Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 21/24] x86/vdso: Implement a vDSO for Intel SGX enclave call Jarkko Sakkinen
2020-10-06  2:57   ` Sean Christopherson
2020-10-06  8:30     ` Jethro Beekman
2020-10-06 15:15       ` Sean Christopherson
2020-10-06 17:28         ` Jarkko Sakkinen
2020-10-06 23:21           ` Sean Christopherson
2020-10-07  0:22             ` Jarkko Sakkinen
2020-10-07  1:17               ` Sean Christopherson
2020-10-07  3:14                 ` Jarkko Sakkinen
2020-10-07  4:34                   ` Sean Christopherson
2020-10-07  7:39                     ` Jarkko Sakkinen
2020-10-07  8:04                       ` Jarkko Sakkinen
2020-10-07 15:25                       ` Sean Christopherson
2020-10-07 17:08                         ` Jarkko Sakkinen
2020-10-07 17:13                           ` Jarkko Sakkinen
2020-10-06 15:49       ` Jarkko Sakkinen
2020-10-06 15:36     ` Jarkko Sakkinen
2020-10-06 21:39     ` Jarkko Sakkinen
2020-10-07  0:23       ` Jarkko Sakkinen
2020-10-17  1:48   ` Andy Lutomirski
2020-10-17 21:02     ` Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 22/24] selftests/x86: Add a selftest for SGX Jarkko Sakkinen
2020-10-12 16:50   ` Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 23/24] docs: x86/sgx: Document SGX micro architecture and kernel internals Jarkko Sakkinen
2020-10-03  4:50 ` [PATCH v39 24/24] x86/sgx: Update MAINTAINERS Jarkko Sakkinen
2020-10-16 21:04   ` Dave Hansen
2020-10-18  4:27     ` Jarkko Sakkinen
2020-10-03 14:32 ` [PATCH v39 00/24] Intel SGX foundations Greg KH
2020-10-03 14:53   ` Jarkko Sakkinen
2020-10-15 19:06 ` Dave Hansen
2020-10-17 20:43   ` 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=20201019204848.GB23072@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=asapek@google.com \
    --cc=bp@alien8.de \
    --cc=cedric.xing@intel.com \
    --cc=chenalexchen@google.com \
    --cc=conradparker@google.com \
    --cc=cyhanish@google.com \
    --cc=darren.kenny@oracle.com \
    --cc=dave.hansen@intel.com \
    --cc=haitao.huang@intel.com \
    --cc=haitao.huang@linux.intel.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jethro@fortanix.com \
    --cc=jorhand@linux.microsoft.com \
    --cc=kai.huang@intel.com \
    --cc=kai.svahn@intel.com \
    --cc=kmoy@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=ludloff@google.com \
    --cc=luto@kernel.org \
    --cc=mikko.ylinen@intel.com \
    --cc=nhorman@redhat.com \
    --cc=npmccallum@redhat.com \
    --cc=puiterwijk@redhat.com \
    --cc=rientjes@google.com \
    --cc=sanqian.hcy@antfin.com \
    --cc=sethmo@google.com \
    --cc=suresh.b.siddha@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yaozhangx@google.com \
    /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).