From da17f2cbd3bd5f230df8bf6be03799ac868fcac1 Mon Sep 17 00:00:00 2001 From: Sebastian Sumpf Date: Thu, 23 May 2019 18:09:42 +0200 Subject: [PATCH] hw: eager FPU switching for x86_64 Since gcc 8.3.0 generates SSE instructions into kernel code, the kernel itself may raise FPU exceptions and/or corrupt user level FPU contexts thereby. Both things are not feasible, and therefore, lazy FPU switching becomes a no go for base-hw because we cannot avoid FPU instructions because of the entanglement of base-hw, base, and the tool chain (libgcc_eh.a). issue #3365 --- .../lib/mk/spec/x86_64/core-hw-muen.mk | 1 - .../base-hw/lib/mk/spec/x86_64/core-hw-pc.mk | 1 - repos/base-hw/src/core/spec/x86_64/cpu.cc | 2 - repos/base-hw/src/core/spec/x86_64/cpu.h | 7 +- .../src/core/spec/x86_64/exception_vector.s | 7 + repos/base-hw/src/core/spec/x86_64/fpu.cc | 61 ------- repos/base-hw/src/core/spec/x86_64/fpu.h | 155 +++++------------- .../src/core/spec/x86_64/kernel/cpu.cc | 2 - .../src/core/spec/x86_64/kernel/thread.cc | 6 +- .../spec/x86_64/kernel/thread_exception.cc | 5 - .../x86_64/muen/kernel/thread_exception.cc | 5 - 11 files changed, 49 insertions(+), 203 deletions(-) delete mode 100644 repos/base-hw/src/core/spec/x86_64/fpu.cc diff --git a/repos/base-hw/lib/mk/spec/x86_64/core-hw-muen.mk b/repos/base-hw/lib/mk/spec/x86_64/core-hw-muen.mk index dfb588358..1eb9c661d 100644 --- a/repos/base-hw/lib/mk/spec/x86_64/core-hw-muen.mk +++ b/repos/base-hw/lib/mk/spec/x86_64/core-hw-muen.mk @@ -25,7 +25,6 @@ SRC_CC += spec/x86/io_port_session_component.cc SRC_CC += spec/x86/io_port_session_support.cc SRC_CC += spec/x86_64/bios_data_area.cc SRC_CC += spec/x86_64/cpu.cc -SRC_CC += spec/x86_64/fpu.cc SRC_CC += spec/x86_64/kernel/cpu.cc SRC_CC += spec/x86_64/kernel/pd.cc SRC_CC += spec/x86_64/kernel/thread.cc diff --git a/repos/base-hw/lib/mk/spec/x86_64/core-hw-pc.mk b/repos/base-hw/lib/mk/spec/x86_64/core-hw-pc.mk index 25c0174fc..7e66f65f1 100644 --- a/repos/base-hw/lib/mk/spec/x86_64/core-hw-pc.mk +++ b/repos/base-hw/lib/mk/spec/x86_64/core-hw-pc.mk @@ -26,7 +26,6 @@ SRC_CC += spec/x86/io_port_session_component.cc SRC_CC += spec/x86/io_port_session_support.cc SRC_CC += spec/x86_64/bios_data_area.cc SRC_CC += spec/x86_64/cpu.cc -SRC_CC += spec/x86_64/fpu.cc SRC_CC += spec/x86_64/kernel/cpu.cc SRC_CC += spec/x86_64/kernel/pd.cc SRC_CC += spec/x86_64/kernel/thread.cc diff --git a/repos/base-hw/src/core/spec/x86_64/cpu.cc b/repos/base-hw/src/core/spec/x86_64/cpu.cc index 6416537ee..54a2db435 100644 --- a/repos/base-hw/src/core/spec/x86_64/cpu.cc +++ b/repos/base-hw/src/core/spec/x86_64/cpu.cc @@ -111,8 +111,6 @@ extern Genode::size_t const kernel_stack_size; void Genode::Cpu::switch_to(Context & context, Mmu_context &mmu_context) { - _fpu.switch_to(context); - if ((context.cs != 0x8) && (mmu_context.cr3 != Cr3::read())) Cr3::write(mmu_context.cr3); diff --git a/repos/base-hw/src/core/spec/x86_64/cpu.h b/repos/base-hw/src/core/spec/x86_64/cpu.h index f88a5df9e..de5fd8273 100644 --- a/repos/base-hw/src/core/spec/x86_64/cpu.h +++ b/repos/base-hw/src/core/spec/x86_64/cpu.h @@ -43,8 +43,6 @@ class Genode::Cpu : public Hw::X86_64_cpu { protected: - Fpu _fpu { }; - public: /** @@ -95,7 +93,7 @@ class Genode::Cpu : public Hw::X86_64_cpu struct Kernel_stack { unsigned long kernel_stack { }; }; /* exception_vector.s depends on the position of the Kernel_stack */ - struct alignas(16) Context : Cpu_state, Kernel_stack, Fpu::Context + struct alignas(16) Context : Cpu_state, Kernel_stack, Fpu_context { enum Eflags { EFLAGS_IF_SET = 1 << 9, @@ -113,9 +111,6 @@ class Genode::Cpu : public Hw::X86_64_cpu Mmu_context(addr_t page_table_base); }; - - Fpu & fpu() { return _fpu; } - /** * Return kernel name of the executing CPU */ diff --git a/repos/base-hw/src/core/spec/x86_64/exception_vector.s b/repos/base-hw/src/core/spec/x86_64/exception_vector.s index 9e4799df6..3d3e8ebcd 100644 --- a/repos/base-hw/src/core/spec/x86_64/exception_vector.s +++ b/repos/base-hw/src/core/spec/x86_64/exception_vector.s @@ -119,8 +119,15 @@ .set REGISTER_SIZE, 8 .set SIZEOF_CPU_STATE, REGISTER_COUNT * REGISTER_SIZE /* sizeof (Cpu_state) */ .set KERNEL_STACK_OFFSET, SIZEOF_CPU_STATE + .set FPU_CONTEXT_OFFSET, KERNEL_STACK_OFFSET + 8 /* rsp contains pointer to Cpu::Context */ + /* save FPU context */ + movq %rsp, %rax + addq $FPU_CONTEXT_OFFSET, %rax + movq (%rax), %rax + fxsave (%rax) + /* Restore kernel stack and continue kernel execution */ movq %rsp, %rax addq $KERNEL_STACK_OFFSET, %rax diff --git a/repos/base-hw/src/core/spec/x86_64/fpu.cc b/repos/base-hw/src/core/spec/x86_64/fpu.cc deleted file mode 100644 index 3e7d64db7..000000000 --- a/repos/base-hw/src/core/spec/x86_64/fpu.cc +++ /dev/null @@ -1,61 +0,0 @@ -/* - * \brief FPU implementation specific to x86_64 - * \author Stefan Kalkowski - * \date 2016-01-19 - */ - -/* - * Copyright (C) 2016-2017 Genode Labs GmbH - * - * This file is part of the Genode OS framework, which is distributed - * under the terms of the GNU Affero General Public License version 3. - */ - -/* core includes */ -#include - -void Genode::Fpu::init() -{ - Cpu::Cr0::access_t cr0_value = Cpu::Cr0::read(); - Cpu::Cr4::access_t cr4_value = Cpu::Cr4::read(); - - Cpu::Cr0::Mp::set(cr0_value); - Cpu::Cr0::Em::clear(cr0_value); - - /* - * Clear task switched so we do not gnerate FPU faults during kernel - * initialisation, it will be turned on by Fpu::disable - */ - Cpu::Cr0::Ts::clear(cr0_value); - Cpu::Cr0::Ne::set(cr0_value); - Cpu::Cr0::write(cr0_value); - - Cpu::Cr4::Osfxsr::set(cr4_value); - Cpu::Cr4::Osxmmexcpt::set(cr4_value); - Cpu::Cr4::write(cr4_value); -} - - -void Genode::Fpu::disable() { - Cpu::Cr0::write(Cpu::Cr0::read() | Cpu::Cr0::Ts::bits(1)); } - - -bool Genode::Fpu::enabled() { return !Cpu::Cr0::Ts::get(Cpu::Cr0::read()); } - - -bool Genode::Fpu::fault(Context &context) -{ - if (enabled()) return false; - - enable(); - if (_context == &context) return true; - - if (_context) { - _save(); - _context->_fpu = nullptr; - } - _context = &context; - _context->_fpu = this; - _load(); - return true; -} diff --git a/repos/base-hw/src/core/spec/x86_64/fpu.h b/repos/base-hw/src/core/spec/x86_64/fpu.h index 87468638f..6f5ab8a0f 100644 --- a/repos/base-hw/src/core/spec/x86_64/fpu.h +++ b/repos/base-hw/src/core/spec/x86_64/fpu.h @@ -1,14 +1,11 @@ /* - * \brief x86_64 FPU driver for core - * \author Adrian-Ken Rueegsegger - * \author Martin stein - * \author Reto Buerki - * \author Stefan Kalkowski - * \date 2016-01-19 + * \brief x86_64 FPU context + * \author Sebastian Sumpf + * \date 2019-05-23 */ /* - * Copyright (C) 2016-2017 Genode Labs GmbH + * Copyright (C) 2019 Genode Labs GmbH * * This file is part of the Genode OS framework, which is distributed * under the terms of the GNU Affero General Public License version 3. @@ -18,128 +15,50 @@ #define _CORE__SPEC__X86_64__FPU_H_ /* Genode includes */ -#include +#include +#include +#include -namespace Genode { class Fpu; } +namespace Genode { class Fpu_context; } -class Genode::Fpu +class Genode::Fpu_context { - public: + addr_t _fxsave_addr { 0 }; - class Context + /* + * FXSAVE area providing storage for x87 FPU, MMX, XMM, + * and MXCSR registers. + * + * For further details see Intel SDM Vol. 2A, + * 'FXSAVE instruction'. + */ + char _fxsave_area[527]; + + struct Context : Mmio + { + struct Fcw : Register<0, 16> { }; + struct Mxcsr : Register<24, 32> { }; + + Context(addr_t const base) : Mmio(base) { - private: - - friend class Fpu; - - /* - * FXSAVE area providing storage for x87 FPU, MMX, XMM, - * and MXCSR registers. - * - * For further details see Intel SDM Vol. 2A, - * 'FXSAVE instruction'. - */ - char _fxsave_area[527]; - - /* 16-byte aligned start of FXSAVE area. */ - char * _start = nullptr; - - Fpu * _fpu = nullptr; - - bool _init() - { - if (_start) return true; - - _start = ((addr_t)_fxsave_area & 15) - ? (char *)((addr_t)_fxsave_area & ~15) + 16 - : _fxsave_area; - return false; - } - - public: - - ~Context() { if (_fpu) _fpu->unset(*this); } - }; - - private: - - enum { MXCSR_DEFAULT = 0x1f80 }; - - Context * _context = nullptr; - - /** - * Reset FPU - * - * Doesn't check for pending unmasked floating-point - * exceptions and explicitly sets the MXCSR to the - * default value. - */ - void _reset() - { - unsigned value = MXCSR_DEFAULT; - asm volatile ("fninit"); - asm volatile ("ldmxcsr %0" : : "m" (value)); + memset((void *)base, 0, 512); + write(0x37f); /* mask exceptions SysV ABI */ + write(0x1f80); } - - /** - * Load x87 FPU context - */ - void _load() - { - if (!_context->_init()) _reset(); - else asm volatile ("fxrstor %0" : : "m" (*_context->_start)); - } - - /** - * Save x87 FPU context - */ - void _save() { - asm volatile ("fxsave %0" : "=m" (*_context->_start)); } + }; public: - /** - * Disable FPU by setting the TS flag in CR0. - */ - void disable(); + Fpu_context() + { + /* align to 16 byte boundary */ + _fxsave_addr = align_addr((addr_t)_fxsave_area, 4); - /** - * Enable FPU by clearing the TS flag in CR0. - */ - void enable() { asm volatile ("clts"); } + /* set fcw/mxcsr */ + Context init(_fxsave_addr); + } - /** - * Initialize all FPU-related CR flags - * - * Initialize FPU with SSE extensions by setting required CR0 and CR4 - * bits to configure the FPU environment according to Intel SDM Vol. - * 3A, sections 9.2 and 9.6. - */ - void init(); - - /** - * Returns True if the FPU is enabled. - */ - bool enabled(); - - /** - * Switch to new context - * - * \param context next FPU context - */ - void switch_to(Context &context) { - if (_context != &context) disable(); } - - /** - * Return whether to retry an FPU instruction after this call - */ - bool fault(Context &context); - - /** - * Unset FPU context - */ - void unset(Context &context) { - if (_context == &context) _context = nullptr; } + addr_t fpu_context() const { return _fxsave_addr; } }; #endif /* _CORE__SPEC__X86_64__FPU_H_ */ diff --git a/repos/base-hw/src/core/spec/x86_64/kernel/cpu.cc b/repos/base-hw/src/core/spec/x86_64/kernel/cpu.cc index f3e6e23c3..3a0fc5fda 100644 --- a/repos/base-hw/src/core/spec/x86_64/kernel/cpu.cc +++ b/repos/base-hw/src/core/spec/x86_64/kernel/cpu.cc @@ -24,8 +24,6 @@ void Kernel::Cpu::_arch_init() Timer::init_cpu_local(); - fpu().init(); - /* enable timer interrupt */ _pic.store_apic_id(id()); _pic.unmask(_timer.interrupt_id(), id()); diff --git a/repos/base-hw/src/core/spec/x86_64/kernel/thread.cc b/repos/base-hw/src/core/spec/x86_64/kernel/thread.cc index f6310770c..d1df6e510 100644 --- a/repos/base-hw/src/core/spec/x86_64/kernel/thread.cc +++ b/repos/base-hw/src/core/spec/x86_64/kernel/thread.cc @@ -41,7 +41,8 @@ void Kernel::Thread::proceed(Cpu & cpu) { cpu.switch_to(*regs, pd().mmu_regs); - asm volatile("mov %0, %%rsp \n" + asm volatile("fxrstor (%1) \n" + "mov %0, %%rsp \n" "popq %%r8 \n" "popq %%r9 \n" "popq %%r10 \n" @@ -58,7 +59,8 @@ void Kernel::Thread::proceed(Cpu & cpu) "popq %%rsi \n" "popq %%rbp \n" "add $16, %%rsp \n" - "iretq \n" :: "r" (®s->r8)); + "iretq \n" + :: "r" (®s->r8), "r" (regs->fpu_context())); } diff --git a/repos/base-hw/src/core/spec/x86_64/kernel/thread_exception.cc b/repos/base-hw/src/core/spec/x86_64/kernel/thread_exception.cc index e7f2ee8fb..aedb2320a 100644 --- a/repos/base-hw/src/core/spec/x86_64/kernel/thread_exception.cc +++ b/repos/base-hw/src/core/spec/x86_64/kernel/thread_exception.cc @@ -27,11 +27,6 @@ void Thread::exception(Cpu & cpu) case Cpu_state::PAGE_FAULT: _mmu_exception(); return; - case Cpu_state::NO_MATH_COPROC: - if (_cpu->fpu().fault(*regs)) { return; } - Genode::raw(*this, ": FPU error"); - _die(); - return; case Cpu_state::UNDEFINED_INSTRUCTION: Genode::raw(*this, ": undefined instruction at ip=", (void*)regs->ip); _die(); diff --git a/repos/base-hw/src/core/spec/x86_64/muen/kernel/thread_exception.cc b/repos/base-hw/src/core/spec/x86_64/muen/kernel/thread_exception.cc index ee8779f80..6890e229a 100644 --- a/repos/base-hw/src/core/spec/x86_64/muen/kernel/thread_exception.cc +++ b/repos/base-hw/src/core/spec/x86_64/muen/kernel/thread_exception.cc @@ -25,11 +25,6 @@ void Thread::exception(Cpu & cpu) case Cpu::Context::PAGE_FAULT: _mmu_exception(); return; - case Cpu::Context::NO_MATH_COPROC: - if (_cpu->fpu().fault(*regs)) { return; } - Genode::raw(*this, ": FPU error"); - _die(); - return; case Cpu::Context::UNDEFINED_INSTRUCTION: Genode::raw(*this, ": undefined instruction at ip=", (void*)regs->ip); _die();