Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752792AbcD0MlG (ORCPT ); Wed, 27 Apr 2016 08:41:06 -0400 Received: from mga02.intel.com ([134.134.136.20]:26349 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751241AbcD0MlD (ORCPT ); Wed, 27 Apr 2016 08:41:03 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,541,1455004800"; d="scan'208";a="967578657" Date: Wed, 27 Apr 2016 15:40:56 +0300 From: Jarkko Sakkinen To: Jethro Beekman Cc: gregkh@linuxfoundation.org, "open list:STAGING SUBSYSTEM" , "maintainer:X86 ARCHITECTURE 32-BIT AND 64-BIT" , "open list:X86 ARCHITECTURE 32-BIT AND 64-BIT" , Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner Subject: Re: [PATCH 3/6] intel_sgx: driver for Intel Secure Guard eXtensions Message-ID: <20160427124056.GA22003@intel.com> References: <1461605698-12385-1-git-send-email-jarkko.sakkinen@linux.intel.com> <1461605698-12385-4-git-send-email-jarkko.sakkinen@linux.intel.com> <57206102.3050507@jbeekman.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57206102.3050507@jbeekman.nl> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1746 Lines: 53 On Tue, Apr 26, 2016 at 11:49:38PM -0700, Jethro Beekman wrote: > On 25-04-16 10:34, Jarkko Sakkinen wrote: > > diff --git a/drivers/staging/intel_sgx/isgx_ioctl.c > b/drivers/staging/intel_sgx/isgx_ioctl.c > > new file mode 100644 > > index 0000000..9d8b36b > > --- /dev/null > > +++ b/drivers/staging/intel_sgx/isgx_ioctl.c > > > > +static long isgx_ioctl_enclave_create(struct file *filep, unsigned int cmd, > > + unsigned long arg) > > > > + secs->base = vm_mmap(filep, 0, secs->size, > > + PROT_READ | PROT_WRITE | PROT_EXEC, > > + MAP_SHARED, 0); > > Why does the ioctl interface map userspace memory for an open device? > There's already a perfectly good syscall for that: mmap. You didn't explain what would be the value in doing this but after thinking for a short while I found out two good reasons: * The current API is ugly in a way that you can anyway call mmap directly too and have a useless zombie enclave. This would make the API less ambiguous. * SGX_IOC_ENCLAVE_CREATE could be removed. SECS could be passed through SGX_IOC_ENCLAVE_ADD_PAGE thus simplifying the API a lot. Given these circumstances I think this does make sense. > > diff --git a/drivers/staging/intel_sgx/isgx_user.h b/drivers/staging/intel_sgx/isgx_user.h > > new file mode 100644 > > index 0000000..672d19c > > --- /dev/null > > +++ b/drivers/staging/intel_sgx/isgx_user.h > > > > +#define SGX_ADD_SKIP_EEXTEND 0x1 > > + > > +struct sgx_add_param { > > + unsigned long addr; > > + unsigned long user_addr; > > + struct isgx_secinfo *secinfo; > > + unsigned int flags; > > +}; > > The hardware supports calling EEXTEND on only a part of a page, I think the > driver should also support that. Why would you want to do that? > Jethro /Jarkko