Hi,

On 1/26/21 2:12 AM, Jarkko Sakkinen wrote:

What the short summary is saying now, is that this commit would make the existing code to use vDSO base address. It's already doing that.

You could instead just "Use getauxval() to simplify the code".

Also, I'd prefer to properly use upper and lower case letter, e.g. vDSO instead of vdso.

Reply-To: In-Reply-To: 20210124062907.88229-2-tianjia.zhang@linux.alibaba.com

On Sun, Jan 24, 2021 at 02:29:03PM +0800, Tianjia Zhang wrote:

...

This patch uses the library function `getauxval(AT_SYSINFO_EHDR)` instead of the custom function `vdso_get_base_addr` to obtain the

Use either double or single quotation mark instead of hyphen.

...

base address of vDSO, which will simplify the code implementation.

Signed-off-by: Tianjia Zhang tianjia.zhang@linux.alibaba.com

This needs to be imperative form, e.g. "Simplify the code implemntation by using getauxval() instead of a custom function."

...

---

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..365d01dea67b 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 *)(uintptr_t)getauxval(AT_SYSINFO_EHDR);

You could just case the result the result directly to void *.

...

if (!addr) goto err; -- 2.19.1.3.ge56e4f7

/Jarkko

Thanks for your suggestions, I will fix it in the v4 patchset.

Best regards, Tianjia