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

From: Jarkko Sakkinen
Date: Mon Dec 17 2018 - 22:27:15 EST


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 regression) when userspace is building multiple enclaves in parallel
> > using goroutines to wrap Cgo (the issue might apply to any M:N scheduler,
> > but I've only confirmed the Golang case). The issue is that allocating
> > an EPC page acts like a blocking syscall when the EPC is under pressure,
> > i.e. an EPC page isn't immediately available. This causes Go's scheduler
> > to thrash and tank performance[1].
>
> I don't see any major issues having that kthread. All the code that
> maps the enclave would be removed.
>
> I would only allow to map enclave to process address space after the
> enclave has been initialized i.e. SGX_IOC_ENCLAVE_ATTACH.

Some refined thoughts.

PTE insertion can done in the #PF handler. In fact, we can PoC this
already with the current architecture (and I will right after sending
v18).

The backing space is a bit more nasty issue in the add pager thread.
The previous shmem swapping would have been a better fit. Maybe that
should be reconsidered?

If shmem was used, all the commits up to "SGX Enclave Driver" could
be reworked to the new model.

When we think about the swapping code, there uprises some difficulties.
Namely, when a page is swapped, the enclave must unmap the PTE from all
processes that have it mapped.

I have a one compromise solution for the problem above: make enclaves
shared BUT mutually exclusive. When you attach an enclave it gets
detached from the previous process that had it. This would still fully
implement the daemon example that Andy gave in earlier response.

/Jarkko