All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup
@ 2019-12-21  0:31 Sean Christopherson
  2019-12-21  0:31 ` [PATCH for_v25 1/4] x86/sgx: Put SECS backing iff retrieving the backing succeeds Sean Christopherson
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-12-21  0:31 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-sgx

One bug fix in the SECS reclaim path and loosely related cleanup.

Sean Christopherson (4):
  x86/sgx: Put SECS backing iff retrieving the backing succeeds
  x86/sgx: Use goto to handle sgx_encl_get_backing() failure in SECS
    flows
  x86/sgx: Drop unused @encl parameter from __sgx_encl_ewb()
  x86/sgx: Pre-calculate VA slot virtual address in sgx_encl_ewb()

 arch/x86/kernel/cpu/sgx/reclaim.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

-- 
2.24.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH for_v25 1/4] x86/sgx: Put SECS backing iff retrieving the backing succeeds
  2019-12-21  0:31 [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Sean Christopherson
@ 2019-12-21  0:31 ` Sean Christopherson
  2019-12-21  0:31 ` [PATCH for_v25 2/4] x86/sgx: Use goto to handle sgx_encl_get_backing() failure in SECS flows Sean Christopherson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-12-21  0:31 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-sgx

Put the SECS backing if and only if retrieving the backing succeeded,
otherwise sgx_encl_put_backing() will consume uninitialized pointers.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kernel/cpu/sgx/reclaim.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/reclaim.c b/arch/x86/kernel/cpu/sgx/reclaim.c
index 21fbfe2ed9c4..67bda5528625 100644
--- a/arch/x86/kernel/cpu/sgx/reclaim.c
+++ b/arch/x86/kernel/cpu/sgx/reclaim.c
@@ -361,9 +361,9 @@ static void sgx_reclaimer_write(struct sgx_epc_page *epc_page,
 
 				sgx_free_page(encl->secs.epc_page);
 				encl->secs.epc_page = NULL;
+
+				sgx_encl_put_backing(&secs_backing, true);
 			}
-
-			sgx_encl_put_backing(&secs_backing, true);
 		}
 	}
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH for_v25 2/4] x86/sgx: Use goto to handle sgx_encl_get_backing() failure in SECS flows
  2019-12-21  0:31 [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Sean Christopherson
  2019-12-21  0:31 ` [PATCH for_v25 1/4] x86/sgx: Put SECS backing iff retrieving the backing succeeds Sean Christopherson
@ 2019-12-21  0:31 ` Sean Christopherson
  2019-12-21  0:31 ` [PATCH for_v25 3/4] x86/sgx: Drop unused @encl parameter from __sgx_encl_ewb() Sean Christopherson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-12-21  0:31 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-sgx

Use a goto for the SECS backing error path in sgx_reclaimer_write() so
that the happy path is a straight shot, e.g. to make it obvious that the
backing is not being leaked.  Reducing the indentation also eliminates a
line wrap.

No functional change intended.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---

I could go either way on this patch, included it in case you have a
preference.

 arch/x86/kernel/cpu/sgx/reclaim.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/reclaim.c b/arch/x86/kernel/cpu/sgx/reclaim.c
index 67bda5528625..c5c0ba92ab63 100644
--- a/arch/x86/kernel/cpu/sgx/reclaim.c
+++ b/arch/x86/kernel/cpu/sgx/reclaim.c
@@ -355,18 +355,19 @@ static void sgx_reclaimer_write(struct sgx_epc_page *epc_page,
 		} else if (atomic_read(&encl->flags) & SGX_ENCL_INITIALIZED) {
 			ret = sgx_encl_get_backing(encl, PFN_DOWN(encl->size),
 						   &secs_backing);
-			if (!ret) {
-				sgx_encl_ewb(encl->secs.epc_page,
-					     &secs_backing);
+			if (ret)
+				goto out;
 
-				sgx_free_page(encl->secs.epc_page);
-				encl->secs.epc_page = NULL;
+			sgx_encl_ewb(encl->secs.epc_page, &secs_backing);
 
-				sgx_encl_put_backing(&secs_backing, true);
-			}
+			sgx_free_page(encl->secs.epc_page);
+			encl->secs.epc_page = NULL;
+
+			sgx_encl_put_backing(&secs_backing, true);
 		}
 	}
 
