[RFC PATCH v6 0/2] Introduce XSAVE feature self-test

[RFC PATCH v6 0/2] Introduce XSAVE feature self-test

[Pengfei Xu]

The XSAVE feature set supports the saving and restoring of xstate components,
which is used for process context switching. The state components include
x87 state for FPU execution environment, SSE state, AVX state and so on.
In order to ensure that XSAVE works correctly, add XSAVE basic test for XSAVE
architecture functionality.

This patch set tests and verifies the basic functions of XSAVE in user
space; it tests "FPU, SSE(XMM), AVX2(YMM), AVX512 opmask and PKRU parts"
xstates with following cases:
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 another nested signal handling is completed; and these xstates
   content in the process should not change after the nested signal handling
   is complete.
2. The xstates content of the child process should be the same as that of the
   parent process. The xstates content of the process should be the same across
   process switching.

This is the xstates position for FP, XMM, Header, YMM, AVX512 opmask and PKRU:
It could be saved by xsave instruction, and mask could control which part will
be saved(Header will be saved as mandatory):
FP              (0 - 159 bytes)
XMM             (160-415 bytes)
Reserved        (416-511 bytes  SDM vol1 13.4.1)
Header_used     (512-527 bytes)
Headed_reserved (528-575 bytes must 00, otherwise xrstor will #GP)
YMM             (Offset:CPUID.(EAX=0D,ECX=2).EBX  Size:CPUID(EAX=0D,ECX=2).EAX)
AVX512 opmask   (Offset:CPUID.(EAX=0D,ECX=5).EBX  Size:CPUID(EAX=0D,ECX=5).EAX)
PKRU            (Offset:CPUID.(EAX=0D,ECX=9).EBX  Size:CPUID(EAX=0D,ECX=9).EAX)

It uses syscall function instead of fork() function, becasue syscall libc
function will resume xstates after syscall if there is some xstates change
in syscall libc function.
And populate the xstates will try not to use libc, and every key test action
will try not to use libc except syscall until it's failed or done.
In order to prevent GCC from generating any FP code by mistake,
"-mno-sse -mno-mmx -mno-sse2 -mno-avx -mno-pku" compiler parameter is added to
avoid fake failure. Thanks Dave Hansen's suggestion.

This series introduces only the most basic XSAVE tests. In the future, the
intention is to continue expanding the scope of these selftests to include
more xstates and kernel XSAVE-related functionality tests.

========
- Change from v5 to v6:
  - In order to prevent GCC from generating any FP code by mistake,
    "-mno-sse -mno-mmx -mno-sse2 -mno-avx -mno-pku" compiler parameter was
    added, it referred to the parameters for compiling the x86 kernel. Thanks
    Dave Hansen's suggestion.
  - Removed the use of "kselftest.h", because kselftest.h included <stdlib.h>,
    and "stdlib.h" would use sse instructions in it's libc, and this *XSAVE*
    test needed to be compiled without libc sse instructions(-mno-sse).
  - Improved the description in commit header, thanks Chen Yu's suggestion.
  - Becasue test code could not use buildin xsave64 in libc without sse, added
    xsave function by instruction way.
  - Every key test action would not use libc(like printf) except syscall until
    it's failed or done. If it's failed, then it would print the failed reason.
  - Used __cpuid_count() instead of native_cpuid(), becasue __cpuid_count()
    was a macro definition function with one instruction in libc and did not
    change xstate. Thanks Chatre Reinette, Shuah.
    https://lore.kernel.org/linux-sgx/8b7c98f4-f050-bc1c-5699-fa598ecc66a2@linuxfoundation.org/

- Change from v4 to v5:
  - Moved code files into tools/testing/selftests/x86.
  - Delete xsave instruction test, becaue it's not related to kernel.
  - Improved case description.
  - Added AVX512 opmask change and related XSAVE content verification.
  - Added PKRU part xstate test into instruction and signal handling test.
  - Added XSAVE process swich test for FPU, AVX2, AVX512 opmask and PKRU part.

- Change from v3 to v4:
  - Improve the comment in patch 1.

- Change from v2 to v3:
  - Improve the description of patch 2 git log.

- Change from v1 to v2:
  - Improve the cover-letter. Thanks Dave Hansen's suggestion.

Pengfei Xu (2):
  selftests/x86: add xsave test related to nested signal handling
  selftests/x86: add xsave test related to process switching

 tools/testing/selftests/x86/Makefile          |   4 +-
 tools/testing/selftests/x86/xsave_common.h    | 380 ++++++++++++++++++
 tools/testing/selftests/x86/xsave_fork_test.c | 117 ++++++
 .../selftests/x86/xsave_signal_handle.c       | 151 +++++++
 4 files changed, 651 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/xsave_common.h
 create mode 100644 tools/testing/selftests/x86/xsave_fork_test.c
 create mode 100644 tools/testing/selftests/x86/xsave_signal_handle.c

-- 
2.31.1

[RFC PATCH v6 1/2] selftests/x86: add xsave test related to nested signal handling

[Pengfei Xu]

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)
+
+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,
+
+	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);
+}
+
+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;
+
+	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;
+
+	/* 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;
+
+	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;
+	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;
+}
-- 
2.31.1

[RFC PATCH v6 2/2] selftests/x86: add xsave test related to process switching

[Pengfei Xu]

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.
+ */
+
+#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;
+}
-- 
2.31.1

