Re: [PATCH RFC] x86: Add SGX_IOC_ENCLAVE_AUGMENT_PAGES

[Jarkko Sakkinen <jarkko@kernel.org>]

On Sun, Mar 06, 2022 at 07:38:40AM -0600, Haitao Huang wrote:
> On Fri, 04 Mar 2022 19:26:12 -0600, Jarkko Sakkinen <jarkko@kernel.org>
> wrote:
> 
> > On Fri, Mar 04, 2022 at 10:27:58AM -0600, Haitao Huang wrote:
> > > On Fri, 04 Mar 2022 06:28:52 -0600, Jarkko Sakkinen <jarkko@kernel.org>
> > > wrote:
> > > 
> > > > With SGX1 an enclave needs to be created with its maximum memory
> > > demands
> > > > allocated. Pages cannot be added to an enclave after it is
> > > initialized.
> > > > SGX2 introduces a new function, ENCLS[EAUG], that can be used to add
> > > > pages
> > > > to an initialized enclave. With SGX2 the enclave still needs to
> > > set aside
> > > > address space for its maximum memory demands during enclave
> > > creation, but
> > > > all pages need not be added before enclave initialization. Pages
> > > can be
> > > > added during enclave runtime.
> > > >
> > > > Add support for dynamically adding pages to an initialized enclave
> > > with
> > > > SGX_IOC_ENCLAVE_AUGMENT_PAGES, which performs EAUG's to a given
> > > range of
> > > > pages. Do not enforce any particular permissions from kernel, like is
> > > > done
> > > > for the pages added during the pre-initialization phase, as enclave
> > > > controls the final permissions and content for these pages by issuing
> > > > either ENCLU[EACCEPT] (empty RW) or ENCLU[EACCEPTCOPY] (arbitrary data
> > > > and
> > > > permissions).
> > > >
> > > > Explicit EAUG ioctl is a better choice than an implicit EAUG from
> > > a page
> > > > fault handler because it allows to have O(1) number of kernel-enclave
> > > > round
> > > > trips for EAUG-EACCEPT{COPY} process, instead of O(n), as it is in the
> > > > case
> > > > when a page fault handler EAUG single page at a time.
> > > >
> > > > Cc: Reinette Chatre <reinette.chatre@intel.com>
> > > > Cc: Nathaniel McCallum <nathaniel@profian.com>
> > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > > ---
> > > > Is contained in sgx2-v2.1 branch of
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-sgx.git
> > > > ---
> > > >  arch/x86/include/uapi/asm/sgx.h |  14 +++
> > > >  arch/x86/kernel/cpu/sgx/ioctl.c | 159
> > > ++++++++++++++++++++++++++++++++
> > > >  2 files changed, 173 insertions(+)
> > > >
> > > > diff --git a/arch/x86/include/uapi/asm/sgx.h
> > > > b/arch/x86/include/uapi/asm/sgx.h
> > > > index c4e0326d281d..2b3a606e78fe 100644
> > > > --- a/arch/x86/include/uapi/asm/sgx.h
> > > > +++ b/arch/x86/include/uapi/asm/sgx.h
> > > > @@ -35,6 +35,8 @@ enum sgx_page_flags {
> > > >  	_IOWR(SGX_MAGIC, 0x06, struct sgx_enclave_modt)
> > > >  #define SGX_IOC_ENCLAVE_REMOVE_PAGES \
> > > >  	_IOWR(SGX_MAGIC, 0x08, struct sgx_enclave_remove_pages)
> > > > +#define SGX_IOC_ENCLAVE_AUGMENT_PAGES \
> > > > +	_IOWR(SGX_MAGIC, 0x09, struct sgx_enclave_augment_pages)
> > > > /**
> > > >   * struct sgx_enclave_create - parameter structure for the
> > > > @@ -138,6 +140,18 @@ struct sgx_enclave_remove_pages {
> > > >  	__u64 count;
> > > >  };
> > > > +/**
> > > > + * struct sgx_enclave_augment_pages - parameter structure for the
> > > > %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
> > > > + * @offset:	starting page offset
> > > > + * @length:	length of the data (multiple of the page size)
> > > > + * @count:	number of bytes added (multiple of the page size)
> > > > + */
> > > > +struct sgx_enclave_augment_pages {
> > > > +	__u64 offset;
> > > > +	__u64 length;
> > > > +	__u64 count;
> > > > +};
> > > > +
> > > 
> > > As I stated in another thread, we need a mechanism to allow EAUG page
> > > lazily, e.g., on #PF. Can we add a field here to indicate that?
> > 
> > ioctl *does not* prevent lazy behaviour where, or if, it makes sense.
> > 
> > For growing memory (e.g. MAP_GROWSDOWN) you should just take advantage of
> > the vDSO's exception handling mechanism and call the ioctl on demand.
> > 
> > For a high-performance user space you still want to be also do minimum
> > round trip "batch jobs" where they are possible.
> > 
> 
> Looks like you are pursuing MAP_POPULATE to optimize out the O(N) trips.
> Just for my understanding of your proposal in case this ever comes back. For
> the on-demand case,  this ioctl is required for for each #PF. That's extra
> round trip compared to automatic kernel EAUG on #PF.

Point taken.

I think we could have both. It would then work quite a lot like for
normal memory.

BR, Jarkko