From 6db4b42346d30139721e55fd308478667008b042 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Fri, 7 Mar 2025 16:39:31 -0500 Subject: [PATCH] kernel/x86_64: Store FPU state from int_bottom_user in arch_thread always. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The XSAVE area can be quite large (1-2KB+), so rather than always putting it on the kernel stack, just use the arch_thread FPU state. Change-Id: Ie64c3ea3ca52cd8a4425ae1da69792588a69832a Reviewed-on: https://review.haiku-os.org/c/haiku/+/9090 Reviewed-by: Jérôme Duval Reviewed-by: waddlesplash Tested-by: Commit checker robot --- .../kernel/arch/x86/arch_thread_types.h | 2 +- src/system/kernel/arch/x86/64/interrupts.S | 27 ++++++------------- src/system/kernel/arch/x86/64/thread.cpp | 16 +++++------ src/system/kernel/arch/x86/arch_cpu.cpp | 2 +- .../kernel/arch/x86/arch_user_debugger.cpp | 4 +-- src/system/kernel/arch/x86/asm_offsets.cpp | 2 ++ 6 files changed, 22 insertions(+), 31 deletions(-) diff --git a/headers/private/kernel/arch/x86/arch_thread_types.h b/headers/private/kernel/arch/x86/arch_thread_types.h index 8da03e8e65..f1bdbdfd5b 100644 --- a/headers/private/kernel/arch/x86/arch_thread_types.h +++ b/headers/private/kernel/arch/x86/arch_thread_types.h @@ -63,7 +63,7 @@ struct arch_thread { // have enough space for all the registers, at least 2560 bytes according // to Intel Architecture Instruction Set Extensions Programming Reference, // Section 3.2.4, table 3-8 - uint8 fpu_state[2560] _ALIGNED(64); + uint8 user_fpu_state[2560] _ALIGNED(64); #endif addr_t GetFramePointer() const; diff --git a/src/system/kernel/arch/x86/64/interrupts.S b/src/system/kernel/arch/x86/64/interrupts.S index f8a197a6c8..54d858d9c2 100644 --- a/src/system/kernel/arch/x86/64/interrupts.S +++ b/src/system/kernel/arch/x86/64/interrupts.S @@ -207,7 +207,6 @@ STATIC_FUNCTION(int_bottom): // Push the rest of the interrupt frame to the stack. PUSH_IFRAME_BOTTOM(IFRAME_TYPE_OTHER) - cld // Frame pointer is the iframe. @@ -265,33 +264,24 @@ STATIC_FUNCTION(int_bottom_user): // Frame pointer is the iframe. movq %rsp, %rbp + andq $~15, %rsp + + // Get thread pointer. + movq %gs:0, %r12 - // xsave needs a 64-byte alignment - andq $~63, %rsp - movq (gFPUSaveLength), %rcx - subq %rcx, %rsp - leaq (%rsp), %rdi - shrq $3, %rcx - movq $0, %rax - rep stosq movl (gXsaveMask), %eax movl (gXsaveMask+4), %edx - - movq %rsp, %rdi + leaq THREAD_user_fpu_state(%r12), %rdi CODEPATCH_START fxsaveq (%rdi) CODEPATCH_END(ALTCODEPATCH_TAG_XSAVE) - - movq %rsp, IFRAME_fpu(%rbp) + movq %rdi, IFRAME_fpu(%rbp) // Set the RF (resume flag) in RFLAGS. This prevents an instruction // breakpoint on the instruction we're returning to to trigger a debug // exception. orq $X86_EFLAGS_RESUME, IFRAME_flags(%rbp) - // Get thread pointer. - movq %gs:0, %r12 - STOP_USER_DEBUGGING() UPDATE_THREAD_USER_TIME() @@ -314,7 +304,7 @@ STATIC_FUNCTION(int_bottom_user): movl (gXsaveMask), %eax movl (gXsaveMask+4), %edx - movq %rsp, %rdi + leaq THREAD_user_fpu_state(%r12), %rdi CODEPATCH_START fxrstorq (%rdi) CODEPATCH_END(ALTCODEPATCH_TAG_XRSTOR) @@ -347,7 +337,7 @@ STATIC_FUNCTION(int_bottom_user): 1: movl (gXsaveMask), %eax movl (gXsaveMask+4), %edx - movq %rsp, %rdi + leaq THREAD_user_fpu_state(%r12), %rdi CODEPATCH_START fxrstorq (%rdi) CODEPATCH_END(ALTCODEPATCH_TAG_XRSTOR) @@ -392,7 +382,6 @@ FUNCTION(x86_64_syscall_entry): push $0 // error_code push $99 // vector PUSH_IFRAME_BOTTOM(IFRAME_TYPE_SYSCALL) - cld // Frame pointer is the iframe. diff --git a/src/system/kernel/arch/x86/64/thread.cpp b/src/system/kernel/arch/x86/64/thread.cpp index 64b2b9cd9f..958d25e4fb 100644 --- a/src/system/kernel/arch/x86/64/thread.cpp +++ b/src/system/kernel/arch/x86/64/thread.cpp @@ -181,7 +181,7 @@ arch_thread_init(kernel_args* args) "movl $0x7,%%eax;" \ "movl $0x0,%%edx;" \ "xsavec64 %0" - :: "m" (sInitialState.fpu_state)); + :: "m" (sInitialState.user_fpu_state)); } else { asm volatile ( "clts;" \ @@ -190,7 +190,7 @@ arch_thread_init(kernel_args* args) "movl $0x7,%%eax;" \ "movl $0x0,%%edx;" \ "xsave64 %0" - :: "m" (sInitialState.fpu_state)); + :: "m" (sInitialState.user_fpu_state)); } } else { asm volatile ( @@ -198,11 +198,11 @@ arch_thread_init(kernel_args* args) "fninit;" \ "fnclex;" \ "fxsaveq %0" - :: "m" (sInitialState.fpu_state)); + :: "m" (sInitialState.user_fpu_state)); } // FNINIT does not affect MXCSR or data registers, so we reset them in the state. - savefpu* initialState = ((savefpu*)&sInitialState.fpu_state); + savefpu* initialState = ((savefpu*)&sInitialState.user_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)); @@ -265,7 +265,7 @@ arch_thread_dump_info(void* info) kprintf("\trsp: %p\n", thread->current_stack); kprintf("\tsyscall_rsp: %p\n", thread->syscall_rsp); kprintf("\tuser_rsp: %p\n", thread->user_rsp); - kprintf("\tfpu_state at %p\n", thread->fpu_state); + kprintf("\tuser_fpu_state at %p\n", thread->user_fpu_state); } @@ -374,7 +374,7 @@ arch_setup_signal_frame(Thread* thread, struct sigaction* action, gFPUSaveLength); } else { memcpy((void*)&signalFrameData->context.uc_mcontext.fpu, - sInitialState.fpu_state, gFPUSaveLength); + sInitialState.user_fpu_state, gFPUSaveLength); } // Fill in signalFrameData->context.uc_stack. @@ -446,9 +446,9 @@ arch_restore_signal_frame(struct signal_frame_data* signalFrameData) Thread* thread = thread_get_current_thread(); - memcpy(thread->arch_info.fpu_state, + memcpy(thread->arch_info.user_fpu_state, (void*)&signalFrameData->context.uc_mcontext.fpu, gFPUSaveLength); - frame->fpu = &thread->arch_info.fpu_state; + frame->fpu = &thread->arch_info.user_fpu_state; // The syscall return code overwrites frame->ax with the return value of // the syscall, need to return it here to ensure the correct value is diff --git a/src/system/kernel/arch/x86/arch_cpu.cpp b/src/system/kernel/arch/x86/arch_cpu.cpp index 4e01c91a38..3ccd20adea 100644 --- a/src/system/kernel/arch/x86/arch_cpu.cpp +++ b/src/system/kernel/arch/x86/arch_cpu.cpp @@ -1908,7 +1908,7 @@ arch_cpu_init_post_vm(kernel_args* args) call_all_cpus_sync(&enable_xsavemask, NULL); get_current_cpuid(&cpuid, IA32_CPUID_LEAF_XSTATE, 0); gFPUSaveLength = cpuid.regs.ebx; - if (gFPUSaveLength > sizeof(((struct arch_thread *)0)->fpu_state)) + if (gFPUSaveLength > sizeof(((struct arch_thread *)0)->user_fpu_state)) gFPUSaveLength = 832; arch_altcodepatch_replace(ALTCODEPATCH_TAG_XSAVE, diff --git a/src/system/kernel/arch/x86/arch_user_debugger.cpp b/src/system/kernel/arch/x86/arch_user_debugger.cpp index f1d8dcc6b0..885a0de5aa 100644 --- a/src/system/kernel/arch/x86/arch_user_debugger.cpp +++ b/src/system/kernel/arch/x86/arch_user_debugger.cpp @@ -748,9 +748,9 @@ arch_set_debug_cpu_state(const debug_cpu_state* cpuState) // not use these registers (not even indirectly). #ifdef __x86_64__ Thread* thread = thread_get_current_thread(); - memcpy(thread->arch_info.fpu_state, &cpuState->extended_registers, + memcpy(thread->arch_info.user_fpu_state, &cpuState->extended_registers, sizeof(cpuState->extended_registers)); - frame->fpu = &thread->arch_info.fpu_state; + frame->fpu = &thread->arch_info.user_fpu_state; #else if (gHasSSE) { // Since fxrstor requires 16-byte alignment and this isn't diff --git a/src/system/kernel/arch/x86/asm_offsets.cpp b/src/system/kernel/arch/x86/asm_offsets.cpp index fbb276e7f2..7ef75bd3a9 100644 --- a/src/system/kernel/arch/x86/asm_offsets.cpp +++ b/src/system/kernel/arch/x86/asm_offsets.cpp @@ -49,6 +49,8 @@ dummy() DEFINE_OFFSET_MACRO(THREAD, Thread, fault_handler); #ifdef __x86_64__ + DEFINE_MACRO(THREAD_user_fpu_state, offsetof(Thread, arch_info.user_fpu_state)); + // struct arch_thread DEFINE_OFFSET_MACRO(ARCH_THREAD, arch_thread, syscall_rsp); DEFINE_OFFSET_MACRO(ARCH_THREAD, arch_thread, user_rsp);