Re: [RFC PATCH v6 1/2] selftests/x86: add xsave test related to nested signal handling

[Shuah Khan]

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

Re: [RFC PATCH v6 2/2] selftests/x86: add xsave test related to process switching

[Shuah Khan]

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

Re: [RFC PATCH v6 1/2] selftests/x86: add xsave test related to nested signal handling

[Pengfei Xu]

On 2021-12-21 at 10:50:08 -0700, Shuah Khan wrote:
> 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
> > 
> > +#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.
> 
  Thanks a lot for taking a look. I will add comments in next version.

> > +
> > +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.
> 
  Thanks, will fix it.

> > +	XFEATURE_MAX,
> > +};
> > +
> > +
..
> > +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()
> 
  compare_xsave_buf() will compare xstate buf1 and buf2 difference, and check
  both buf1 and buf2 xstate reserved header 63:16 should be all 00.
  SDM mentioned that:
  "Software should ensure that bytes 63:16 of the XSAVE header are 0"

  If there is difference between buf1 and buf2,  show_buf_diff() will be called
  by compare_xsave_buf(), and find the offsets in all the different places.
  show_part_buf() called by show_buf_diff() will print all the different
  places between buf1 and buf2.

  For example, change ymm0 and ymm1 xstates:
"
	uint32_t ui32=0xf1f2;
	xsave(xsave_buf0, XSAVE_TEST_MASK);
	asm volatile ("vbroadcastss %0, %%ymm0" : : "m" (ui32));
	asm volatile ("vbroadcastss %0, %%ymm1" : : "m" (ui32));
	xsave(xsave_buf1, XSAVE_TEST_MASK);
	compare_xsave_buf(xsave_buf0, xsave_buf1, xsave_size,
		test_name1, NO_CHANGE);
"
It will only show different places as below:
00a0: 4f 3f 2f 1f 4f 3f 2f 1f 4f 3f 2f 1f 4f 3f 2f 1f  ->  f2 f1 00 00 f2 f1 00 00 f2 f1 00 00 f2 f1 00 00 
00b0: 4f 3f 2f 1f 4f 3f 2f 1f 4f 3f 2f 1f 4f 3f 2f 1f  ->  f2 f1 00 00 f2 f1 00 00 f2 f1 00 00 f2 f1 00 00 
0240: 4f 3f 2f 1f 4f 3f 2f 1f 4f 3f 2f 1f 4f 3f 2f 1f  ->  f2 f1 00 00 f2 f1 00 00 f2 f1 00 00 f2 f1 00 00 
0250: 4f 3f 2f 1f 4f 3f 2f 1f 4f 3f 2f 1f 4f 3f 2f 1f  ->  f2 f1 00 00 f2 f1 00 00 f2 f1 00 00 f2 f1 00 00 

> > +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;
> > +	}
> > +}
> > +
...
> > +
> > +/* 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.
> 
  Thanks for suggestion, yes, I will add comments for it.
  It will fill 0xBAB00500FAB7 and some different values into FP xstate ST0/MM0-
  ST7/MM7.

> > +	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?
> 
  pkey_change means PKRU(Memory Protection Keys Register for Userspace) xstate
  will change to 0x00000000ffffff00.

  At beginning  of test, populate_xstate_regs() -> set_pkru_xstate(pkey) will
  set PKRU xstate to 0x00000000fffffffc.

  When signal handling test, signal handling function will use it's xstate,
  and it changes PKRU xstate to 0x00000000ffffff00. After signal handling is
  finished, process xstate should be resumed by xsaves/xrstors kernel schedule,
  process xstate should be 0x00000000fffffffc, it's same as
  populate_xstate_regs() -> set_pkru_xstate(pkey) pkru xstate.

> > +
> > +	/* 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?
> 
  populate_xstate_regs() will populate FP, XMM(SSE), YMM(AVX2), AVX512_opmask
  and PKRU xstates with values at the begining of the test. If there are all 00
  in above xstates, we could not confirm kernel xsaves and xrstors the xstates
  works well after signal handling and process switching.

> > +
> > +	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.
> 
  In nested signal handling for SIGUSR2, it changes signal handling own ymm0-15
  xstates to 0x3f4fffff.
  Process has set ymm0-15 to 0x1F2F3F4F at begining of test
  populate_xstate_regs().
  Process ymm xstate should not change after signal handling test.

> > +	if (signum == SIGUSR2) {
> > +		printf("SIGUSR2:0x%x changed fld & ymm0-15:0x%x\n",
> > +			SIGUSR2, ui32);
> > +		change_xstate(ui32);
> > +	}
> > +}
> > +
...
> > +
> > +int main(void)
> > +{
> > +	case_num = 2;
> > +
> > +	prepare_environment();
> > +	test_xsave_sig_handle();
> > +
> > +	return 0;
> > +}
> > 
> 
> Please add information on what these tests are doing?
> 
  Thanks, yes, I will add test information to the head comments of the test
  code in next version.

  Thanks very much!
  BR.

> thanks,
> -- Shuah
> 

Re: [RFC PATCH v6 2/2] selftests/x86: add xsave test related to process switching

[Pengfei Xu]

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