[intel-sgx-kernel-dev,v3,2/2] intel_sgx: backing storage file for PCMD
diff mbox

Message ID 20170130195730.14975-3-jarkko.sakkinen@linux.intel.com
State New
Headers show

Commit Message

Jarkko Sakkinen Jan. 30, 2017, 7:57 p.m. UTC
Move PCMD's to a backing storage file in order to give more control
to do swapping and discarding.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/platform/x86/intel_sgx.h            |  5 +++--
 drivers/platform/x86/intel_sgx_ioctl.c      | 16 +++++++++++--
 drivers/platform/x86/intel_sgx_page_cache.c | 21 ++++++++++++++---
 drivers/platform/x86/intel_sgx_util.c       | 21 ++++++++++++-----
 drivers/platform/x86/intel_sgx_vma.c        | 35 ++++++++++++++++++++---------
 5 files changed, 74 insertions(+), 24 deletions(-)

Comments

Sean Christopherson Feb. 6, 2017, 9:49 p.m. UTC | #1
On Mon, 2017-01-30 at 21:57 +0200, Jarkko Sakkinen wrote:
> Move PCMD's to a backing storage file in order to give more control
> to do swapping and discarding.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/platform/x86/intel_sgx.h            |  5 +++--
>  drivers/platform/x86/intel_sgx_ioctl.c      | 16 +++++++++++--
>  drivers/platform/x86/intel_sgx_page_cache.c | 21 ++++++++++++++---
>  drivers/platform/x86/intel_sgx_util.c       | 21 ++++++++++++-----
>  drivers/platform/x86/intel_sgx_vma.c        | 35 ++++++++++++++++++++------
> ---
>  5 files changed, 74 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_sgx.h
> b/drivers/platform/x86/intel_sgx.h
> index ed9e8e6..fb58d60 100644
> --- a/drivers/platform/x86/intel_sgx.h
> +++ b/drivers/platform/x86/intel_sgx.h
> @@ -115,7 +115,6 @@ struct sgx_encl_page {
>  	struct list_head load_list;
>  	struct sgx_va_page *va_page;
>  	unsigned int va_offset;
> -	struct sgx_pcmd pcmd;
>  };
>  
>  struct sgx_tgid_ctx {
> @@ -141,6 +140,7 @@ struct sgx_encl {
>  	struct task_struct *owner;
>  	struct mm_struct *mm;
>  	struct file *backing;
> +	struct file *pcmd;
>  	struct list_head load_list;
>  	struct kref refcount;
>  	unsigned long base;
> @@ -196,7 +196,8 @@ int sgx_test_and_clear_young(struct sgx_encl_page *page,
> struct sgx_encl *encl);
>  void *sgx_get_epc_page(struct sgx_epc_page *entry);
>  void sgx_put_epc_page(void *epc_page_vaddr);
>  struct page *sgx_get_backing(struct sgx_encl *encl,
> -			     struct sgx_encl_page *entry);
> +			     struct sgx_encl_page *entry,
> +			     bool pcmd);
>  void sgx_put_backing(struct page *backing, bool write);
>  void sgx_insert_pte(struct sgx_encl *encl,
>  		    struct sgx_encl_page *encl_page,

Nitpick - can we rename sgx_get_backing to sgx_get_shmem or something along
those lines?
Jarkko Sakkinen Feb. 8, 2017, 10:41 a.m. UTC | #2
On Mon, Feb 06, 2017 at 01:49:45PM -0800, Sean Christopherson wrote:
> On Mon, 2017-01-30 at 21:57 +0200, Jarkko Sakkinen wrote:
> > Move PCMD's to a backing storage file in order to give more control
> > to do swapping and discarding.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >  drivers/platform/x86/intel_sgx.h            |  5 +++--
> >  drivers/platform/x86/intel_sgx_ioctl.c      | 16 +++++++++++--
> >  drivers/platform/x86/intel_sgx_page_cache.c | 21 ++++++++++++++---
> >  drivers/platform/x86/intel_sgx_util.c       | 21 ++++++++++++-----
> >  drivers/platform/x86/intel_sgx_vma.c        | 35 ++++++++++++++++++++------
> > ---
> >  5 files changed, 74 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/intel_sgx.h
> > b/drivers/platform/x86/intel_sgx.h
> > index ed9e8e6..fb58d60 100644
> > --- a/drivers/platform/x86/intel_sgx.h
> > +++ b/drivers/platform/x86/intel_sgx.h
> > @@ -115,7 +115,6 @@ struct sgx_encl_page {
> >  	struct list_head load_list;
> >  	struct sgx_va_page *va_page;
> >  	unsigned int va_offset;
> > -	struct sgx_pcmd pcmd;
> >  };
> >  
> >  struct sgx_tgid_ctx {
> > @@ -141,6 +140,7 @@ struct sgx_encl {
> >  	struct task_struct *owner;
> >  	struct mm_struct *mm;
> >  	struct file *backing;
> > +	struct file *pcmd;
> >  	struct list_head load_list;
> >  	struct kref refcount;
> >  	unsigned long base;
> > @@ -196,7 +196,8 @@ int sgx_test_and_clear_young(struct sgx_encl_page *page,
> > struct sgx_encl *encl);
> >  void *sgx_get_epc_page(struct sgx_epc_page *entry);
> >  void sgx_put_epc_page(void *epc_page_vaddr);
> >  struct page *sgx_get_backing(struct sgx_encl *encl,
> > -			     struct sgx_encl_page *entry);
> > +			     struct sgx_encl_page *entry,
> > +			     bool pcmd);
> >  void sgx_put_backing(struct page *backing, bool write);
> >  void sgx_insert_pte(struct sgx_encl *encl,
> >  		    struct sgx_encl_page *encl_page,
> 
> Nitpick - can we rename sgx_get_backing to sgx_get_shmem or something along
> those lines?

Hmm...  sgx_get_swap_mem() or something like would be fine for me but I
do not want to underline the shmem. We've also used succesfully a VMA as
a backing storage, which has its own set of benefits and disadvantages
(before the code is in the mainline I won't accept any change to the
backing storage mechanism, just mentioning this for insight).

PS. I have this commit as part of my SGX2 patch set, which I will also
CC to platform-driver-x86. Maybe it's better to give final review on
that. I can change the name before I send that patch set but I'm still
testing it through.

/Jarkko
Sean Christopherson Feb. 8, 2017, 2:29 p.m. UTC | #3
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> On Mon, Feb 06, 2017 at 01:49:45PM -0800, Sean Christopherson wrote:
> > On Mon, 2017-01-30 at 21:57 +0200, Jarkko Sakkinen wrote:
> > > Move PCMD's to a backing storage file in order to give more control
> > > to do swapping and discarding.
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > ---
> > >  drivers/platform/x86/intel_sgx.h            |  5 +++--
> > >  drivers/platform/x86/intel_sgx_ioctl.c      | 16 +++++++++++--
> > >  drivers/platform/x86/intel_sgx_page_cache.c | 21 ++++++++++++++---
> > >  drivers/platform/x86/intel_sgx_util.c       | 21 ++++++++++++-----
> > >  drivers/platform/x86/intel_sgx_vma.c        | 35
> > > ++++++++++++++++++++------ ---
> > >  5 files changed, 74 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/intel_sgx.h
> > > b/drivers/platform/x86/intel_sgx.h
> > > index ed9e8e6..fb58d60 100644
> > > --- a/drivers/platform/x86/intel_sgx.h
> > > +++ b/drivers/platform/x86/intel_sgx.h
> > > @@ -115,7 +115,6 @@ struct sgx_encl_page {
> > >  	struct list_head load_list;
> > >  	struct sgx_va_page *va_page;
> > >  	unsigned int va_offset;
> > > -	struct sgx_pcmd pcmd;
> > >  };
> > >  
> > >  struct sgx_tgid_ctx {
> > > @@ -141,6 +140,7 @@ struct sgx_encl {
> > >  	struct task_struct *owner;
> > >  	struct mm_struct *mm;
> > >  	struct file *backing;
> > > +	struct file *pcmd;
> > >  	struct list_head load_list;
> > >  	struct kref refcount;
> > >  	unsigned long base;
> > > @@ -196,7 +196,8 @@ int sgx_test_and_clear_young(struct sgx_encl_page
> > > *page, struct sgx_encl *encl);
> > >  void *sgx_get_epc_page(struct sgx_epc_page *entry);
> > >  void sgx_put_epc_page(void *epc_page_vaddr);
> > >  struct page *sgx_get_backing(struct sgx_encl *encl,
> > > -			     struct sgx_encl_page *entry);
> > > +			     struct sgx_encl_page *entry,
> > > +			     bool pcmd);
> > >  void sgx_put_backing(struct page *backing, bool write);
> > >  void sgx_insert_pte(struct sgx_encl *encl,
> > >  		    struct sgx_encl_page *encl_page,
> > 
> > Nitpick - can we rename sgx_get_backing to sgx_get_shmem or something along
> > those lines?
> 
> Hmm...  sgx_get_swap_mem() or something like would be fine for me but I
> do not want to underline the shmem. We've also used succesfully a VMA as
> a backing storage, which has its own set of benefits and disadvantages
> (before the code is in the mainline I won't accept any change to the
> backing storage mechanism, just mentioning this for insight).
> 
> PS. I have this commit as part of my SGX2 patch set, which I will also
> CC to platform-driver-x86. Maybe it's better to give final review on
> that. I can change the name before I send that patch set but I'm still
> testing it through.
> 
> /Jarkko

What about renaming "struct file *backing" to "struct file *swap" and keeping sgx_get/put_backing?  The only reason I don't like "backing" as the function name in the current is because it makes me think it's acting only on encl->backing.
Jarkko Sakkinen Feb. 8, 2017, 2:54 p.m. UTC | #4
On Wed, Feb 08, 2017 at 02:29:14PM +0000, Christopherson, Sean J wrote:
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > On Mon, Feb 06, 2017 at 01:49:45PM -0800, Sean Christopherson wrote:
> > > On Mon, 2017-01-30 at 21:57 +0200, Jarkko Sakkinen wrote:
> > > > Move PCMD's to a backing storage file in order to give more control
> > > > to do swapping and discarding.
> > > > 
> > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > ---
> > > >  drivers/platform/x86/intel_sgx.h            |  5 +++--
> > > >  drivers/platform/x86/intel_sgx_ioctl.c      | 16 +++++++++++--
> > > >  drivers/platform/x86/intel_sgx_page_cache.c | 21 ++++++++++++++---
> > > >  drivers/platform/x86/intel_sgx_util.c       | 21 ++++++++++++-----
> > > >  drivers/platform/x86/intel_sgx_vma.c        | 35
> > > > ++++++++++++++++++++------ ---
> > > >  5 files changed, 74 insertions(+), 24 deletions(-)
> > > > 
> > > > diff --git a/drivers/platform/x86/intel_sgx.h
> > > > b/drivers/platform/x86/intel_sgx.h
> > > > index ed9e8e6..fb58d60 100644
> > > > --- a/drivers/platform/x86/intel_sgx.h
> > > > +++ b/drivers/platform/x86/intel_sgx.h
> > > > @@ -115,7 +115,6 @@ struct sgx_encl_page {
> > > >  	struct list_head load_list;
> > > >  	struct sgx_va_page *va_page;
> > > >  	unsigned int va_offset;
> > > > -	struct sgx_pcmd pcmd;
> > > >  };
> > > >  
> > > >  struct sgx_tgid_ctx {
> > > > @@ -141,6 +140,7 @@ struct sgx_encl {
> > > >  	struct task_struct *owner;
> > > >  	struct mm_struct *mm;
> > > >  	struct file *backing;
> > > > +	struct file *pcmd;
> > > >  	struct list_head load_list;
> > > >  	struct kref refcount;
> > > >  	unsigned long base;
> > > > @@ -196,7 +196,8 @@ int sgx_test_and_clear_young(struct sgx_encl_page
> > > > *page, struct sgx_encl *encl);
> > > >  void *sgx_get_epc_page(struct sgx_epc_page *entry);
> > > >  void sgx_put_epc_page(void *epc_page_vaddr);
> > > >  struct page *sgx_get_backing(struct sgx_encl *encl,
> > > > -			     struct sgx_encl_page *entry);
> > > > +			     struct sgx_encl_page *entry,
> > > > +			     bool pcmd);
> > > >  void sgx_put_backing(struct page *backing, bool write);
> > > >  void sgx_insert_pte(struct sgx_encl *encl,
> > > >  		    struct sgx_encl_page *encl_page,
> > > 
> > > Nitpick - can we rename sgx_get_backing to sgx_get_shmem or something along
> > > those lines?
> > 
> > Hmm...  sgx_get_swap_mem() or something like would be fine for me but I
> > do not want to underline the shmem. We've also used succesfully a VMA as
> > a backing storage, which has its own set of benefits and disadvantages
> > (before the code is in the mainline I won't accept any change to the
> > backing storage mechanism, just mentioning this for insight).
> > 
> > PS. I have this commit as part of my SGX2 patch set, which I will also
> > CC to platform-driver-x86. Maybe it's better to give final review on
> > that. I can change the name before I send that patch set but I'm still
> > testing it through.
> > 
> > /Jarkko
> 
> What about renaming "struct file *backing" to "struct file *swap" and
> keeping sgx_get/put_backing?  The only reason I don't like "backing"
> as the function name in the current is because it makes me think it's
> acting only on encl->backing.

I see your point. I'll think about this a bit and propose
naming when I send the patches, but yeah, the naming could
be better

/Jarkko

Patch
diff mbox

diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h
index ed9e8e6..fb58d60 100644
--- a/drivers/platform/x86/intel_sgx.h
+++ b/drivers/platform/x86/intel_sgx.h
@@ -115,7 +115,6 @@  struct sgx_encl_page {
 	struct list_head load_list;
 	struct sgx_va_page *va_page;
 	unsigned int va_offset;
-	struct sgx_pcmd pcmd;
 };
 
 struct sgx_tgid_ctx {
@@ -141,6 +140,7 @@  struct sgx_encl {
 	struct task_struct *owner;
 	struct mm_struct *mm;
 	struct file *backing;
+	struct file *pcmd;
 	struct list_head load_list;
 	struct kref refcount;
 	unsigned long base;
@@ -196,7 +196,8 @@  int sgx_test_and_clear_young(struct sgx_encl_page *page, struct sgx_encl *encl);
 void *sgx_get_epc_page(struct sgx_epc_page *entry);
 void sgx_put_epc_page(void *epc_page_vaddr);
 struct page *sgx_get_backing(struct sgx_encl *encl,
-			     struct sgx_encl_page *entry);
+			     struct sgx_encl_page *entry,
+			     bool pcmd);
 void sgx_put_backing(struct page *backing, bool write);
 void sgx_insert_pte(struct sgx_encl *encl,
 		    struct sgx_encl_page *encl_page,
diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c
index f2cc2c1..b480187 100644
--- a/drivers/platform/x86/intel_sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx_ioctl.c
@@ -234,7 +234,7 @@  static bool sgx_process_add_page_req(struct sgx_add_page_req *req)
 	if (sgx_find_encl(encl->mm, encl_page->addr, &vma))
 		goto out;
 
-	backing = sgx_get_backing(encl, encl_page);
+	backing = sgx_get_backing(encl, encl_page, false);
 	if (IS_ERR(backing))
 		goto out;
 
@@ -484,6 +484,7 @@  static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd,
 	struct vm_area_struct *vma;
 	void *secs_vaddr = NULL;
 	struct file *backing;
+	struct file *pcmd;
 	long ret;
 
 	secs = kzalloc(sizeof(*secs),  GFP_KERNEL);
@@ -508,9 +509,19 @@  static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd,
 		goto out;
 	}
 
+	pcmd = shmem_file_setup("dev/sgx",
+				(secs->size + PAGE_SIZE) >> 5,
+				VM_NORESERVE);
+	if (IS_ERR(pcmd)) {
+		fput(backing);
+		ret = PTR_ERR(pcmd);
+		goto out;
+	}
+
 	encl = kzalloc(sizeof(*encl), GFP_KERNEL);
 	if (!encl) {
 		fput(backing);
+		fput(pcmd);
 		ret = -ENOMEM;
 		goto out;
 	}
@@ -529,6 +540,7 @@  static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd,
 	encl->base = secs->base;
 	encl->size = secs->size;
 	encl->backing = backing;
+	encl->pcmd = pcmd;
 
 	secs_epc = sgx_alloc_page(encl->tgid_ctx, 0);
 	if (IS_ERR(secs_epc)) {
@@ -706,7 +718,7 @@  static int __encl_add_page(struct sgx_encl *encl,
 		goto out;
 	}
 
-	backing = sgx_get_backing(encl, encl_page);
+	backing = sgx_get_backing(encl, encl_page, false);
 	if (IS_ERR((void *)backing)) {
 		ret = PTR_ERR((void *)backing);
 		goto out;
diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
index d073057..8e8f6eb 100644
--- a/drivers/platform/x86/intel_sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx_page_cache.c
@@ -237,11 +237,15 @@  static int __sgx_ewb(struct sgx_encl *encl,
 {
 	struct sgx_page_info pginfo;
 	struct page *backing;
+	struct page *pcmd;
+	unsigned long pcmd_offset;
 	void *epc;
 	void *va;
 	int ret;
 
-	backing = sgx_get_backing(encl, encl_page);
+	pcmd_offset = ((encl_page->addr >> PAGE_SHIFT) & 31) * 128;
+
+	backing = sgx_get_backing(encl, encl_page, false);
 	if (IS_ERR(backing)) {
 		ret = PTR_ERR(backing);
 		sgx_warn(encl, "pinning the backing page for EWB failed with %d\n",
@@ -249,21 +253,32 @@  static int __sgx_ewb(struct sgx_encl *encl,
 		return ret;
 	}
 
+	pcmd = sgx_get_backing(encl, encl_page, true);
+	if (IS_ERR(pcmd)) {
+		ret = PTR_ERR(pcmd);
+		sgx_warn(encl, "pinning the pcmd page for EWB failed with %d\n",
+			 ret);
+		goto out;
+	}
+
 	epc = sgx_get_epc_page(encl_page->epc_page);
 	va = sgx_get_epc_page(encl_page->va_page->epc_page);
 
 	pginfo.srcpge = (unsigned long)kmap_atomic(backing);
-	pginfo.pcmd = (unsigned long)&encl_page->pcmd;
+	pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset;
 	pginfo.linaddr = 0;
 	pginfo.secs = 0;
 	ret = __ewb(&pginfo, epc,
 		    (void *)((unsigned long)va + encl_page->va_offset));
+	kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset));
 	kunmap_atomic((void *)(unsigned long)pginfo.srcpge);
 
 	sgx_put_epc_page(va);
 	sgx_put_epc_page(epc);
-	sgx_put_backing(backing, true);
+	sgx_put_backing(pcmd, true);
 
+out:
+	sgx_put_backing(backing, true);
 	return ret;
 }
 
diff --git a/drivers/platform/x86/intel_sgx_util.c b/drivers/platform/x86/intel_sgx_util.c
index 2c390c5..c8d788a 100644
--- a/drivers/platform/x86/intel_sgx_util.c
+++ b/drivers/platform/x86/intel_sgx_util.c
@@ -79,22 +79,28 @@  void sgx_put_epc_page(void *epc_page_vaddr)
 }
 
 struct page *sgx_get_backing(struct sgx_encl *encl,
-			     struct sgx_encl_page *entry)
+			     struct sgx_encl_page *entry,
+			     bool pcmd)
 {
-	struct page *backing;
 	struct inode *inode;
 	struct address_space *mapping;
 	gfp_t gfpmask;
 	pgoff_t index;
 
-	inode = encl->backing->f_path.dentry->d_inode;
+	if (pcmd)
+		inode = encl->pcmd->f_path.dentry->d_inode;
+	else
+		inode = encl->backing->f_path.dentry->d_inode;
+
 	mapping = inode->i_mapping;
 	gfpmask = mapping_gfp_mask(mapping);
 
-	index = (entry->addr - encl->base) >> PAGE_SHIFT;
-	backing = shmem_read_mapping_page_gfp(mapping, index, gfpmask);
+	if (pcmd)
+		index = (entry->addr - encl->base) >> (PAGE_SHIFT + 5);
+	else
+		index = (entry->addr - encl->base) >> PAGE_SHIFT;
 
-	return backing;
+	return shmem_read_mapping_page_gfp(mapping, index, gfpmask);
 }
 
 void sgx_put_backing(struct page *backing_page, bool write)
@@ -245,5 +251,8 @@  void sgx_encl_release(struct kref *ref)
 	if (encl->backing)
 		fput(encl->backing);
 
+	if (encl->pcmd)
+		fput(encl->pcmd);
+
 	kfree(encl);
 }
diff --git a/drivers/platform/x86/intel_sgx_vma.c b/drivers/platform/x86/intel_sgx_vma.c
index 9cc8e83..8541369 100644
--- a/drivers/platform/x86/intel_sgx_vma.c
+++ b/drivers/platform/x86/intel_sgx_vma.c
@@ -99,13 +99,17 @@  static int sgx_eldu(struct sgx_encl *encl,
 		    bool is_secs)
 {
 	struct page *backing;
+	struct page *pcmd;
+	unsigned long pcmd_offset;
 	struct sgx_page_info pginfo;
 	void *secs_ptr = NULL;
 	void *epc_ptr;
 	void *va_ptr;
 	int ret;
 
-	backing = sgx_get_backing(encl, encl_page);
+	pcmd_offset = ((encl_page->addr >> PAGE_SHIFT) & 31) * 128;
+
+	backing = sgx_get_backing(encl, encl_page, false);
 	if (IS_ERR(backing)) {
 		ret = PTR_ERR(backing);
 		sgx_warn(encl, "pinning the backing page for ELDU failed with %d\n",
@@ -113,22 +117,34 @@  static int sgx_eldu(struct sgx_encl *encl,
 		return ret;
 	}
 
+	pcmd = sgx_get_backing(encl, encl_page, true);
+	if (IS_ERR(pcmd)) {
+		ret = PTR_ERR(pcmd);
+		sgx_warn(encl, "pinning the pcmd page for EWB failed with %d\n",
+			 ret);
+		goto out;
+	}
+
 	if (!is_secs)
 		secs_ptr = sgx_get_epc_page(encl->secs_page.epc_page);
-	pginfo.secs = (unsigned long)secs_ptr;
 
 	epc_ptr = sgx_get_epc_page(epc_page);
 	va_ptr = sgx_get_epc_page(encl_page->va_page->epc_page);
 	pginfo.srcpge = (unsigned long)kmap_atomic(backing);
-
+	pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset;
 	pginfo.linaddr = is_secs ? 0 : encl_page->addr;
-	pginfo.pcmd = (unsigned long)&encl_page->pcmd;
+	pginfo.secs = (unsigned long)secs_ptr;
 
 	ret = __eldu((unsigned long)&pginfo,
 		     (unsigned long)epc_ptr,
 		     (unsigned long)va_ptr +
 		     encl_page->va_offset);
+	if (ret) {
+		sgx_err(encl, "ELDU returned %d\n", ret);
+		ret = -EFAULT;
+	}
 
+	kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset));
 	kunmap_atomic((void *)(unsigned long)pginfo.srcpge);
 	sgx_put_epc_page(va_ptr);
 	sgx_put_epc_page(epc_ptr);
@@ -136,14 +152,11 @@  static int sgx_eldu(struct sgx_encl *encl,
 	if (!is_secs)
 		sgx_put_epc_page(secs_ptr);
 
-	sgx_put_backing(backing, false);
+	sgx_put_backing(pcmd, false);
 
-	if (ret) {
-		sgx_err(encl, "ELDU returned %d\n", ret);
-		return -EFAULT;
-	}
-
-	return 0;
+out:
+	sgx_put_backing(backing, false);
+	return ret;
 }
 
 static struct sgx_encl_page *sgx_vma_do_fault(struct vm_area_struct *vma,