Messages in this thread | | | Subject | Re: [RFC PATCH v6 1/2] selftests/x86: add xsave test related to nested signal handling | From | Shuah Khan <> | Date | Tue, 21 Dec 2021 10:50:08 -0700 |
| |
On 12/20/21 8:22 PM, Pengfei Xu wrote: > In order to ensure that content of FPU, SSE(XMM), AVX2(YMM), AVX512 opmask and > PKRU xstates in the same process is not affected by signal handling, this case > tests that: > 1. In nested signal processing, the signal handling will use each signal's > own xstates, and the xstates of the signal handling under test should > not be changed after previous nested signal handling is completed; > 2. The xstates content of the process should not change after the entire > nested signal handling. > > Process will change above mentioned xstates before signal handling test to > ensure that these xstates have been tested. > > Signed-off-by: Pengfei Xu <pengfei.xu@intel.com> > Reported-by: kernel test robot <lkp@intel.com> # compile issues during review > --- > tools/testing/selftests/x86/Makefile | 3 +- > tools/testing/selftests/x86/xsave_common.h | 380 ++++++++++++++++++ > .../selftests/x86/xsave_signal_handle.c | 151 +++++++ > 3 files changed, 533 insertions(+), 1 deletion(-) > create mode 100644 tools/testing/selftests/x86/xsave_common.h > create mode 100644 tools/testing/selftests/x86/xsave_signal_handle.c > > diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile > index 8a1f62ab3c8e..a9e452b65ba2 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 > + corrupt_xstate_header amx xsave_signal_handle > # Some selftests require 32bit support enabled also on 64bit systems > TARGETS_C_32BIT_NEEDED := ldt_gdt ptrace_syscall > > @@ -105,3 +105,4 @@ $(OUTPUT)/test_syscall_vdso_32: thunks_32.S > # state. > $(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 > diff --git a/tools/testing/selftests/x86/xsave_common.h b/tools/testing/selftests/x86/xsave_common.h > new file mode 100644 > index 000000000000..5b1226d3feae > --- /dev/null > +++ b/tools/testing/selftests/x86/xsave_common.h > @@ -0,0 +1,380 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#define _GNU_SOURCE > + > +#include <stdio.h> > +#include <stdint.h> > +#include <string.h> > +#include <signal.h> > +#include <unistd.h> > +#include <cpuid.h> > + > +#define XSAVE_MASK_FP (1 << XFEATURE_FP) > +#define XSAVE_MASK_XMM (1 << XFEATURE_SSE) > +#define XSAVE_MASK_YMM (1 << XFEATURE_YMM) > +#define XSAVE_MASK_OPMASK (1 << XFEATURE_OPMASK) > +#define XSAVE_MASK_PKRU (1 << XFEATURE_PKRU) > +#define XSAVE_TEST_MASK (XSAVE_MASK_FP | XSAVE_MASK_XMM | XSAVE_MASK_YMM \ > + | XSAVE_MASK_OPMASK | XSAVE_MASK_PKRU) > +#define RESULT_PASS 0 > +#define RESULT_FAIL 1 > +#define RESULT_ERROR 3 > +#define CHANGE 10 > +#define NO_CHANGE 11 > + > +/* Intel-defined CPU features, CPUID level 0x00000007:0 (EBX), word 9 */ > +#define X86_FEATURE_AVX2 (1U << 5) /* AVX2 instructions */ > +#define X86_FEATURE_AVX512F (1U << 16) /* AVX-512 Foundation */ > + > +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */ > +#define X86_FEATURE_PKU (1U << 3) /* Protection Keys for Userspace */ > +#define X86_FEATURE_OSPKE (1U << 4) /* OS Protection Keys Enable */ > + > +#define XSTATE_CPUID (0xd)
Match the define in arch/x86/include/asm/fpu/xstate.h and add a comment that this also defined in the above include.
Same comment for any other duplicate defines in this file.
> + > +static int err_num, case_num, pass_num; > + > +/* > + * List of XSAVE features Linux knows about: > + */ > +enum xfeature { > + XFEATURE_FP, > + XFEATURE_SSE, > + /* > + * Values above here are "legacy states". > + * Those below are "extended states". > + */ > + XFEATURE_YMM, > + XFEATURE_BNDREGS, > + XFEATURE_BNDCSR, > + XFEATURE_OPMASK, > + XFEATURE_ZMM_Hi256, > + XFEATURE_Hi16_ZMM, > + XFEATURE_PT_UNIMPLEMENTED_SO_FAR, > + XFEATURE_PKRU, > + XFEATURE_PASID, > + XFEATURE_RSRVD_COMP_11, > + XFEATURE_RSRVD_COMP_12, > + XFEATURE_RSRVD_COMP_13, > + XFEATURE_RSRVD_COMP_14, > + XFEATURE_LBR, > +
Remove the black line.
> + XFEATURE_MAX, > +}; > + > +/* > + * Could not use buildin xsave64 in libc without sse, added the function > + * to save xstates in buf > + */ > +static void xsave(unsigned char *buf, unsigned long mask) > +{ > + uint32_t hmask = (uint32_t)(mask >> 32); > + uint32_t lmask = (uint32_t)(mask & 0xffffffff); > + > + asm volatile ("movl %0, %%edx" : : "r" (hmask)); > + asm volatile ("movl %0, %%eax" : : "r" (lmask)); > + asm volatile ("xsave64 %0" : : "m" (*buf)); > +} > + > +/* Check whether cpu has avx2(Advanced Vector Extensions-2) */ > +static inline int cpu_has_avx2(void) > +{ > + unsigned int eax, ebx, ecx, edx; > + > + /* It is a macro-defining function and will not affect xstates */ > + __cpuid_count(7, 0, eax, ebx, ecx, edx); > + > + return (ebx & X86_FEATURE_AVX2); > +} > + > +/* Check whether cpu has avx512f(Advanced Vector Extensions-512 Foundation) */ > +static inline int cpu_has_avx512f(void) > +{ > + unsigned int eax, ebx, ecx, edx; > + > + __cpuid_count(7, 0, eax, ebx, ecx, edx); > + > + return (ebx & X86_FEATURE_AVX512F); > +} > + > +/* Check whether cpu has PKRU(protection keys for user-mode pages) */ > +static inline int cpu_has_pkeys(void) > +{ > + unsigned int eax, ebx, ecx, edx; > + > + __cpuid_count(7, 0, eax, ebx, ecx, edx); > + > + if (!(ecx & X86_FEATURE_PKU)) > + return 0; > + > + if (!(ecx & X86_FEATURE_OSPKE)) > + return 0; > + > + return 1; > +} > + > +int execution_failed(char *reason, ...) > +{ > + printf("FAIL:%s", reason); > + err_num++; > + > + return 1; > +} > + > +int get_xsave_size(void) > +{ > + unsigned int eax, ebx, ecx, edx; > + > + __cpuid_count(0xd, 0, eax, ebx, ecx, edx); > + > + return (int)ecx; > +} > + > +void dump_buffer(unsigned char *buf, int size) > +{ > + int i, j; > + > + printf("xsave size = %d (%03xh)\n", size, size); > + > + for (i = 0; i < size; i += 16) { > + printf("%04x: ", i); > + > + for (j = i; ((j < i + 16) && (j < size)); j++) > + printf("%02x ", buf[j]); > + printf("\n"); > + } > +} > + > +void show_part_buf(unsigned char *buf0, unsigned char *buf1, int start, > + int size) > +{ > + int c; > + > + printf("%04x: ", start); > + for (c = start; ((c < start + 16) && (c < size)); c++) > + printf("%02x ", buf0[c]); > + printf(" -> "); > + for (c = start; ((c < start + 16) && (c < size)); c++) > + printf("%02x ", buf1[c]); > + printf("\n"); > +} > + > +int show_buf_diff(unsigned char *buf0, unsigned char *buf1, int size) > +{ > + int a, b, result_buf = NO_CHANGE; > + > + for (a = 0; a < size; a += 16) { > + for (b = a; ((b < a + 16) && (b < size)); b++) { > + if (buf0[b] != buf1[b]) { > + show_part_buf(buf0, buf1, a, size); > + result_buf = CHANGE; > + break; > + } > + } > + } > + > + return result_buf; > +} > + > +int check_xsave_reserved_header(unsigned char *buf0, > + unsigned char *buf1, int size, const char *test_name) > +{ > + int a, b, result_resv_header = RESULT_PASS; > + > + /* SDM: Software should ensure that bytes 63:16 of the XSAVE header are 0 */ > + for (a = 528; a < 576 ; a += 16) { > + for (b = a; ((b < a + 16) && (b < size)); b++) { > + if ((buf0[b] != 0) || (buf1[b] != 0)) { > + printf("%s FAIL: buf0[%d]:%d or buf1[%d]:%d not 0\n", > + test_name, b, buf0[b], b, buf1[b]); > + show_part_buf(buf0, buf1, a, size); > + result_resv_header = RESULT_FAIL; > + break; > + } > + } > + } > + > + return result_resv_header; > +} > + > +int compare_xsave_buf(unsigned char *buf0, unsigned char *buf1, > + int size, const char *test_name, int change) > +{ > + int result_buf = RESULT_ERROR, result_resv_header = RESULT_ERROR; > + > + switch (change) { > + case CHANGE: > + if (show_buf_diff(buf0, buf1, size) == CHANGE) > + result_buf = RESULT_PASS; > + else { > + printf("%s FAIL: xsave content was same\n", test_name); > + result_buf = RESULT_FAIL; > + } > + break; > + case NO_CHANGE: > + if (show_buf_diff(buf0, buf1, size) == CHANGE) { > + printf("%s FAIL: xsave content changed\n", test_name); > + show_buf_diff(buf0, buf1, size); > + result_buf = RESULT_FAIL; > + } else > + result_buf = RESULT_PASS; > + break; > + default: > + printf("%s ERROR: invalid change:%d\n", test_name, > + change); > + break; > + } > + > + result_resv_header = check_xsave_reserved_header(buf0, buf1, size, > + test_name); > + > + return (result_buf || result_resv_header); > +} > +
Please explain why all these 3 routines are needed? compare_xsave_buf(), show_buf_diff() and show_part_buf()
> +void check_result(int result, const char *test_name) > +{ > + switch (result) { > + case RESULT_PASS: > + printf("[PASS]:%s\n", test_name); > + pass_num++; > + break; > + case RESULT_FAIL: > + printf("[FAIL]:%s\n", test_name); > + break; > + case RESULT_ERROR: > + printf("[ERROR]:%s\n", test_name); > + err_num++; > + break; > + default: > + printf("[ERROR]:%s:invalid result:%c\n", > + test_name, result); > + err_num++; > + break; > + } > +} > + > +/* Populate YMM with ui32_ymm */ > +static void set_avx2_ymm(uint32_t ui32_ymm) > +{ > + if (cpu_has_avx2()) { > + asm volatile ("vbroadcastss %0, %%ymm0" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm1" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm2" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm3" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm4" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm5" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm6" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm7" : : "m" (ui32_ymm)); > + #ifndef __i386__ > + asm volatile ("vbroadcastss %0, %%ymm8" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm9" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm10" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm11" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm12" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm13" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm14" : : "m" (ui32_ymm)); > + asm volatile ("vbroadcastss %0, %%ymm15" : : "m" (ui32_ymm)); > + #endif > + } > +} > + > +static void set_pkru_xstate(uint32_t pkey_reg) > +{ > + unsigned int ecx = 0, edx = 0; > + > + if (cpu_has_pkeys()) > + asm volatile(".byte 0x0f, 0x01, 0xef\n\t" > + : : "a" (pkey_reg), "c" (ecx), "d" (edx)); > +} > + > +/* Populate avx512 opmask k0 to k7 with ffffffffffffffff */ > +static void set_avx512_opmask(void) > +{ > + if (cpu_has_avx512f()) { > + asm volatile("kxnorq %%k0, %%k0, %%k0;" : : : "rcx"); > + asm volatile("kxnorq %%k1, %%k1, %%k1;" : : : "rcx"); > + asm volatile("kxnorq %%k2, %%k2, %%k2;" : : : "rcx"); > + asm volatile("kxnorq %%k3, %%k3, %%k3;" : : : "rcx"); > + asm volatile("kxnorq %%k4, %%k4, %%k4;" : : : "rcx"); > + asm volatile("kxnorq %%k5, %%k5, %%k5;" : : : "rcx"); > + asm volatile("kxnorq %%k6, %%k6, %%k6;" : : : "rcx"); > + asm volatile("kxnorq %%k7, %%k7, %%k7;" : : : "rcx"); > + } > +} > + > +/* Initialize FPU and push different values onto FPU register stack */ > +static void set_fpu_reg(void) > +{ > + uint32_t ui32; > + uint64_t ui64; > + float a = 0.12, b = 0.23; > + > + ui32 = 1; > + ui64 = 0xBAB00500FAB7; > +
Just intialze at the time of define up above. Also add some commeng indicating what 0xBAB00500FAB7 is for.
> + asm volatile("finit"); > + asm volatile("fldl %0" : : "m" (ui64)); > + asm volatile("flds %0" : : "m" (ui32)); > + ui64 += 0x93ABE13; > + asm volatile("fldl %0" : : "m" (ui64)); > + ui64 += 0x93; > + asm volatile("fldl %0" : : "m" (ui64)); > + asm volatile("flds %0" : : "m" (ui32)); > + asm volatile("fldl %0" : : "m" (ui64)); > + ui64 -= 0x21; > + asm volatile("fldl %0" : : "m" (ui64)); > + asm volatile("flds %0" : : "m" (ui32)); > + asm volatile("fldl %0" : : "m" (ui64)); > + a = a * b; > +} > + > +void change_xstate(uint32_t ui32) > +{ > + uint32_t pkey_change = 0xffffff00, ecx = 0, edx = 0;
What does thsi value of pkey_change mean? What are you testing for?
> + > + /* Push value onto FP stack */ > + asm volatile("flds %0" : : "m" (ui32)); > + > + /* change ymm0 to ymm15 with ui32 in 64bit kernel */ > + if (cpu_has_avx2()) > + set_avx2_ymm(ui32); > + > + /* change avx512 opmask k0 to k7 with ff */ > + if (cpu_has_avx512f()) { > + asm volatile("kxnorb %%k0, %%k0, %%k0;" : : : "rcx"); > + asm volatile("kxnorb %%k1, %%k1, %%k1;" : : : "rcx"); > + asm volatile("kxnorb %%k2, %%k2, %%k2;" : : : "rcx"); > + asm volatile("kxnorb %%k3, %%k3, %%k3;" : : : "rcx"); > + asm volatile("kxnorb %%k4, %%k4, %%k4;" : : : "rcx"); > + asm volatile("kxnorb %%k5, %%k5, %%k5;" : : : "rcx"); > + asm volatile("kxnorb %%k6, %%k6, %%k6;" : : : "rcx"); > + asm volatile("kxnorb %%k7, %%k7, %%k7;" : : : "rcx"); > + } > + > + /* Change PKRU xstate */ > + if (cpu_has_pkeys()) > + asm volatile(".byte 0x0f,0x01,0xef\n\t" > + : : "a" (pkey_change), "c" (ecx), "d" (edx)); > +} > + > +void check_cpu_capability(void) > +{ > + if (!cpu_has_avx2()) > + printf("[SKIP] No avx2 capability, skip avx2 part xstate.\n"); > + > + if (!cpu_has_avx512f()) > + printf("[SKIP] No avx512f capability, skip avx512f part xstate.\n"); > + > + if (!cpu_has_pkeys()) > + printf("[SKIP] No pkeys capability, skip pkru part xstate.\n"); > +} > + > +void populate_xstate_regs(void) > +{ > + uint32_t ui32_ymm_value = 0x1F2F3F4F, pkey = 0xfffffffc;
Same comment here. What does this test do?
> + > + set_fpu_reg(); > + set_avx2_ymm(ui32_ymm_value); > + set_avx512_opmask(); > + set_pkru_xstate(pkey); > +} > diff --git a/tools/testing/selftests/x86/xsave_signal_handle.c b/tools/testing/selftests/x86/xsave_signal_handle.c > new file mode 100644 > index 000000000000..e1754b279d00 > --- /dev/null > +++ b/tools/testing/selftests/x86/xsave_signal_handle.c > @@ -0,0 +1,151 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * It's used for XSAVE test with signal handling. > + */ > + > +#define _GNU_SOURCE > + > +#include <stdio.h> > +#include <stdint.h> > +#include <string.h> > +#include <signal.h> > +#include <unistd.h> > +#include <sched.h> > +#include <sys/wait.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, *xsave_buf3; > +static int result[2], xsave_size; > + > +static void usr1_handler(int signum, siginfo_t *info, void *__ctxp) > +{ > + uint32_t ui32; > + const char *usr1_name = "USR1 signal xstates should same after USR2 signal"; > + > + /* Xstate change in signal handling USR1 will not affect process xstates */ > + ui32 = 0x1f2fffff; > + if (signum == SIGUSR1) { > + printf("SIGUSR1:0x%x changed fld & ymm0-15:0x%x\n", > + SIGUSR1, ui32); > + change_xstate(ui32); > + } > + /* > + * SIGUSR1 handler has independent XSAVE content, which is not affected by > + * the SIGUSR2 handler > + */ > + xsave(xsave_buf2, XSAVE_TEST_MASK); > + kill(getpid(), SIGUSR2); > + xsave(xsave_buf3, XSAVE_TEST_MASK); > + result[0] = compare_xsave_buf(xsave_buf2, xsave_buf3, xsave_size, usr1_name, > + NO_CHANGE); > +} > + > +static void usr2_handler(int signum, siginfo_t *info, void *__ctxp) > +{ > + uint32_t ui32; > + > + ui32 = 0x3f4fffff;
Same here.
> + if (signum == SIGUSR2) { > + printf("SIGUSR2:0x%x changed fld & ymm0-15:0x%x\n", > + SIGUSR2, ui32); > + change_xstate(ui32); > + } > +} > + > +static void set_signal_handle(void) > +{ > + struct sigaction sigact; > + > + memset(&sigact, 0, sizeof(sigact)); > + if (sigemptyset(&sigact.sa_mask)) > + execution_failed("sigemptyset error\n"); > + > + sigact.sa_flags = SA_SIGINFO; > + > + sigact.sa_sigaction = usr1_handler; > + if (sigaction(SIGUSR1, &sigact, NULL)) > + execution_failed("SIGUSR1 handling failed\n"); > + > + sigact.sa_sigaction = usr2_handler; > + if (sigaction(SIGUSR2, &sigact, NULL)) > + execution_failed("SIGUSR2 handling failed\n"); > +} > + > +void prepare_environment(void) > +{ > + set_signal_handle(); > + xsave_size = get_xsave_size(); > + printf("XSAVE_TEST_MASK:0x%x, xstate 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"); > + xsave_buf3 = aligned_alloc(64, xsave_size); > + if (!xsave_buf3) > + execution_failed("aligned_alloc xsave_buf3 failed\n"); > +} > + > +static int test_xsave_sig_handle(void) > +{ > + const char *test_name0 = "Signal xstate was same after previous signal handling"; > + const char *test_name1 = "Child xstate was same after signal handling"; > + pid_t child; > + int status; > + > + /* Use child process testing to avoid abnormal blocking the next test */ > + child = syscall(SYS_fork); > + if (child < 0) > + execution_failed("Create child pid failed\n"); > + else if (child == 0) { > + populate_xstate_regs(); > + xsave(xsave_buf0, XSAVE_TEST_MASK); > + /* Dump buffer to show the tested xstates */ > + dump_buffer(xsave_buf0, xsave_size); > + > + /* > + * Because printf in dump will change or clean xstates, populate xstates > + * again to fill the xstates we need to test for next test case > + */ > + populate_xstate_regs(); > + xsave(xsave_buf0, XSAVE_TEST_MASK); > + kill(getpid(), SIGUSR1); > + xsave(xsave_buf1, XSAVE_TEST_MASK); > + result[1] = compare_xsave_buf(xsave_buf0, xsave_buf1, xsave_size, > + test_name1, NO_CHANGE); > + check_result(result[0], test_name0); > + check_result(result[1], test_name1); > + printf("Xstate in signal hanlding 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)) > + execution_failed("Child quit unexpectedly\n"); > + } > + > + return 0; > +} > + > +int main(void) > +{ > + case_num = 2; > + > + prepare_environment(); > + test_xsave_sig_handle(); > + > + return 0; > +} >
Please add information on what these tests are doing?
thanks, -- Shuah
| |