Linux-Sgx Archive on lore.kernel.org
 help / color / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: x86@kernel.org, linux-sgx@vger.kernel.org,
	akpm@linux-foundation.org, dave.hansen@intel.com,
	nhorman@redhat.com, npmccallum@redhat.com, serge.ayoun@intel.com,
	shay.katz-zamir@intel.com, haitao.huang@intel.com,
	andriy.shevchenko@linux.intel.com, tglx@linutronix.de,
	kai.svahn@intel.com, bp@alien8.de, josh@joshtriplett.org,
	luto@kernel.org, kai.huang@intel.com, rientjes@google.com,
	Suresh Siddha <suresh.b.siddha@intel.com>
Subject: Re: [PATCH v19 12/27] x86/sgx: Enumerate and track EPC sections
Date: Fri, 22 Mar 2019 12:50:58 +0200
Message-ID: <20190322105058.GE3122@linux.intel.com> (raw)
In-Reply-To: <20190322101940.GC3122@linux.intel.com>

On Fri, Mar 22, 2019 at 12:19:40PM +0200, Jarkko Sakkinen wrote:
> On Thu, Mar 21, 2019 at 08:28:10AM -0700, Sean Christopherson wrote:
> > On Thu, Mar 21, 2019 at 04:40:56PM +0200, Jarkko Sakkinen wrote:
> > > On Mon, Mar 18, 2019 at 12:50:43PM -0700, Sean Christopherson wrote:
> > > > On Sun, Mar 17, 2019 at 11:14:41PM +0200, Jarkko Sakkinen wrote:
> > > > Dynamically allocating sgx_epc_sections isn't exactly difficult, and
> > > > AFAICT the static allocation is the primary motivation for capping
> > > > SGX_MAX_EPC_SECTIONS at such a low value (8).  I still think it makes
> > > > sense to define SGX_MAX_EPC_SECTIONS so that the section number can
> > > > be embedded in the offset, along with flags.  But the max can be
> > > > significantly higher, e.g. using 7 bits to support 128 sections.
> > > > 
> > > 
> > > I don't disagree with you but I think for the existing and forseeable
> > > hardware this is good enough. Can be refined if there is ever need.
> > 
> > My concern is that there may be virtualization use cases that want to
> > expose more than 8 EPC sections to a guest.  I have no idea if this is
> > anything more than paranoia, but at the same time the cost to increase
> > support to 128+ sections is quite low.
> > 
> > > > I realize hardware is highly unlikely to have more than 8 sections, at
> > > > least for the near future, but IMO the small amount of extra complexity
> > > > is worth having a bit of breathing room.
> > > 
> > > Yup.
> > > 
> > > > > +static __init int sgx_init_epc_section(u64 addr, u64 size, unsigned long index,
> > > > > +				       struct sgx_epc_section *section)
> > > > > +{
> > > > > +	unsigned long nr_pages = size >> PAGE_SHIFT;
> > > > > +	struct sgx_epc_page *page;
> > > > > +	unsigned long i;
> > > > > +
> > > > > +	section->va = memremap(addr, size, MEMREMAP_WB);
> > > > > +	if (!section->va)
> > > > > +		return -ENOMEM;
> > > > > +
> > > > > +	section->pa = addr;
> > > > > +	spin_lock_init(&section->lock);
> > > > > +	INIT_LIST_HEAD(&section->page_list);
> > > > > +
> > > > > +	for (i = 0; i < nr_pages; i++) {
> > > > > +		page = kzalloc(sizeof(*page), GFP_KERNEL);
> > > > > +		if (!page)
> > > > > +			goto out;
> > > > > +		page->desc = (addr + (i << PAGE_SHIFT)) | index;
> > > > > +		sgx_section_put_page(section, page);
> > > > > +	}
> > > > 
> > > > Not sure if this is the correct location, but at some point the kernel
> > > > needs to sanitize the EPC during init.  EPC pages may be in an unknown
> > > > state, e.g. after kexec(), which will cause all manner of faults and
> > > > warnings.  Maybe the best approach is to sanitize on-demand, e.g. suppress
> > > > the first WARN due to unexpected ENCLS failure and purge the EPC at that
> > > > time.  The downside of that approach is that exposing EPC to a guest would
> > > > need to implement its own sanitization flow.
> > > 
> > > Hmm... Lets think this through. I'm just thinking how sanitization on
> > > demand would actually work given the parent-child relationships.
> > 
> > It's ugly.
> > 
> >   1. Temporarily disable EPC allocation and enclave fault handling
> >   2. Zap all TCS PTEs in all enclaves
> >   3. Flush all logical CPUs from enclaves via IPI
> >   4. Forcefully reclaim all EPC pages from enclaves
> >   5. EREMOVE all "free" EPC pages, track pages that fail with SGX_CHILD_PRESENT
> >   6. EREMOVE all EPC pages that failed with SGX_CHILD_PRESENT
> >   7. Disable SGX if any EREMOVE failed in step 6
> >   8. Re-enable EPC allocation and enclave fault handling
> > 
> > Exposing EPC to a VM would still require sanitization.
> > 
> > Sanitizing during boot is a lot cleaner, the primary concern is that it
> > will significantly increase boot time on systems with large EPCs.  If we
> > can somehow limit this to kexec() and that's the only scenario where the
> > EPC needs to be sanitized, then that would mitigate the boot time concern.
> > 
> > We might also be able to get away with unconditionally sanitizing the EPC
> > post-boot, e.g. via worker threads, returning -EBUSY for everything until
> > the EPC is good to go.
> 
> I like the worker threads approach better. It is something that is
> maintainable. I don't see any better solution given the hierarchical
> nature of enclaves. It is also fairly to implement without making
> major changes to the other parts of the implementation.
> 
> I.e. every time the driver initializes:
> 
> 1. Move all EPC first to a bad pool.
> 2. Let worker threads move EPC to the real allocation pool.
> 
> Then the OS can immediately start to use EPC.
> 
> Is this about along the lines what you had in mind?

We could even simplify this by using the already existing reclaimer
thread for the purpose.

/Jarkko

  reply index

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-17 21:14 [PATCH v19 00/27] Intel SGX1 support Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 01/27] x86/cpufeatures: Add Intel-defined SGX feature bit Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 02/27] x86/cpufeatures: Add SGX sub-features (as Linux-defined bits) Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 03/27] x86/msr: Add IA32_FEATURE_CONTROL.SGX_ENABLE definition Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 04/27] x86/cpufeatures: Add Intel-defined SGX_LC feature bit Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 05/27] x86/msr: Add SGX Launch Control MSR definitions Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 06/27] x86/mm: x86/sgx: Add new 'PF_SGX' page fault error code bit Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 07/27] x86/mm: x86/sgx: Signal SIGSEGV for userspace #PFs w/ PF_SGX Jarkko Sakkinen
2019-03-18 17:15   ` Dave Hansen
2019-03-18 19:53     ` Sean Christopherson
2019-03-17 21:14 ` [PATCH v19 08/27] x86/cpu/intel: Detect SGX support and update caps appropriately Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 09/27] x86/sgx: Add ENCLS architectural error codes Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 10/27] x86/sgx: Add SGX1 and SGX2 architectural data structures Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 11/27] x86/sgx: Add definitions for SGX's CPUID leaf and variable sub-leafs Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 12/27] x86/sgx: Enumerate and track EPC sections Jarkko Sakkinen
2019-03-18 19:50   ` Sean Christopherson
2019-03-21 14:40     ` Jarkko Sakkinen
2019-03-21 15:28       ` Sean Christopherson
2019-03-22 10:19         ` Jarkko Sakkinen
2019-03-22 10:50           ` Jarkko Sakkinen [this message]
2019-03-17 21:14 ` [PATCH v19 13/27] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen
2019-03-19 19:59   ` Sean Christopherson
2019-03-21 14:51     ` Jarkko Sakkinen
2019-03-21 15:40       ` Sean Christopherson
2019-03-22 11:00         ` Jarkko Sakkinen
2019-03-22 16:43           ` Sean Christopherson
2019-03-17 21:14 ` [PATCH v19 16/27] x86/sgx: Add the Linux SGX Enclave Driver Jarkko Sakkinen
2019-03-19 21:19   ` Sean Christopherson
2019-03-21 15:51     ` Jarkko Sakkinen
2019-03-21 16:47       ` Sean Christopherson
2019-03-22 11:10         ` Jarkko Sakkinen
2019-03-26 13:26       ` Jarkko Sakkinen
2019-03-26 23:58         ` Sean Christopherson
2019-03-27  5:28           ` Jarkko Sakkinen
2019-03-27 17:57             ` Sean Christopherson
2019-03-27 18:38             ` Jethro Beekman
2019-03-27 20:06               ` Sean Christopherson
2019-03-28  1:21                 ` Jethro Beekman
2019-03-28 13:19                 ` Jarkko Sakkinen
2019-03-28 19:05                   ` Andy Lutomirski
2019-03-29  9:43                     ` Jarkko Sakkinen
2019-03-29 16:20                     ` Sean Christopherson
2019-04-01 10:01                       ` Jarkko Sakkinen
2019-04-01 17:25                         ` Jethro Beekman
2019-04-01 22:57                           ` Jarkko Sakkinen
2019-03-28 13:15               ` Jarkko Sakkinen
2019-03-19 23:00   ` Sean Christopherson
2019-03-21 16:18     ` Jarkko Sakkinen
2019-03-21 17:38       ` Sean Christopherson
2019-03-22 11:17         ` Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 17/27] x86/sgx: Add provisioning Jarkko Sakkinen
2019-03-19 20:09   ` Sean Christopherson
2019-03-21  2:08     ` Huang, Kai
2019-03-21 14:32       ` Jarkko Sakkinen
2019-03-21 21:41         ` Huang, Kai
2019-03-22 11:31           ` Jarkko Sakkinen
2019-03-21 14:30     ` Jarkko Sakkinen
2019-03-21 14:38   ` Nathaniel McCallum
2019-03-22 11:22     ` Jarkko Sakkinen
2019-03-21 16:50   ` Andy Lutomirski
2019-03-22 11:29     ` Jarkko Sakkinen
2019-03-22 11:43       ` Jarkko Sakkinen
2019-03-22 18:20         ` Andy Lutomirski
2019-03-25 14:55           ` Jarkko Sakkinen
2019-03-27  0:14             ` Sean Christopherson
2019-04-05 10:18             ` Jarkko Sakkinen
2019-04-05 13:53               ` Andy Lutomirski
2019-04-05 14:20                 ` Jarkko Sakkinen
2019-04-05 14:34                   ` Greg KH
2019-04-09 13:37                     ` Jarkko Sakkinen
2019-04-05 14:21                 ` Greg KH
2019-03-17 21:14 ` [PATCH v19 19/27] x86/sgx: ptrace() support for the SGX driver Jarkko Sakkinen
2019-03-19 22:22   ` Sean Christopherson
2019-03-21 15:02     ` Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 20/27] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 21/27] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 22/27] x86/fault: Attempt to fixup unhandled #PF in vDSO before signaling Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 23/27] x86/traps: Attempt to fixup exceptions " Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 25/27] x86/sgx: SGX documentation Jarkko Sakkinen
2019-03-20 17:14   ` Sean Christopherson
2019-03-21 16:24     ` Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 26/27] selftests/x86: Add a selftest for SGX Jarkko Sakkinen
2019-03-17 21:14 ` [PATCH v19 27/27] x86/sgx: Update MAINTAINERS Jarkko Sakkinen
2019-03-19 17:12   ` Sean Christopherson
2019-03-21 14:42     ` Jarkko Sakkinen
     [not found] ` <20190317211456.13927-19-jarkko.sakkinen@linux.intel.com>
2019-03-19 22:09   ` [PATCH v19 18/27] x86/sgx: Add swapping code to the core and SGX driver Sean Christopherson
2019-03-21 14:59     ` Jarkko Sakkinen
2019-03-19 23:41 ` [PATCH v19 00/27] Intel SGX1 support Sean Christopherson
2019-03-19 23:52   ` Jethro Beekman
2019-03-20  0:22     ` Sean Christopherson
2019-03-21 16:20     ` Jarkko Sakkinen
2019-03-21 16:00   ` Jarkko Sakkinen

Reply instructions:

You may reply publically 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=20190322105058.GE3122@linux.intel.com \
    --to=jarkko.sakkinen@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=haitao.huang@intel.com \
    --cc=josh@joshtriplett.org \
    --cc=kai.huang@intel.com \
    --cc=kai.svahn@intel.com \
    --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=suresh.b.siddha@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

Linux-Sgx Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sgx/0 linux-sgx/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sgx linux-sgx/ https://lore.kernel.org/linux-sgx \
		linux-sgx@vger.kernel.org linux-sgx@archiver.kernel.org
	public-inbox-index linux-sgx


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sgx


AGPL code for this site: git clone https://public-inbox.org/ public-inbox