From 4c76039f6fdc81f4ed0c0211793c0ac9b41d80db Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Wed, 5 Mar 2025 17:24:21 -0500 Subject: [PATCH] kernel/x86_64: Keep FPU and SSE control on context switch. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts most of c7360f4b02123700e1dbf54bda1e3c9956d53014. Unlike the other floating-point registers, these values are callee-saved (see "System V Application Binary Interface, AMD64 Architecture Supplement" section 3.2.1.) So we need to preserve their values and not reset defaults on context switch. EMMS, as the previous changes used, does not suffice to clear exceptions. We could use FNCLEX instead, but we need to reset x87 state on context switch anyway, so use FNINIT. Do not reinstate FNCLEX in the exception handler since it will be executed on context switch anyway. We also need to ensure a clean initial state, so take care of that in arch_thread_init. See also: * remarks in 396b74228eefcf4bc21333e05c1909b8692d1b86 * as well as 53e2dc0f85563a0ec4b316521d20248f840ac05d Fixes #19454 and #18624. Doesn't regress #18656 and #19063. Doesn't seem to affect #19450. Change-Id: I7179f1ec7304e7aed09ff80f6773e53d5dbdf5f9 Reviewed-on: https://review.haiku-os.org/c/haiku/+/9081 Reviewed-by: Jérôme Duval Tested-by: Commit checker robot Reviewed-by: waddlesplash --- headers/private/kernel/arch/x86/64/cpu.h | 15 +++++++++------ src/system/kernel/arch/x86/64/thread.cpp | 13 +++++++------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/headers/private/kernel/arch/x86/64/cpu.h b/headers/private/kernel/arch/x86/64/cpu.h index 33358c60f8..3cc0ceb31f 100644 --- a/headers/private/kernel/arch/x86/64/cpu.h +++ b/headers/private/kernel/arch/x86/64/cpu.h @@ -9,13 +9,13 @@ #include -extern uint16 gFPUControlDefault; -extern uint32 gFPUMXCSRDefault; - - static inline void x86_context_switch(arch_thread* oldState, arch_thread* newState) { + uint16_t fpuControl; + asm volatile("fnstcw %0" : "=m" (fpuControl)); + uint32_t sseControl; + asm volatile("stmxcsr %0" : "=m" (sseControl)); asm volatile( "pushq %%rbp;" "movq $1f, %c[rip](%0);" @@ -32,8 +32,11 @@ x86_context_switch(arch_thread* oldState, arch_thread* newState) "r14", "r15", "xmm0", "xmm1", "xmm2", "xmm3", "xmm4", "xmm5", "xmm6", "xmm7", "xmm8", "xmm9", "xmm10", "xmm11", "xmm12", "xmm13", "xmm14", "xmm15", "memory"); - asm volatile("ldmxcsr %0" : : "m" (gFPUMXCSRDefault)); - asm volatile("fldcw %0" : : "m" (gFPUControlDefault)); + asm volatile("fninit"); + // The kernel only needs FNCLEX (so that FLDCW won't trigger exceptions) + // but we must not leak x87 FPU state between teams, so reset it. + asm volatile("ldmxcsr %0" : : "m" (sseControl)); + asm volatile("fldcw %0" : : "m" (fpuControl)); } diff --git a/src/system/kernel/arch/x86/64/thread.cpp b/src/system/kernel/arch/x86/64/thread.cpp index d0192f3f2f..64b2b9cd9f 100644 --- a/src/system/kernel/arch/x86/64/thread.cpp +++ b/src/system/kernel/arch/x86/64/thread.cpp @@ -71,8 +71,6 @@ extern "C" void x86_64_thread_entry(); // Initial thread saved state. static arch_thread sInitialState _ALIGNED(64); -uint16 gFPUControlDefault; -uint32 gFPUMXCSRDefault; extern uint64 gFPUSaveLength; extern bool gHasXsave; extern bool gHasXsavec; @@ -175,7 +173,6 @@ arch_thread_init(kernel_args* args) // Save one global valid FPU state; it will be copied in the arch dependent // part of each new thread. if (gHasXsave || gHasXsavec) { - memset(sInitialState.fpu_state, 0, gFPUSaveLength); if (gHasXsavec) { asm volatile ( "clts;" \ @@ -203,11 +200,15 @@ arch_thread_init(kernel_args* args) "fxsaveq %0" :: "m" (sInitialState.fpu_state)); } - gFPUControlDefault = ((savefpu*)&sInitialState.fpu_state)->fp_fxsave.control; - gFPUMXCSRDefault = ((savefpu*)&sInitialState.fpu_state)->fp_fxsave.mxcsr; + + // FNINIT does not affect MXCSR or data registers, so we reset them in the state. + savefpu* initialState = ((savefpu*)&sInitialState.fpu_state); + initialState->fp_fxsave.mxcsr = 0x1F80; // __INITIAL_MXCSR__ + memset(initialState->fp_fxsave.fp, 0, sizeof(initialState->fp_fxsave.fp)); + memset(initialState->fp_fxsave.xmm, 0, sizeof(initialState->fp_fxsave.xmm)); + memset(initialState->fp_ymm, 0, sizeof(initialState->fp_ymm)); register_generic_syscall(THREAD_SYSCALLS, arch_thread_control, 1, 0); - return B_OK; }