+out:
 	mutex_unlock(&encl->lock);
 }
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH for_v25 3/4] x86/sgx: Drop unused @encl parameter from __sgx_encl_ewb()
  2019-12-21  0:31 [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Sean Christopherson
  2019-12-21  0:31 ` [PATCH for_v25 1/4] x86/sgx: Put SECS backing iff retrieving the backing succeeds Sean Christopherson
  2019-12-21  0:31 ` [PATCH for_v25 2/4] x86/sgx: Use goto to handle sgx_encl_get_backing() failure in SECS flows Sean Christopherson
@ 2019-12-21  0:31 ` Sean Christopherson
  2019-12-21  0:31 ` [PATCH for_v25 4/4] x86/sgx: Pre-calculate VA slot virtual address in sgx_encl_ewb() Sean Christopherson
  2020-01-02 17:06 ` [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Jarkko Sakkinen
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-12-21  0:31 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-sgx

Remove the unused @encl parameter from __sgx_encl_ewb() to help shorten
the line lengths of its call sites.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kernel/cpu/sgx/reclaim.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/reclaim.c b/arch/x86/kernel/cpu/sgx/reclaim.c
index c5c0ba92ab63..030b1c24da07 100644
--- a/arch/x86/kernel/cpu/sgx/reclaim.c
+++ b/arch/x86/kernel/cpu/sgx/reclaim.c
@@ -223,7 +223,7 @@ static void sgx_reclaimer_block(struct sgx_epc_page *epc_page)
 	mutex_unlock(&encl->lock);
 }
 
-static int __sgx_encl_ewb(struct sgx_encl *encl, struct sgx_epc_page *epc_page,
+static int __sgx_encl_ewb(struct sgx_epc_page *epc_page,
 			  struct sgx_va_page *va_page, unsigned int va_offset,
 			  struct sgx_backing *backing)
 {
@@ -292,7 +292,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
 	if (sgx_va_page_full(va_page))
 		list_move_tail(&va_page->list, &encl->va_pages);
 
-	ret = __sgx_encl_ewb(encl, epc_page, va_page, va_offset, backing);
+	ret = __sgx_encl_ewb(epc_page, va_page, va_offset, backing);
 	if (ret == SGX_NOT_TRACKED) {
 		ret = __etrack(sgx_epc_addr(encl->secs.epc_page));
 		if (ret) {
@@ -300,8 +300,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
 				ENCLS_WARN(ret, "ETRACK");
 		}
 
-		ret = __sgx_encl_ewb(encl, epc_page, va_page, va_offset,
-				     backing);
+		ret = __sgx_encl_ewb(epc_page, va_page, va_offset, backing);
 		if (ret == SGX_NOT_TRACKED) {
 			/*
 			 * Slow path, send IPIs to kick cpus out of the
@@ -312,8 +311,8 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
 			 */
 			on_each_cpu_mask(sgx_encl_ewb_cpumask(encl),
 					 sgx_ipi_cb, NULL, 1);
-			ret = __sgx_encl_ewb(encl, epc_page, va_page,
-					     va_offset, backing);
+			ret = __sgx_encl_ewb(epc_page, va_page, va_offset,
+					     backing);
 		}
 	}
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH for_v25 4/4] x86/sgx: Pre-calculate VA slot virtual address in sgx_encl_ewb()
  2019-12-21  0:31 [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Sean Christopherson
                   ` (2 preceding siblings ...)
  2019-12-21  0:31 ` [PATCH for_v25 3/4] x86/sgx: Drop unused @encl parameter from __sgx_encl_ewb() Sean Christopherson
@ 2019-12-21  0:31 ` Sean Christopherson
  2020-01-02 17:06 ` [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Jarkko Sakkinen
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-12-21  0:31 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-sgx

Now that sgx_epc_addr() is purely a calculation, calculate the VA slot
in sgx_encl_ewb() and pass it to __sgx_encl_ewb() to reduce line lengths
and avoid re-calculating the address on every EWB attempt.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kernel/cpu/sgx/reclaim.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/reclaim.c b/arch/x86/kernel/cpu/sgx/reclaim.c
index 030b1c24da07..a33f1c45477a 100644
--- a/arch/x86/kernel/cpu/sgx/reclaim.c
+++ b/arch/x86/kernel/cpu/sgx/reclaim.c
@@ -223,8 +223,7 @@ static void sgx_reclaimer_block(struct sgx_epc_page *epc_page)
 	mutex_unlock(&encl->lock);
 }
 
-static int __sgx_encl_ewb(struct sgx_epc_page *epc_page,
-			  struct sgx_va_page *va_page, unsigned int va_offset,
+static int __sgx_encl_ewb(struct sgx_epc_page *epc_page, void *va_slot,
 			  struct sgx_backing *backing)
 {
 	struct sgx_pageinfo pginfo;
@@ -237,8 +236,7 @@ static int __sgx_encl_ewb(struct sgx_epc_page *epc_page,
 	pginfo.metadata = (unsigned long)kmap_atomic(backing->pcmd) +
 			  backing->pcmd_offset;
 
-	ret = __ewb(&pginfo, sgx_epc_addr(epc_page),
-		    sgx_epc_addr(va_page->epc_page) + va_offset);
+	ret = __ewb(&pginfo, sgx_epc_addr(epc_page), va_slot);
 
 	kunmap_atomic((void *)(unsigned long)(pginfo.metadata -
 					      backing->pcmd_offset));
@@ -282,6 +280,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
 	struct sgx_encl *encl = encl_page->encl;
 	struct sgx_va_page *va_page;
 	unsigned int va_offset;
+	void *va_slot;
 	int ret;
 
 	encl_page->desc &= ~SGX_ENCL_PAGE_RECLAIMED;
@@ -289,10 +288,11 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
 	va_page = list_first_entry(&encl->va_pages, struct sgx_va_page,
 				   list);
 	va_offset = sgx_alloc_va_slot(va_page);
+	va_slot = sgx_epc_addr(va_page->epc_page) + va_offset;
 	if (sgx_va_page_full(va_page))
 		list_move_tail(&va_page->list, &encl->va_pages);
 
-	ret = __sgx_encl_ewb(epc_page, va_page, va_offset, backing);
+	ret = __sgx_encl_ewb(epc_page, va_slot, backing);
 	if (ret == SGX_NOT_TRACKED) {
 		ret = __etrack(sgx_epc_addr(encl->secs.epc_page));
 		if (ret) {
@@ -300,7 +300,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
 				ENCLS_WARN(ret, "ETRACK");
 		}
 
-		ret = __sgx_encl_ewb(epc_page, va_page, va_offset, backing);
+		ret = __sgx_encl_ewb(epc_page, va_slot, backing);
 		if (ret == SGX_NOT_TRACKED) {
 			/*
 			 * Slow path, send IPIs to kick cpus out of the
@@ -311,8 +311,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
 			 */
 			on_each_cpu_mask(sgx_encl_ewb_cpumask(encl),
 					 sgx_ipi_cb, NULL, 1);
-			ret = __sgx_encl_ewb(epc_page, va_page, va_offset,
-					     backing);
+			ret = __sgx_encl_ewb(epc_page, va_slot, backing);
 		}
 	}
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup
  2019-12-21  0:31 [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Sean Christopherson
                   ` (3 preceding siblings ...)
  2019-12-21  0:31 ` [PATCH for_v25 4/4] x86/sgx: Pre-calculate VA slot virtual address in sgx_encl_ewb() Sean Christopherson
@ 2020-01-02 17:06 ` Jarkko Sakkinen
  4 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2020-01-02 17:06 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-sgx

On Fri, Dec 20, 2019 at 04:31:52PM -0800, Sean Christopherson wrote:
> One bug fix in the SECS reclaim path and loosely related cleanup.
> 
> Sean Christopherson (4):
>   x86/sgx: Put SECS backing iff retrieving the backing succeeds
>   x86/sgx: Use goto to handle sgx_encl_get_backing() failure in SECS
>     flows
>   x86/sgx: Drop unused @encl parameter from __sgx_encl_ewb()
>   x86/sgx: Pre-calculate VA slot virtual address in sgx_encl_ewb()
> 
>  arch/x86/kernel/cpu/sgx/reclaim.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> -- 
> 2.24.1
> 

Thank you. I merged these and added a changelog entry describing the bug
that was fixed.

/Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-01-02 17:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-21  0:31 [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Sean Christopherson
2019-12-21  0:31 ` [PATCH for_v25 1/4] x86/sgx: Put SECS backing iff retrieving the backing succeeds Sean Christopherson
2019-12-21  0:31 ` [PATCH for_v25 2/4] x86/sgx: Use goto to handle sgx_encl_get_backing() failure in SECS flows Sean Christopherson
2019-12-21  0:31 ` [PATCH for_v25 3/4] x86/sgx: Drop unused @encl parameter from __sgx_encl_ewb() Sean Christopherson
2019-12-21  0:31 ` [PATCH for_v25 4/4] x86/sgx: Pre-calculate VA slot virtual address in sgx_encl_ewb() Sean Christopherson
2020-01-02 17:06 ` [PATCH for_v25 0/4] x86/sgx: Reclaim bug fix and cleanup Jarkko Sakkinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.