On Fri, Oct 13, 2017 at 11:08:24PM +0300, Jarkko Sakkinen wrote:
> On Fri, Oct 13, 2017 at 11:02:53PM +0300, Jarkko Sakkinen wrote:
> > On Fri, Oct 13, 2017 at 10:58:49PM +0300, Jarkko Sakkinen wrote:
> > > 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
> >
> > And in the case of ioctls (sgx_encl_get()) it would probably make sense
> > to deliver -ENOENT back to the user space instead of -EINVAL, wouldn't it?
> >
> > /Jarkko
>
> Please ignore this response! :-)
>
> /Jarkko
Or actually why not. Does not harm to tell user space that it is an
enclave VMA but not yet created.
/Jarkko
