| Message ID | 153cbbb0279d99d454b06393c19e541fba44d4cd.1640052713.git.pengfei.xu@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce XSAVE feature self-test | expand |
On 12/20/21 8:22 PM, Pengfei Xu wrote: > It will change FPU, SSE(XMM), AVX2(YMM), AVX512, PKRU xstates before process > switching test to ensure that these xstates have been tested. > In order to ensure that the content of xstates is not affected across process > switching, this case tests that: > 1. The xstates content of the child process should be the same as that of the > parent process. > 2. The xstates content of the process should be the same across process > switching. > > Signed-off-by: Pengfei Xu <pengfei.xu@intel.com> > --- > tools/testing/selftests/x86/Makefile | 3 +- > tools/testing/selftests/x86/xsave_fork_test.c | 117 ++++++++++++++++++ > 2 files changed, 119 insertions(+), 1 deletion(-) > create mode 100644 tools/testing/selftests/x86/xsave_fork_test.c > > diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile > index a9e452b65ba2..049f8ffb2742 100644 > --- a/tools/testing/selftests/x86/Makefile > +++ b/tools/testing/selftests/x86/Makefile > @@ -18,7 +18,7 @@ TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \ > test_FCMOV test_FCOMI test_FISTTP \ > vdso_restorer > TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip syscall_numbering \ > - corrupt_xstate_header amx xsave_signal_handle > + corrupt_xstate_header amx xsave_signal_handle xsave_fork_test > # Some selftests require 32bit support enabled also on 64bit systems > TARGETS_C_32BIT_NEEDED := ldt_gdt ptrace_syscall > > @@ -106,3 +106,4 @@ $(OUTPUT)/test_syscall_vdso_32: thunks_32.S > $(OUTPUT)/check_initial_reg_state_32: CFLAGS += -Wl,-ereal_start -static > $(OUTPUT)/check_initial_reg_state_64: CFLAGS += -Wl,-ereal_start -static > $(OUTPUT)/xsave_signal_handle_64: CFLAGS += -mno-sse -mno-mmx -mno-sse2 -mno-avx -mno-pku > +$(OUTPUT)/xsave_fork_test_64: CFLAGS += -mno-sse -mno-mmx -mno-sse2 -mno-avx -mno-pku > diff --git a/tools/testing/selftests/x86/xsave_fork_test.c b/tools/testing/selftests/x86/xsave_fork_test.c > new file mode 100644 > index 000000000000..507334e25eba > --- /dev/null > +++ b/tools/testing/selftests/x86/xsave_fork_test.c > @@ -0,0 +1,117 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * It's used for XSAVE test with process switching. Add text from your change log here - more details on what this test for will be helpful. > + */ > + > +#define _GNU_SOURCE > + > +#include <stdio.h> > +#include <stdint.h> > +#include <string.h> > +#include <sys/wait.h> > +#include <unistd.h> > +#include <sched.h> > +#include <sys/syscall.h> > + > +#include "xsave_common.h" > + > +void *aligned_alloc(size_t alignment, size_t size); > +static unsigned char *xsave_buf0, *xsave_buf1, *xsave_buf2; > +static int xsave_size; > + > +void prepare_environment(void) > +{ > + xsave_size = get_xsave_size(); > + printf("XSAVE_TEST_MASK:0x%x, xsave size:0x%x\n", > + XSAVE_TEST_MASK, xsave_size); > + check_cpu_capability(); > + > + /* SDM XSAVE: misalignment to a 64-byte boundary will result in #GP */ > + xsave_buf0 = aligned_alloc(64, xsave_size); > + if (!xsave_buf0) > + execution_failed("aligned_alloc xsave_buf0 failed\n"); > + xsave_buf1 = aligned_alloc(64, xsave_size); > + if (!xsave_buf1) > + execution_failed("aligned_alloc xsave_buf1 failed\n"); > + xsave_buf2 = aligned_alloc(64, xsave_size); > + if (!xsave_buf2) > + execution_failed("aligned_alloc xsave_buf2 failed\n"); > +} > + > +/* Use fork to create pid and trigger process switch test */ > +int test_xsave_fork(void) > +{ > + pid_t child, grandchild; > + int status, result[2]; > + const char *test_xsave_child = "Child xstate was same as parent"; > + const char *test_process_switch = "Xstate after the process switch didn't change"; > + uint32_t ui32_change = 0xffff0000; > + > + populate_xstate_regs(); > + xsave(xsave_buf0, XSAVE_TEST_MASK); > + child = syscall(SYS_fork); > + if (child < 0) > + execution_failed("fork failed\n"); Please use strerr() instead so we know why fork() failed? Same comment on all other error messages. Use strerror() so we know why syscalls failed. So this would be the change you would make to execution_failed() and pass in the strerror() > + if (child == 0) { > + xsave(xsave_buf1, XSAVE_TEST_MASK); > + result[0] = compare_xsave_buf(xsave_buf0, xsave_buf1, xsave_size, > + test_xsave_child, NO_CHANGE); > + > + /* > + * If above case is failed and prints some failed reason, in NIT: "If the above case fails, print reason for failure > + * order to avoid libc printf change and clean up some xstates, > + * populate xstates again for next test This is not very clear. Is this for avoiding cleanup? What are "some xstates"? > + */ > + populate_xstate_regs(); > + xsave(xsave_buf1, XSAVE_TEST_MASK); > + > + /* fork grandchild will trigger process switching in child */ > + grandchild = syscall(SYS_fork); > + if (grandchild == 0) { > + printf("Grandchild pid:%d change it's own xstates\n", getpid()); > + change_xstate(ui32_change); > + return 0; > + } > + if (grandchild) { > + if (waitpid(grandchild, &status, 0) != grandchild || !WIFEXITED(status)) > + printf("[FAIL]:Grandchild exit with error, status:0x%x\n", > + status); > + } > + /* After switch back to child process and check xstate */ > + xsave(xsave_buf2, XSAVE_TEST_MASK); > + result[1] = compare_xsave_buf(xsave_buf1, xsave_buf2, xsave_size, > + test_process_switch, NO_CHANGE); > + printf("Child pid:%d check xstate after swtich back\n", > + getpid()); > + > + check_result(result[0], test_xsave_child); > + check_result(result[1], test_process_switch); > + printf("Xstate in process switch test pass[%d/%d], err_num:%d\n", > + pass_num, case_num, err_num); > + > + return 0; > + } > + > + if (child) { > + if (waitpid(child, &status, 0) != child || !WIFEXITED(status)) > + printf("[FAIL]:Child exit with error, status:0x%x\n", > + status); > + } > + > + return 0; > +} > + > +int main(void) > +{ > + cpu_set_t set; > + > + case_num = 2; > + CPU_ZERO(&set); > + CPU_SET(0, &set); > + sched_setaffinity(getpid(), sizeof(set), &set); > + > + prepare_environment(); > + test_xsave_fork(); > + > + return 0; > +} > thanks, -- Shuah
Hi Shuah, On 2021-12-21 at 11:05:03 -0700, Shuah Khan wrote: > On 12/20/21 8:22 PM, Pengfei Xu wrote: > > It will change FPU, SSE(XMM), AVX2(YMM), AVX512, PKRU xstates before process > > switching test to ensure that these xstates have been tested. > > In order to ensure that the content of xstates is not affected across process > > switching, this case tests that: > > 1. The xstates content of the child process should be the same as that of the > > parent process. > > 2. The xstates content of the process should be the same across process > > switching. > > > > Signed-off-by: Pengfei Xu <pengfei.xu@intel.com> > > --- > > tools/testing/selftests/x86/Makefile | 3 +- > > tools/testing/selftests/x86/xsave_fork_test.c | 117 ++++++++++++++++++ > > 2 files changed, 119 insertions(+), 1 deletion(-) > > create mode 100644 tools/testing/selftests/x86/xsave_fork_test.c > > > > diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile > > index a9e452b65ba2..049f8ffb2742 100644 > > --- a/tools/testing/selftests/x86/Makefile > > +++ b/tools/testing/selftests/x86/Makefile > > @@ -18,7 +18,7 @@ TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \ > > test_FCMOV test_FCOMI test_FISTTP \ > > vdso_restorer > > TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip syscall_numbering \ > > - corrupt_xstate_header amx xsave_signal_handle > > + corrupt_xstate_header amx xsave_signal_handle xsave_fork_test > > # Some selftests require 32bit support enabled also on 64bit systems > > TARGETS_C_32BIT_NEEDED := ldt_gdt ptrace_syscall > > @@ -106,3 +106,4 @@ $(OUTPUT)/test_syscall_vdso_32: thunks_32.S > > $(OUTPUT)/check_initial_reg_state_32: CFLAGS += -Wl,-ereal_start -static > > $(OUTPUT)/check_initial_reg_state_64: CFLAGS += -Wl,-ereal_start -static > > $(OUTPUT)/xsave_signal_handle_64: CFLAGS += -mno-sse -mno-mmx -mno-sse2 -mno-avx -mno-pku > > +$(OUTPUT)/xsave_fork_test_64: CFLAGS += -mno-sse -mno-mmx -mno-sse2 -mno-avx -mno-pku > > diff --git a/tools/testing/selftests/x86/xsave_fork_test.c b/tools/testing/selftests/x86/xsave_fork_test.c > > new file mode 100644 > > index 000000000000..507334e25eba > > --- /dev/null > > +++ b/tools/testing/selftests/x86/xsave_fork_test.c > > @@ -0,0 +1,117 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * It's used for XSAVE test with process switching. > > Add text from your change log here - more details on what this test > for will be helpful. > Thanks very much for suggestion, I will add detailed description from change log in comments. > > + */ > > + > > +#define _GNU_SOURCE > > + > > +#include <stdio.h> > > +#include <stdint.h> > > +#include <string.h> > > +#include <sys/wait.h> > > +#include <unistd.h> > > +#include <sched.h> > > +#include <sys/syscall.h> > > + > > +#include "xsave_common.h" > > + > > +void *aligned_alloc(size_t alignment, size_t size); > > +static unsigned char *xsave_buf0, *xsave_buf1, *xsave_buf2; > > +static int xsave_size; > > + > > +void prepare_environment(void) > > +{ > > + xsave_size = get_xsave_size(); > > + printf("XSAVE_TEST_MASK:0x%x, xsave size:0x%x\n", > > + XSAVE_TEST_MASK, xsave_size); > > + check_cpu_capability(); > > + > > + /* SDM XSAVE: misalignment to a 64-byte boundary will result in #GP */ > > + xsave_buf0 = aligned_alloc(64, xsave_size); > > + if (!xsave_buf0) > > + execution_failed("aligned_alloc xsave_buf0 failed\n"); > > + xsave_buf1 = aligned_alloc(64, xsave_size); > > + if (!xsave_buf1) > > + execution_failed("aligned_alloc xsave_buf1 failed\n"); > > + xsave_buf2 = aligned_alloc(64, xsave_size); > > + if (!xsave_buf2) > > + execution_failed("aligned_alloc xsave_buf2 failed\n"); > > +} > > + > > +/* Use fork to create pid and trigger process switch test */ > > +int test_xsave_fork(void) > > +{ > > + pid_t child, grandchild; > > + int status, result[2]; > > + const char *test_xsave_child = "Child xstate was same as parent"; > > + const char *test_process_switch = "Xstate after the process switch didn't change"; > > + uint32_t ui32_change = 0xffff0000; > > + > > + populate_xstate_regs(); > > + xsave(xsave_buf0, XSAVE_TEST_MASK); > > + child = syscall(SYS_fork); > > + if (child < 0) > > + execution_failed("fork failed\n"); > > Please use strerr() instead so we know why fork() failed? > Same comment on all other error messages. Use strerror() so > we know why syscalls failed. > > So this would be the change you would make to execution_failed() > and pass in the strerror() > > Thanks very much for suggestion! Yes, I should add errno, strerror(errno) into failed reason. I will add it like as follows: " int execution_failed(char *reason, ...) { printf("FAIL:errno=%d, %s, %s", errno, strerror(errno), reason); err_num++; return 1; } " > > + if (child == 0) { > > + xsave(xsave_buf1, XSAVE_TEST_MASK); > > + result[0] = compare_xsave_buf(xsave_buf0, xsave_buf1, xsave_size, > > + test_xsave_child, NO_CHANGE); > > + > > + /* > > + * If above case is failed and prints some failed reason, in > > NIT: "If the above case fails, print reason for failure Thanks, I will fix it as suggestion. > > + * order to avoid libc printf change and clean up some xstates, > > + * populate xstates again for next test > > This is not very clear. Is this for avoiding cleanup? What are "some xstates"? > Yes, it's not very clear, if printf could not use sse(added -mno-sse), printf will clean xmm registers by "vzeroupper(clean YMM,ZMM, XMM is included in YMM)" in function __strlen_avx2: " ... 0x00007ffff7f1a810 <+272>: f3 0f bc c0 tzcnt %eax,%eax 0x00007ffff7f1a814 <+276>: 48 01 f8 add %rdi,%rax 0x00007ffff7f1a817 <+279>: 48 29 d0 sub %rdx,%rax => 0x00007ffff7f1a81a <+282>: c5 f8 77 vzeroupper " I will improve it as follow if there is no comments: " If the above case fails, print reason for failure, and then xstate like xmm, ymm would be cleared by vzeroupper in printf(-mnosse), so populate xstates for next test case, otherwise, xmm and ymm xstate is all 0, there is no guarantee that xmm, ymm will be restored as expected. " > > + */ > > + populate_xstate_regs(); > > + xsave(xsave_buf1, XSAVE_TEST_MASK); > > + > > + /* fork grandchild will trigger process switching in child */ > > + grandchild = syscall(SYS_fork); > > + if (grandchild == 0) { > > + printf("Grandchild pid:%d change it's own xstates\n", getpid()); > > + change_xstate(ui32_change); > > + return 0; > > + } > > + if (grandchild) { > > + if (waitpid(grandchild, &status, 0) != grandchild || !WIFEXITED(status)) > > + printf("[FAIL]:Grandchild exit with error, status:0x%x\n", > > + status); > > + } > > + /* After switch back to child process and check xstate */ > > + xsave(xsave_buf2, XSAVE_TEST_MASK); > > + result[1] = compare_xsave_buf(xsave_buf1, xsave_buf2, xsave_size, > > + test_process_switch, NO_CHANGE); > > + printf("Child pid:%d check xstate after swtich back\n", > > + getpid()); > > + > > + check_result(result[0], test_xsave_child); > > + check_result(result[1], test_process_switch); > > + printf("Xstate in process switch test pass[%d/%d], err_num:%d\n", > > + pass_num, case_num, err_num); > > + > > + return 0; > > + } > > + > > + if (child) { > > + if (waitpid(child, &status, 0) != child || !WIFEXITED(status)) > > + printf("[FAIL]:Child exit with error, status:0x%x\n", > > + status); > > + } > > + > > + return 0; > > +} > > + > > +int main(void) > > +{ > > + cpu_set_t set; > > + > > + case_num = 2; > > + CPU_ZERO(&set); > > + CPU_SET(0, &set); > > + sched_setaffinity(getpid(), sizeof(set), &set); > > + > > + prepare_environment(); > > + test_xsave_fork(); > > + > > + return 0; > > +} > > > > thanks, > -- Shuah
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index a9e452b65ba2..049f8ffb2742 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -18,7 +18,7 @@ TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \ test_FCMOV test_FCOMI test_FISTTP \ vdso_restorer TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip syscall_numbering \ - corrupt_xstate_header amx xsave_signal_handle + corrupt_xstate_header amx xsave_signal_handle xsave_fork_test # Some selftests require 32bit support enabled also on 64bit systems TARGETS_C_32BIT_NEEDED := ldt_gdt ptrace_syscall @@ -106,3 +106,4 @@ $(OUTPUT)/test_syscall_vdso_32: thunks_32.S $(OUTPUT)/check_initial_reg_state_32: CFLAGS += -Wl,-ereal_start -static $(OUTPUT)/check_initial_reg_state_64: CFLAGS += -Wl,-ereal_start -static $(OUTPUT)/xsave_signal_handle_64: CFLAGS += -mno-sse -mno-mmx -mno-sse2 -mno-avx -mno-pku +$(OUTPUT)/xsave_fork_test_64: CFLAGS += -mno-sse -mno-mmx -mno-sse2 -mno-avx -mno-pku diff --git a/tools/testing/selftests/x86/xsave_fork_test.c b/tools/testing/selftests/x86/xsave_fork_test.c new file mode 100644 index 000000000000..507334e25eba --- /dev/null +++ b/tools/testing/selftests/x86/xsave_fork_test.c @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * It's used for XSAVE test with process switching. + */ + +#define _GNU_SOURCE + +#include <stdio.h> +#include <stdint.h> +#include <string.h> +#include <sys/wait.h> +#include <unistd.h> +#include <sched.h> +#include <sys/syscall.h> + +#include "xsave_common.h" + +void *aligned_alloc(size_t alignment, size_t size); +static unsigned char *xsave_buf0, *xsave_buf1, *xsave_buf2; +static int xsave_size; + +void prepare_environment(void) +{ + xsave_size = get_xsave_size(); + printf("XSAVE_TEST_MASK:0x%x, xsave size:0x%x\n", + XSAVE_TEST_MASK, xsave_size); + check_cpu_capability(); + + /* SDM XSAVE: misalignment to a 64-byte boundary will result in #GP */ + xsave_buf0 = aligned_alloc(64, xsave_size); + if (!xsave_buf0) + execution_failed("aligned_alloc xsave_buf0 failed\n"); + xsave_buf1 = aligned_alloc(64, xsave_size); + if (!xsave_buf1) + execution_failed("aligned_alloc xsave_buf1 failed\n"); + xsave_buf2 = aligned_alloc(64, xsave_size); + if (!xsave_buf2) + execution_failed("aligned_alloc xsave_buf2 failed\n"); +} + +/* Use fork to create pid and trigger process switch test */ +int test_xsave_fork(void) +{ + pid_t child, grandchild; + int status, result[2]; + const char *test_xsave_child = "Child xstate was same as parent"; + const char *test_process_switch = "Xstate after the process switch didn't change"; + uint32_t ui32_change = 0xffff0000; + + populate_xstate_regs(); + xsave(xsave_buf0, XSAVE_TEST_MASK); + child = syscall(SYS_fork); + if (child < 0) + execution_failed("fork failed\n"); + if (child == 0) { + xsave(xsave_buf1, XSAVE_TEST_MASK); + result[0] = compare_xsave_buf(xsave_buf0, xsave_buf1, xsave_size, + test_xsave_child, NO_CHANGE); + + /* + * If above case is failed and prints some failed reason, in + * order to avoid libc printf change and clean up some xstates, + * populate xstates again for next test + */ + populate_xstate_regs(); + xsave(xsave_buf1, XSAVE_TEST_MASK); + + /* fork grandchild will trigger process switching in child */ + grandchild = syscall(SYS_fork); + if (grandchild == 0) { + printf("Grandchild pid:%d change it's own xstates\n", getpid()); + change_xstate(ui32_change); + return 0; + } + if (grandchild) { + if (waitpid(grandchild, &status, 0) != grandchild || !WIFEXITED(status)) + printf("[FAIL]:Grandchild exit with error, status:0x%x\n", + status); + } + /* After switch back to child process and check xstate */ + xsave(xsave_buf2, XSAVE_TEST_MASK); + result[1] = compare_xsave_buf(xsave_buf1, xsave_buf2, xsave_size, + test_process_switch, NO_CHANGE); + printf("Child pid:%d check xstate after swtich back\n", + getpid()); + + check_result(result[0], test_xsave_child); + check_result(result[1], test_process_switch); + printf("Xstate in process switch test pass[%d/%d], err_num:%d\n", + pass_num, case_num, err_num); + + return 0; + } + + if (child) { + if (waitpid(child, &status, 0) != child || !WIFEXITED(status)) + printf("[FAIL]:Child exit with error, status:0x%x\n", + status); + } + + return 0; +} + +int main(void) +{ + cpu_set_t set; + + case_num = 2; + CPU_ZERO(&set); + CPU_SET(0, &set); + sched_setaffinity(getpid(), sizeof(set), &set); + + prepare_environment(); + test_xsave_fork(); + + return 0; +}
It will change FPU, SSE(XMM), AVX2(YMM), AVX512, PKRU xstates before process switching test to ensure that these xstates have been tested. In order to ensure that the content of xstates is not affected across process switching, this case tests that: 1. The xstates content of the child process should be the same as that of the parent process. 2. The xstates content of the process should be the same across process switching. Signed-off-by: Pengfei Xu <pengfei.xu@intel.com> --- tools/testing/selftests/x86/Makefile | 3 +- tools/testing/selftests/x86/xsave_fork_test.c | 117 ++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/x86/xsave_fork_test.c