diff mbox series

[v8,1/5] selftests/sgx: Rename 'eenter' and 'sgx_call_vdso'

Message ID 20210610083021.392269-1-jarkko@kernel.org (mailing list archive)
State New
Headers show
Series [v8,1/5] selftests/sgx: Rename 'eenter' and 'sgx_call_vdso' | expand

Commit Message

Jarkko Sakkinen June 10, 2021, 8:30 a.m. UTC
Rename symbols for better clarity:

* 'eenter' might be confused for directly calling ENCLU[EENTER].  It does
  not.  It calls into the VDSO, which actually has the EENTER instruction.
* 'sgx_call_vdso' is *only* used for entering the enclave.  It's not some
  generic SGX call into the VDSO.

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

v3:
Refine the commit message according to Dave's suggestions.

v2:
Refined the renames just a bit.

 tools/testing/selftests/sgx/call.S |  6 +++---
 tools/testing/selftests/sgx/main.c | 25 +++++++++++++------------
 tools/testing/selftests/sgx/main.h |  4 ++--
 3 files changed, 18 insertions(+), 17 deletions(-)

Comments

Dave Hansen June 10, 2021, 3:45 p.m. UTC | #1
On 6/10/21 1:30 AM, Jarkko Sakkinen wrote:
> Rename symbols for better clarity:
> 
> * 'eenter' might be confused for directly calling ENCLU[EENTER].  It does
>   not.  It calls into the VDSO, which actually has the EENTER instruction.
> * 'sgx_call_vdso' is *only* used for entering the enclave.  It's not some
>   generic SGX call into the VDSO.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

These all look fine to me.  Feel free to add my ack on them.

Since these are pure x86 selftests and the initial code went through the
x86 maintainers, should these got through them as well?  Or, since this
is only selftest code, should Shuah pick them up?
Shuah Khan June 11, 2021, 5:35 p.m. UTC | #2
On 6/10/21 9:45 AM, Dave Hansen wrote:
> On 6/10/21 1:30 AM, Jarkko Sakkinen wrote:
>> Rename symbols for better clarity:
>>
>> * 'eenter' might be confused for directly calling ENCLU[EENTER].  It does
>>    not.  It calls into the VDSO, which actually has the EENTER instruction.
>> * 'sgx_call_vdso' is *only* used for entering the enclave.  It's not some
>>    generic SGX call into the VDSO.
>>
>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> These all look fine to me.  Feel free to add my ack on them.
> 
> Since these are pure x86 selftests and the initial code went through the
> x86 maintainers, should these got through them as well?  Or, since this
> is only selftest code, should Shuah pick them up?
> 

I will queue these up for 5.14-rc1

thanks,
-- Shuah
Shuah Khan June 11, 2021, 10:47 p.m. UTC | #3
On 6/11/21 11:35 AM, Shuah Khan wrote:
> On 6/10/21 9:45 AM, Dave Hansen wrote:
>> On 6/10/21 1:30 AM, Jarkko Sakkinen wrote:
>>> Rename symbols for better clarity:
>>>
>>> * 'eenter' might be confused for directly calling ENCLU[EENTER].  It 
>>> does
>>>    not.  It calls into the VDSO, which actually has the EENTER 
>>> instruction.
>>> * 'sgx_call_vdso' is *only* used for entering the enclave.  It's not 
>>> some
>>>    generic SGX call into the VDSO.
>>>
>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>>
>> These all look fine to me.  Feel free to add my ack on them.
>>
>> Since these are pure x86 selftests and the initial code went through the
>> x86 maintainers, should these got through them as well?  Or, since this
>> is only selftest code, should Shuah pick them up?
>>
> 
> I will queue these up for 5.14-rc1
> 

I almost applied these. Does this require root access, if so,
please add logic to skip the test if non-root user runs it.

Please check for code paths that require root access and skip
the tests.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/sgx/call.S b/tools/testing/selftests/sgx/call.S
index 4ecadc7490f4..b09a25890f3b 100644
--- a/tools/testing/selftests/sgx/call.S
+++ b/tools/testing/selftests/sgx/call.S
@@ -5,8 +5,8 @@ 
 
 	.text
 
