Re: [PATCH v8 0/6] Introduce CET supervisor state support

[Chao Gao <chao.gao@intel.com>]

On Fri, May 23, 2025 at 10:12:22AM -0700, Dave Hansen wrote:
>On 5/23/25 09:57, Sean Christopherson wrote:
>> Side topic, and *probably* unrelated to this series, I tripped the following
>> WARN when running it through the KVM tests (though I don't think it has anything
>> to do with KVM?).  The WARN is the version of xfd_validate_state() that's guarded
>> by CONFIG_X86_DEBUG_FPU=y.
>> 
>>    WARNING: CPU: 232 PID: 15391 at arch/x86/kernel/fpu/xstate.c:1543 xfd_validate_state+0x65/0x70
>
>Huh, and the two processes getting hit by it:
>
>   CPU: 232 UID: 0 PID: 15391 Comm: DefaultEventMan ...
>   CPU: 77  UID: 0 PID: 14821 Comm: futex-default-S ...
>
>don't _look_ like KVM test processes. My guess would be it's some
>mixture of KVM and a signal handler fighting with XFD state.

We are hitting the third case in the table below [*]:

 MSR | fpstate | cur->fpstate | valid
-------------------------------------
   0 |	   0   |	  x   |    1  // MSR matches @fpstate
   0 |	   1   |          0   |    1  // MSR matches cur->fpstate
   0 |	   1   |          1   |    0  <- *** MSR matches nothing!
   1 |	   0   |          0   |    0  <- *** MSR matches nothing!
   1 |	   0   |          1   |    1  // MSR matches cur->fpstate
   1 |	   1   |          x   |    1  // MSR matches @fpstate

*: https://lore.kernel.org/all/88cb75d3-01b9-38ea-e29f-b8fefb548573@intel.com/

The issue arises because the XFD MSR retains the value (i.e., 0, indicating
AMX enabled) from the previous process, while both the passed-in fpstate
(init_fpstate) and the current fpstate have AMX disabled.

To reproduce this issue, compile the kernel with CONFIG_PREEMPT=y, apply the
attached diff to the amx selftest and run:

# numactl -C 1 ./tools/testing/selftests/x86/amx_64

diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c
index 40769c16de1b..4d533d1a530d 100644
--- a/tools/testing/selftests/x86/amx.c
+++ b/tools/testing/selftests/x86/amx.c
@@ -430,6 +430,10 @@ static inline void validate_tiledata_regs_changed(struct xsave_buffer *xbuf)
		fatal_error("TILEDATA registers did not change");
 }
 
+static void dummy_handler(int sig)
+{
+}
+
 /* tiledata inheritance test */
 
 static void test_fork(void)
@@ -444,6 +448,10 @@ static void test_fork(void)
		/* fork() succeeded.  Now in the parent. */
		int status;
 
+		req_xtiledata_perm();
+		load_rand_tiledata(stashed_xsave);
+		while(1);
+
		wait(&status);
		if (!WIFEXITED(status) || WEXITSTATUS(status))
			fatal_error("fork test child");
@@ -452,7 +460,9 @@ static void test_fork(void)
	/* fork() succeeded.  Now in the child. */
	printf("[RUN]\tCheck tile data inheritance.\n\tBefore fork(), load tiledata\n");
 
-	load_rand_tiledata(stashed_xsave);
+	signal(SIGSEGV, dummy_handler);
+	while(1)
+		raise(SIGSEGV);
 
	grandchild = fork();
	if (grandchild < 0) {
@@ -500,9 +510,6 @@ int main(void)
 
	test_dynamic_state();
 
-	/* Request permission for the following tests */
-	req_xtiledata_perm();
-
	test_fork();
 
	/*


>
>I take it this is a Sapphire Rapids system? Is there anything
>interesting about the config other than CONFIG_X86_DEBUG_FPU?