linux-sgx.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] Some optimizations related to sgx
@ 2021-02-01 13:26 Tianjia Zhang
  2021-02-01 13:26 ` [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx Tianjia Zhang
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-01 13:26 UTC (permalink / raw)
  To: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Sean Christopherson, Shuah Khan, x86, linux-sgx,
	linux-kselftest, linux-kernel, Jia Zhang
  Cc: Tianjia Zhang

This is an optimization of a set of sgx-related codes, each of which
is independent of the patch. Because the second and third patches have
conflicting dependencies, these patches are put together.

---
v4 changes:
  * Improvements suggested by review

v3 changes:
  * split free_cnt count and spin lock optimization into two patches

v2 changes:
  * review suggested changes

Tianjia Zhang (5):
  selftests/x86: Use getauxval() to simplify the code in sgx
  x86/sgx: Reduce the locking range in sgx_sanitize_section()
  x86/sgx: Optimize the free_cnt count in sgx_epc_section
  x86/sgx: Allows ioctl PROVISION to execute before CREATE
  x86/sgx: Remove redundant if conditions in sgx_encl_create

 arch/x86/kernel/cpu/sgx/driver.c   |  1 +
 arch/x86/kernel/cpu/sgx/ioctl.c    |  8 ++++----
 arch/x86/kernel/cpu/sgx/main.c     | 13 +++++--------
 tools/testing/selftests/sgx/main.c | 24 ++++--------------------
 4 files changed, 14 insertions(+), 32 deletions(-)

-- 
2.19.1.3.ge56e4f7


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

* [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx
  2021-02-01 13:26 [PATCH v4 0/5] Some optimizations related to sgx Tianjia Zhang
@ 2021-02-01 13:26 ` Tianjia Zhang
  2021-02-02 22:02   ` Jarkko Sakkinen
  2021-02-01 13:26 ` [PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section() Tianjia Zhang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-01 13:26 UTC (permalink / raw)
  To: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Sean Christopherson, Shuah Khan, x86, linux-sgx,
	linux-kselftest, linux-kernel, Jia Zhang
  Cc: Tianjia Zhang

Simplify the sgx code implemntation by using library function
getauxval() instead of a custom function to get the base address
of vDSO.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 tools/testing/selftests/sgx/main.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index 724cec700926..5167505fbb46 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -15,6 +15,7 @@
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/types.h>
+#include <sys/auxv.h>
 #include "defines.h"
 #include "main.h"
 #include "../kselftest.h"
@@ -28,24 +29,6 @@ struct vdso_symtab {
 	Elf64_Word *elf_hashtab;
 };
 
-static void *vdso_get_base_addr(char *envp[])
-{
-	Elf64_auxv_t *auxv;
-	int i;
-
-	for (i = 0; envp[i]; i++)
-		;
-
-	auxv = (Elf64_auxv_t *)&envp[i + 1];
-
-	for (i = 0; auxv[i].a_type != AT_NULL; i++) {
-		if (auxv[i].a_type == AT_SYSINFO_EHDR)
-			return (void *)auxv[i].a_un.a_val;
-	}
-
-	return NULL;
-}
-
 static Elf64_Dyn *vdso_get_dyntab(void *addr)
 {
 	Elf64_Ehdr *ehdr = addr;
@@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r
 	return 0;
 }
 
-int main(int argc, char *argv[], char *envp[])
+int main(int argc, char *argv[])
 {
 	struct sgx_enclave_run run;
 	struct vdso_symtab symtab;
@@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[])
 	memset(&run, 0, sizeof(run));
 	run.tcs = encl.encl_base;
 
-	addr = vdso_get_base_addr(envp);
+	/* Get vDSO base address */
+	addr = (void *)getauxval(AT_SYSINFO_EHDR);
 	if (!addr)
 		goto err;
 
-- 
2.19.1.3.ge56e4f7


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

* [PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section()
  2021-02-01 13:26 [PATCH v4 0/5] Some optimizations related to sgx Tianjia Zhang
  2021-02-01 13:26 ` [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx Tianjia Zhang
@ 2021-02-01 13:26 ` Tianjia Zhang
  2021-02-02 22:00   ` Jarkko Sakkinen
  2021-02-01 13:26 ` [PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section Tianjia Zhang
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-01 13:26 UTC (permalink / raw)
  To: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Sean Christopherson, Shuah Khan, x86, linux-sgx,
	linux-kselftest, linux-kernel, Jia Zhang
  Cc: Tianjia Zhang

The spin lock of sgx_epc_section only locks the page_list. The
EREMOVE operation and init_laundry_list is not necessary in the
protection range of the spin lock. This patch reduces the lock
range of the spin lock in the function sgx_sanitize_section()
and only protects the operation of the page_list.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 arch/x86/kernel/cpu/sgx/main.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index c519fc5f6948..4465912174fd 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -41,20 +41,17 @@ static void sgx_sanitize_section(struct sgx_epc_section *section)
 		if (kthread_should_stop())
 			return;
 
-		/* needed for access to ->page_list: */
-		spin_lock(&section->lock);
-
 		page = list_first_entry(&section->init_laundry_list,
 					struct sgx_epc_page, list);
 
 		ret = __eremove(sgx_get_epc_virt_addr(page));
-		if (!ret)
+		if (!ret) {
+			spin_lock(&section->lock);
 			list_move(&page->list, &section->page_list);
-		else
+			spin_unlock(&section->lock);
+		} else
 			list_move_tail(&page->list, &dirty);
 
-		spin_unlock(&section->lock);
-
 		cond_resched();
 	}
 
-- 
2.19.1.3.ge56e4f7


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

* [PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
  2021-02-01 13:26 [PATCH v4 0/5] Some optimizations related to sgx Tianjia Zhang
  2021-02-01 13:26 ` [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx Tianjia Zhang
  2021-02-01 13:26 ` [PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section() Tianjia Zhang
@ 2021-02-01 13:26 ` Tianjia Zhang
  2021-02-02 21:54   ` Jarkko Sakkinen
  2021-02-01 13:26 ` [PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE Tianjia Zhang
  2021-02-01 13:26 ` [PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create Tianjia Zhang
  4 siblings, 1 reply; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-01 13:26 UTC (permalink / raw)
  To: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Sean Christopherson, Shuah Khan, x86, linux-sgx,
	linux-kselftest, linux-kernel, Jia Zhang
  Cc: Tianjia Zhang

'section->free_cnt' represents the free page in sgx_epc_section,
which is assigned once after initialization. In fact, just after the
initialization is completed, the pages are in the init_laundry_list
list and cannot be allocated. This needs to be recovered by EREMOVE
of function sgx_sanitize_section() before it can be used as a page
that can be allocated. The sgx_sanitize_section() will be called in
the kernel thread ksgxd.

This patch moves the initialization of 'section->free_cnt' from the
initialization function sgx_setup_epc_section() to the function
sgx_sanitize_section(), and then accumulates the count after the
successful execution of EREMOVE. This seems to be more reasonable,
free_cnt will also truly reflect the allocatable free pages in EPC.

Sined-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kernel/cpu/sgx/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 4465912174fd..e455ec7b3449 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -48,6 +48,7 @@ static void sgx_sanitize_section(struct sgx_epc_section *section)
 		if (!ret) {
 			spin_lock(&section->lock);
 			list_move(&page->list, &section->page_list);
+			section->free_cnt++;
 			spin_unlock(&section->lock);
 		} else
 			list_move_tail(&page->list, &dirty);
@@ -643,7 +644,6 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64 size,
 		list_add_tail(&section->pages[i].list, &section->init_laundry_list);
 	}
 
-	section->free_cnt = nr_pages;
 	return true;
 }
 
-- 
2.19.1.3.ge56e4f7


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

* [PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE
  2021-02-01 13:26 [PATCH v4 0/5] Some optimizations related to sgx Tianjia Zhang
                   ` (2 preceding siblings ...)
  2021-02-01 13:26 ` [PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section Tianjia Zhang
@ 2021-02-01 13:26 ` Tianjia Zhang
  2021-02-02 21:57   ` Jarkko Sakkinen
  2021-02-01 13:26 ` [PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create Tianjia Zhang
  4 siblings, 1 reply; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-01 13:26 UTC (permalink / raw)
  To: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Sean Christopherson, Shuah Khan, x86, linux-sgx,
	linux-kselftest, linux-kernel, Jia Zhang
  Cc: Tianjia Zhang

In the function sgx_create_enclave(), the direct assignment
operation of attributes_mask determines that the ioctl PROVISION
operation must be executed after the ioctl CREATE operation,
which will limit the flexibility of sgx developers.

This patch takes the assignment of attributes_mask from the
function sgx_create_enclave() has been moved to the function
sgx_open(), this will allow users to perform ioctl PROVISION
operations before ioctl CREATE, increase the flexibility of
the API and reduce restrictions.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 arch/x86/kernel/cpu/sgx/driver.c | 1 +
 arch/x86/kernel/cpu/sgx/ioctl.c  | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
index f2eac41bb4ff..fba0d0bfe976 100644
--- a/arch/x86/kernel/cpu/sgx/driver.c
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -36,6 +36,7 @@ static int sgx_open(struct inode *inode, struct file *file)
 		return ret;
 	}
 
+	encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS;
 	file->private_data = encl;
 
 	return 0;
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 90a5caf76939..1c6ecf9fbeff 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
 	encl->base = secs->base;
 	encl->size = secs->size;
 	encl->attributes = secs->attributes;
-	encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS;
 
 	/* Set only after completion, as encl->lock has not been taken. */
 	set_bit(SGX_ENCL_CREATED, &encl->flags);
-- 
2.19.1.3.ge56e4f7


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

* [PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create
  2021-02-01 13:26 [PATCH v4 0/5] Some optimizations related to sgx Tianjia Zhang
                   ` (3 preceding siblings ...)
  2021-02-01 13:26 ` [PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE Tianjia Zhang
@ 2021-02-01 13:26 ` Tianjia Zhang
  2021-02-02 22:04   ` Jarkko Sakkinen
  4 siblings, 1 reply; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-01 13:26 UTC (permalink / raw)
  To: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Sean Christopherson, Shuah Khan, x86, linux-sgx,
	linux-kselftest, linux-kernel, Jia Zhang
  Cc: Tianjia Zhang

In this scenario, there is no case where va_page is NULL, and
the error has been checked. The if condition statement here is
redundant, so remove the condition detection.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 arch/x86/kernel/cpu/sgx/ioctl.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 1c6ecf9fbeff..719c21cca569 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -66,9 +66,10 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
 	va_page = sgx_encl_grow(encl);
 	if (IS_ERR(va_page))
 		return PTR_ERR(va_page);
-	else if (va_page)
-		list_add(&va_page->list, &encl->va_pages);
-	/* else the tail page of the VA page list had free slots. */
+	if (!va_page)
+		return -EIO;
+
+	list_add(&va_page->list, &encl->va_pages);
 
 	/* The extra page goes to SECS. */
 	encl_size = secs->size + PAGE_SIZE;
-- 
2.19.1.3.ge56e4f7


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

* Re: [PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
  2021-02-01 13:26 ` [PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section Tianjia Zhang
@ 2021-02-02 21:54   ` Jarkko Sakkinen
  2021-02-11  6:05     ` Tianjia Zhang
  0 siblings, 1 reply; 17+ messages in thread
From: Jarkko Sakkinen @ 2021-02-02 21:54 UTC (permalink / raw)
  To: Tianjia Zhang
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang

On Mon, Feb 01, 2021 at 09:26:51PM +0800, Tianjia Zhang wrote:
> 'section->free_cnt' represents the free page in sgx_epc_section,
> which is assigned once after initialization. In fact, just after the
> initialization is completed, the pages are in the init_laundry_list
> list and cannot be allocated. This needs to be recovered by EREMOVE
> of function sgx_sanitize_section() before it can be used as a page
> that can be allocated. The sgx_sanitize_section() will be called in
> the kernel thread ksgxd.
> 
> This patch moves the initialization of 'section->free_cnt' from the
> initialization function sgx_setup_epc_section() to the function
> sgx_sanitize_section(), and then accumulates the count after the
> successful execution of EREMOVE. This seems to be more reasonable,
> free_cnt will also truly reflect the allocatable free pages in EPC.
> 
> Sined-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> Reviewed-by: Sean Christopherson <seanjc@google.com>

I just copy-paste my earlier response to save time sice you  seem
to save time by ignoring it and spamming with the same obviously
illegit patch.

https://lore.kernel.org/linux-sgx/YBGlodsOaX4cWAtl@kernel.org/

/Jarkko

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

* Re: [PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE
  2021-02-01 13:26 ` [PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE Tianjia Zhang
@ 2021-02-02 21:57   ` Jarkko Sakkinen
  2021-02-11  6:11     ` Tianjia Zhang
  0 siblings, 1 reply; 17+ messages in thread
From: Jarkko Sakkinen @ 2021-02-02 21:57 UTC (permalink / raw)
  To: Tianjia Zhang
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang

On Mon, Feb 01, 2021 at 09:26:52PM +0800, Tianjia Zhang wrote:
> In the function sgx_create_enclave(), the direct assignment
> operation of attributes_mask determines that the ioctl PROVISION
> operation must be executed after the ioctl CREATE operation,
> which will limit the flexibility of sgx developers.

Please write acronyms correctly. It's not 'sgx'. It's 'SGX'.

Who are the "sgx developers" and how do they benefit from this?

/Jarkko

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

* Re: [PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section()
  2021-02-01 13:26 ` [PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section() Tianjia Zhang
@ 2021-02-02 22:00   ` Jarkko Sakkinen
  2021-02-11  6:15     ` Tianjia Zhang
  0 siblings, 1 reply; 17+ messages in thread
From: Jarkko Sakkinen @ 2021-02-02 22:00 UTC (permalink / raw)
  To: Tianjia Zhang
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang

On Mon, Feb 01, 2021 at 09:26:50PM +0800, Tianjia Zhang wrote:
> The spin lock of sgx_epc_section only locks the page_list. The
> EREMOVE operation and init_laundry_list is not necessary in the
> protection range of the spin lock. This patch reduces the lock
> range of the spin lock in the function sgx_sanitize_section()
> and only protects the operation of the page_list.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

I'm not confident that this change has any practical value.

/Jarkko

> ---
>  arch/x86/kernel/cpu/sgx/main.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index c519fc5f6948..4465912174fd 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -41,20 +41,17 @@ static void sgx_sanitize_section(struct sgx_epc_section *section)
>  		if (kthread_should_stop())
>  			return;
>  
> -		/* needed for access to ->page_list: */
> -		spin_lock(&section->lock);
> -
>  		page = list_first_entry(&section->init_laundry_list,
>  					struct sgx_epc_page, list);
>  
>  		ret = __eremove(sgx_get_epc_virt_addr(page));
> -		if (!ret)
> +		if (!ret) {
> +			spin_lock(&section->lock);
>  			list_move(&page->list, &section->page_list);
> -		else
> +			spin_unlock(&section->lock);
> +		} else
>  			list_move_tail(&page->list, &dirty);
>  
> -		spin_unlock(&section->lock);
> -
>  		cond_resched();
>  	}
>  
> -- 
> 2.19.1.3.ge56e4f7
> 
> 

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

* Re: [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx
  2021-02-01 13:26 ` [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx Tianjia Zhang
@ 2021-02-02 22:02   ` Jarkko Sakkinen
  2021-02-09  0:09     ` Shuah Khan
  0 siblings, 1 reply; 17+ messages in thread
From: Jarkko Sakkinen @ 2021-02-02 22:02 UTC (permalink / raw)
  To: Tianjia Zhang
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang

On Mon, Feb 01, 2021 at 09:26:49PM +0800, Tianjia Zhang wrote:
> Simplify the sgx code implemntation by using library function
> getauxval() instead of a custom function to get the base address
> of vDSO.
> 
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

This needs also ack from Shuah.

/Jarkko

> ---
>  tools/testing/selftests/sgx/main.c | 24 ++++--------------------
>  1 file changed, 4 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
> index 724cec700926..5167505fbb46 100644
> --- a/tools/testing/selftests/sgx/main.c
> +++ b/tools/testing/selftests/sgx/main.c
> @@ -15,6 +15,7 @@
>  #include <sys/stat.h>
>  #include <sys/time.h>
>  #include <sys/types.h>
> +#include <sys/auxv.h>
>  #include "defines.h"
>  #include "main.h"
>  #include "../kselftest.h"
> @@ -28,24 +29,6 @@ struct vdso_symtab {
>  	Elf64_Word *elf_hashtab;
>  };
>  
> -static void *vdso_get_base_addr(char *envp[])
> -{
> -	Elf64_auxv_t *auxv;
> -	int i;
> -
> -	for (i = 0; envp[i]; i++)
> -		;
> -
> -	auxv = (Elf64_auxv_t *)&envp[i + 1];
> -
> -	for (i = 0; auxv[i].a_type != AT_NULL; i++) {
> -		if (auxv[i].a_type == AT_SYSINFO_EHDR)
> -			return (void *)auxv[i].a_un.a_val;
> -	}
> -
> -	return NULL;
> -}
> -
>  static Elf64_Dyn *vdso_get_dyntab(void *addr)
>  {
>  	Elf64_Ehdr *ehdr = addr;
> @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r
>  	return 0;
>  }
>  
> -int main(int argc, char *argv[], char *envp[])
> +int main(int argc, char *argv[])
>  {
>  	struct sgx_enclave_run run;
>  	struct vdso_symtab symtab;
> @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[])
>  	memset(&run, 0, sizeof(run));
>  	run.tcs = encl.encl_base;
>  
> -	addr = vdso_get_base_addr(envp);
> +	/* Get vDSO base address */
> +	addr = (void *)getauxval(AT_SYSINFO_EHDR);
>  	if (!addr)
>  		goto err;
>  
> -- 
> 2.19.1.3.ge56e4f7
> 
> 

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

* Re: [PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create
  2021-02-01 13:26 ` [PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create Tianjia Zhang
@ 2021-02-02 22:04   ` Jarkko Sakkinen
  2021-02-11  6:17     ` Tianjia Zhang
  0 siblings, 1 reply; 17+ messages in thread
From: Jarkko Sakkinen @ 2021-02-02 22:04 UTC (permalink / raw)
  To: Tianjia Zhang
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang

On Mon, Feb 01, 2021 at 09:26:53PM +0800, Tianjia Zhang wrote:
> In this scenario, there is no case where va_page is NULL, and
> the error has been checked. The if condition statement here is

if-condition, i.e. dash missing

> redundant, so remove the condition detection.
> 
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index 1c6ecf9fbeff..719c21cca569 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -66,9 +66,10 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
>  	va_page = sgx_encl_grow(encl);
>  	if (IS_ERR(va_page))
>  		return PTR_ERR(va_page);
> -	else if (va_page)
> -		list_add(&va_page->list, &encl->va_pages);
> -	/* else the tail page of the VA page list had free slots. */
> +	if (!va_page)
> +		return -EIO;
> +
> +	list_add(&va_page->list, &encl->va_pages);
>  
>  	/* The extra page goes to SECS. */
>  	encl_size = secs->size + PAGE_SIZE;
> -- 
> 2.19.1.3.ge56e4f7
> 
> 

Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

/Jarkko

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

* Re: [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx
  2021-02-02 22:02   ` Jarkko Sakkinen
@ 2021-02-09  0:09     ` Shuah Khan
  2021-02-12 12:20       ` Jarkko Sakkinen
  0 siblings, 1 reply; 17+ messages in thread
From: Shuah Khan @ 2021-02-09  0:09 UTC (permalink / raw)
  To: Jarkko Sakkinen, Tianjia Zhang
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang, Shuah Khan

On 2/2/21 3:02 PM, Jarkko Sakkinen wrote:
> On Mon, Feb 01, 2021 at 09:26:49PM +0800, Tianjia Zhang wrote:
>> Simplify the sgx code implemntation by using library function
>> getauxval() instead of a custom function to get the base address
>> of vDSO.
>>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> This needs also ack from Shuah.
> 

Looks good to me. Thank you.

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah


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

* Re: [PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
  2021-02-02 21:54   ` Jarkko Sakkinen
@ 2021-02-11  6:05     ` Tianjia Zhang
  0 siblings, 0 replies; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-11  6:05 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang



On 2/3/21 5:54 AM, Jarkko Sakkinen wrote:
> On Mon, Feb 01, 2021 at 09:26:51PM +0800, Tianjia Zhang wrote:
>> 'section->free_cnt' represents the free page in sgx_epc_section,
>> which is assigned once after initialization. In fact, just after the
>> initialization is completed, the pages are in the init_laundry_list
>> list and cannot be allocated. This needs to be recovered by EREMOVE
>> of function sgx_sanitize_section() before it can be used as a page
>> that can be allocated. The sgx_sanitize_section() will be called in
>> the kernel thread ksgxd.
>>
>> This patch moves the initialization of 'section->free_cnt' from the
>> initialization function sgx_setup_epc_section() to the function
>> sgx_sanitize_section(), and then accumulates the count after the
>> successful execution of EREMOVE. This seems to be more reasonable,
>> free_cnt will also truly reflect the allocatable free pages in EPC.
>>
>> Sined-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>> Reviewed-by: Sean Christopherson <seanjc@google.com>
> 
> I just copy-paste my earlier response to save time sice you  seem
> to save time by ignoring it and spamming with the same obviously
> illegit patch.
> 
> https://lore.kernel.org/linux-sgx/YBGlodsOaX4cWAtl@kernel.org/
> 
> /Jarkko
> 

Sorry for the late reply, I already responded in the original email.

Best regards,
Tianjia

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

* Re: [PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE
  2021-02-02 21:57   ` Jarkko Sakkinen
@ 2021-02-11  6:11     ` Tianjia Zhang
  0 siblings, 0 replies; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-11  6:11 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang



On 2/3/21 5:57 AM, Jarkko Sakkinen wrote:
> On Mon, Feb 01, 2021 at 09:26:52PM +0800, Tianjia Zhang wrote:
>> In the function sgx_create_enclave(), the direct assignment
>> operation of attributes_mask determines that the ioctl PROVISION
>> operation must be executed after the ioctl CREATE operation,
>> which will limit the flexibility of sgx developers.
> 
> Please write acronyms correctly. It's not 'sgx'. It's 'SGX'.
> 
> Who are the "sgx developers" and how do they benefit from this?
> 
> /Jarkko
> 

It mainly refers to application developers based on SGX technology.

One of the benefits that this brings is that the PROVISION operation can 
be called before or after the enclave is created, compared to the 
previous PROVISION operation can only be executed after the enclave is 
created.

Thanks,
Tianjia

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

* Re: [PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section()
  2021-02-02 22:00   ` Jarkko Sakkinen
@ 2021-02-11  6:15     ` Tianjia Zhang
  0 siblings, 0 replies; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-11  6:15 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang



On 2/3/21 6:00 AM, Jarkko Sakkinen wrote:
> On Mon, Feb 01, 2021 at 09:26:50PM +0800, Tianjia Zhang wrote:
>> The spin lock of sgx_epc_section only locks the page_list. The
>> EREMOVE operation and init_laundry_list is not necessary in the
>> protection range of the spin lock. This patch reduces the lock
>> range of the spin lock in the function sgx_sanitize_section()
>> and only protects the operation of the page_list.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> 
> I'm not confident that this change has any practical value.
> 
> /Jarkko
> 

As a process executed during initialization, this optimization effect 
may not be obvious. If possible, this critical area can be moved outside 
to protect the entire while loop.

Best regards,
Tianjia
>> ---
>>   arch/x86/kernel/cpu/sgx/main.c | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
>> index c519fc5f6948..4465912174fd 100644
>> --- a/arch/x86/kernel/cpu/sgx/main.c
>> +++ b/arch/x86/kernel/cpu/sgx/main.c
>> @@ -41,20 +41,17 @@ static void sgx_sanitize_section(struct sgx_epc_section *section)
>>   		if (kthread_should_stop())
>>   			return;
>>   
>> -		/* needed for access to ->page_list: */
>> -		spin_lock(&section->lock);
>> -
>>   		page = list_first_entry(&section->init_laundry_list,
>>   					struct sgx_epc_page, list);
>>   
>>   		ret = __eremove(sgx_get_epc_virt_addr(page));
>> -		if (!ret)
>> +		if (!ret) {
>> +			spin_lock(&section->lock);
>>   			list_move(&page->list, &section->page_list);
>> -		else
>> +			spin_unlock(&section->lock);
>> +		} else
>>   			list_move_tail(&page->list, &dirty);
>>   
>> -		spin_unlock(&section->lock);
>> -
>>   		cond_resched();
>>   	}
>>   
>> -- 
>> 2.19.1.3.ge56e4f7
>>
>>

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

* Re: [PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create
  2021-02-02 22:04   ` Jarkko Sakkinen
@ 2021-02-11  6:17     ` Tianjia Zhang
  0 siblings, 0 replies; 17+ messages in thread
From: Tianjia Zhang @ 2021-02-11  6:17 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Shuah Khan, x86, linux-sgx, linux-kselftest,
	linux-kernel, Jia Zhang



On 2/3/21 6:04 AM, Jarkko Sakkinen wrote:
> On Mon, Feb 01, 2021 at 09:26:53PM +0800, Tianjia Zhang wrote:
>> In this scenario, there is no case where va_page is NULL, and
>> the error has been checked. The if condition statement here is
> 
> if-condition, i.e. dash missing
> 

Will do in the next patch.

Thanks,
Tianjia

>> redundant, so remove the condition detection.
>>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>> ---
>>   arch/x86/kernel/cpu/sgx/ioctl.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
>> index 1c6ecf9fbeff..719c21cca569 100644
>> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
>> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
>> @@ -66,9 +66,10 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
>>   	va_page = sgx_encl_grow(encl);
>>   	if (IS_ERR(va_page))
>>   		return PTR_ERR(va_page);
>> -	else if (va_page)
>> -		list_add(&va_page->list, &encl->va_pages);
>> -	/* else the tail page of the VA page list had free slots. */
>> +	if (!va_page)
>> +		return -EIO;
>> +
>> +	list_add(&va_page->list, &encl->va_pages);
>>   
>>   	/* The extra page goes to SECS. */
>>   	encl_size = secs->size + PAGE_SIZE;
>> -- 
>> 2.19.1.3.ge56e4f7
>>
>>
> 
> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> /Jarkko
> 

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

* Re: [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx
  2021-02-09  0:09     ` Shuah Khan
@ 2021-02-12 12:20       ` Jarkko Sakkinen
  0 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2021-02-12 12:20 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Tianjia Zhang, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Sean Christopherson, Shuah Khan, x86, linux-sgx,
	linux-kselftest, linux-kernel, Jia Zhang

On Mon, Feb 08, 2021 at 05:09:21PM -0700, Shuah Khan wrote:
> On 2/2/21 3:02 PM, Jarkko Sakkinen wrote:
> > On Mon, Feb 01, 2021 at 09:26:49PM +0800, Tianjia Zhang wrote:
> > > Simplify the sgx code implemntation by using library function
> > > getauxval() instead of a custom function to get the base address
> > > of vDSO.
> > > 
> > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> > 
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
> > This needs also ack from Shuah.
> > 
> 
> Looks good to me. Thank you.
> 
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> 
> thanks,
> -- Shuah

Thank you.

/Jarkko

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

end of thread, other threads:[~2021-02-12 12:23 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01 13:26 [PATCH v4 0/5] Some optimizations related to sgx Tianjia Zhang
2021-02-01 13:26 ` [PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx Tianjia Zhang
2021-02-02 22:02   ` Jarkko Sakkinen
2021-02-09  0:09     ` Shuah Khan
2021-02-12 12:20       ` Jarkko Sakkinen
2021-02-01 13:26 ` [PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section() Tianjia Zhang
2021-02-02 22:00   ` Jarkko Sakkinen
2021-02-11  6:15     ` Tianjia Zhang
2021-02-01 13:26 ` [PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section Tianjia Zhang
2021-02-02 21:54   ` Jarkko Sakkinen
2021-02-11  6:05     ` Tianjia Zhang
2021-02-01 13:26 ` [PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE Tianjia Zhang
2021-02-02 21:57   ` Jarkko Sakkinen
2021-02-11  6:11     ` Tianjia Zhang
2021-02-01 13:26 ` [PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create Tianjia Zhang
2021-02-02 22:04   ` Jarkko Sakkinen
2021-02-11  6:17     ` Tianjia Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).