-	.global sgx_call_vdso
-sgx_call_vdso:
+	.global sgx_enter_enclave
+sgx_enter_enclave:
 	.cfi_startproc
 	push	%r15
 	.cfi_adjust_cfa_offset	8
@@ -27,7 +27,7 @@  sgx_call_vdso:
 	.cfi_adjust_cfa_offset	8
 	push	0x38(%rsp)
 	.cfi_adjust_cfa_offset	8
-	call	*eenter(%rip)
+	call	*vdso_sgx_enter_enclave(%rip)
 	add	$0x10, %rsp
 	.cfi_adjust_cfa_offset	-0x10
 	pop	%rbx
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index d304a4044eb9..43da68388e25 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -21,7 +21,7 @@ 
 #include "../kselftest.h"
 
 static const uint64_t MAGIC = 0x1122334455667788ULL;
-vdso_sgx_enter_enclave_t eenter;
+vdso_sgx_enter_enclave_t vdso_sgx_enter_enclave;
 
 struct vdso_symtab {
 	Elf64_Sym *elf_symtab;
@@ -149,7 +149,7 @@  int main(int argc, char *argv[])
 {
 	struct sgx_enclave_run run;
 	struct vdso_symtab symtab;
-	Elf64_Sym *eenter_sym;
+	Elf64_Sym *sgx_enter_enclave_sym;
 	uint64_t result = 0;
 	struct encl encl;
 	unsigned int i;
@@ -194,29 +194,30 @@  int main(int argc, char *argv[])
 	if (!vdso_get_symtab(addr, &symtab))
 		goto err;
 
-	eenter_sym = vdso_symtab_get(&symtab, "__vdso_sgx_enter_enclave");
-	if (!eenter_sym)
+	sgx_enter_enclave_sym = vdso_symtab_get(&symtab, "__vdso_sgx_enter_enclave");
+	if (!sgx_enter_enclave_sym)
 		goto err;
 
-	eenter = addr + eenter_sym->st_value;
+	vdso_sgx_enter_enclave = addr + sgx_enter_enclave_sym->st_value;
 
-	ret = sgx_call_vdso((void *)&MAGIC, &result, 0, EENTER, NULL, NULL, &run);
-	if (!report_results(&run, ret, result, "sgx_call_vdso"))
+	ret = sgx_enter_enclave((void *)&MAGIC, &result, 0, EENTER,
+					    NULL, NULL, &run);
+	if (!report_results(&run, ret, result, "sgx_enter_enclave_unclobbered"))
 		goto err;
 
 
 	/* Invoke the vDSO directly. */
 	result = 0;
-	ret = eenter((unsigned long)&MAGIC, (unsigned long)&result, 0, EENTER,
-		     0, 0, &run);
-	if (!report_results(&run, ret, result, "eenter"))
+	ret = vdso_sgx_enter_enclave((unsigned long)&MAGIC, (unsigned long)&result,
+				     0, EENTER, 0, 0, &run);
+	if (!report_results(&run, ret, result, "sgx_enter_enclave"))
 		goto err;
 
 	/* And with an exit handler. */
 	run.user_handler = (__u64)user_handler;
 	run.user_data = 0xdeadbeef;
-	ret = eenter((unsigned long)&MAGIC, (unsigned long)&result, 0, EENTER,
-		     0, 0, &run);
+	ret = vdso_sgx_enter_enclave((unsigned long)&MAGIC, (unsigned long)&result,
+				     0, EENTER, 0, 0, &run);
 	if (!report_results(&run, ret, result, "user_handler"))
 		goto err;
 
diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h
index 67211a708f04..68672fd86cf9 100644
--- a/tools/testing/selftests/sgx/main.h
+++ b/tools/testing/selftests/sgx/main.h
@@ -35,7 +35,7 @@  bool encl_load(const char *path, struct encl *encl);
 bool encl_measure(struct encl *encl);
 bool encl_build(struct encl *encl);
 
-int sgx_call_vdso(void *rdi, void *rsi, long rdx, u32 function, void *r8, void *r9,
-		  struct sgx_enclave_run *run);
+int sgx_enter_enclave(void *rdi, void *rsi, long rdx, u32 function, void *r8, void *r9,
+		      struct sgx_enclave_run *run);
 
 #endif /* MAIN_H */