From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jarkko Sakkinen Subject: Re: [intel-sgx-kernel-dev] [PATCH RFC v3 07/12] intel_sgx: driver for Intel Software Guard Extensions Date: Fri, 13 Oct 2017 22:58:49 +0300 Message-ID: <20171013195849.2vt2n6542ejc6ah4@linux.intel.com> References: <20171010143258.21623-1-jarkko.sakkinen@linux.intel.com> <20171010143258.21623-8-jarkko.sakkinen@linux.intel.com> <20171010182606.GA20986@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mga09.intel.com ([134.134.136.24]:62776 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752984AbdJMT6y (ORCPT ); Fri, 13 Oct 2017 15:58:54 -0400 Content-Disposition: inline In-Reply-To: <20171010182606.GA20986@linux.intel.com> Sender: platform-driver-x86-owner@vger.kernel.org List-ID: To: Sean Christopherson Cc: intel-sgx-kernel-dev@lists.01.org, platform-driver-x86@vger.kernel.org On Tue, Oct 10, 2017 at 11:26:06AM -0700, Sean Christopherson wrote: > On Tue, Oct 10, 2017 at 05:32:53PM +0300, Jarkko Sakkinen wrote: > > diff --git a/drivers/platform/x86/intel_sgx/sgx_encl.c b/drivers/platform/x86/intel_sgx/sgx_encl.c > > new file mode 100644 > > index 000000000000..aa0deed08cee > > --- /dev/null > > +++ b/drivers/platform/x86/intel_sgx/sgx_encl.c > > @@ -0,0 +1,989 @@ > > > > +/** > > + * sgx_encl_find - find an enclave > > + * @mm: mm struct of the current process > > + * @addr: address in the ELRANGE > > + * @created is the enclave already created? > > + * @vma: the resulting VMA > > + * > > + * Finds an enclave identified by the given address. Gives back the VMA, that is > > + * part of the enclave, located in that address. > > + * > > + * Return: > > + * 0 on success, > > + * -EINVAL if not found, > > + */ > > +int sgx_encl_find(struct mm_struct *mm, unsigned long addr, bool created, > > + struct vm_area_struct **vma) > > +{ > > + struct vm_area_struct *result; > > + struct sgx_encl *encl; > > + > > + result = find_vma(mm, addr); > > + if (!result || result->vm_ops != &sgx_vm_ops || addr < result->vm_start) > > + return -EINVAL; > > + > > + encl = result->vm_private_data; > > + if (created) { > > + if (!encl) > > + return -EINVAL; > > + } else { > > + if (encl) > > + return -EINVAL; > > + } > > What about removing @created and returning -ENOENT (or -ENXIO?) if > result->vm_private_data is NULL? Removing @created will eliminate > any potential confusion for the common case of @created=true. For > @created=false, which should be limited to sgx_encl_create, I think > that explicitly checking for "ret != -ENOENT" is more intuitive > than checking whether or not sgx_encl_find succeeded, e.g. I knew > the intent of the check in sgx_encl_create ahead of time and I still > had to walk through sgx_encl_find to verify the behavior. Would make sense. Thank you. /Jarkko