From be4e34b6b5afb95433afbb6871ee568dddd12655 Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Wed, 25 Oct 2017 18:57:58 +0200 Subject: [PATCH] hw: unify mmu fault handling Recent work related to issue 1723 showed that there is potential to get rid of code duplication in MMU fault handling especially with regard to ARM cpus. --- repos/base-hw/src/core/kernel/thread.cc | 36 +++++++++++- repos/base-hw/src/core/kernel/thread.h | 26 ++++++--- repos/base-hw/src/core/pager.cc | 10 ++-- repos/base-hw/src/core/pager.h | 15 +---- repos/base-hw/src/core/region_map_support.cc | 5 +- repos/base-hw/src/core/spec/arm/cpu.cc | 44 ++++++++++++-- repos/base-hw/src/core/spec/arm/cpu_support.h | 46 ++------------- .../src/core/spec/arm/kernel/thread.cc | 35 ------------ repos/base-hw/src/core/spec/cortex_a15/cpu.cc | 20 +++++++ repos/base-hw/src/core/spec/cortex_a15/cpu.h | 57 +------------------ repos/base-hw/src/core/spec/riscv/cpu.cc | 7 +++ repos/base-hw/src/core/spec/riscv/cpu.h | 3 + .../src/core/spec/riscv/kernel/thread.cc | 10 ---- repos/base-hw/src/core/spec/x86_64/cpu.cc | 29 ++++++++++ repos/base-hw/src/core/spec/x86_64/cpu.h | 4 ++ .../src/core/spec/x86_64/kernel/thread.cc | 34 ----------- 16 files changed, 169 insertions(+), 212 deletions(-) diff --git a/repos/base-hw/src/core/kernel/thread.cc b/repos/base-hw/src/core/kernel/thread.cc index ea62c8340..20ef5781c 100644 --- a/repos/base-hw/src/core/kernel/thread.cc +++ b/repos/base-hw/src/core/kernel/thread.cc @@ -38,6 +38,20 @@ extern "C" void _core_start(void); using namespace Kernel; +void Thread_fault::print(Genode::Output &out) const +{ + Genode::print(out, "ip=", Genode::Hex(ip)); + Genode::print(out, " fault-addr=", Genode::Hex(addr)); + Genode::print(out, " type="); + switch (type) { + case WRITE: Genode::print(out, "write-fault"); return; + case EXEC: Genode::print(out, "exec-fault"); return; + case PAGE_MISSING: Genode::print(out, "no-page"); return; + case UNKNOWN: Genode::print(out, "unknown"); return; + }; +} + + void Thread::_signal_context_kill_pending() { assert(_state == ACTIVE); @@ -619,11 +633,29 @@ void Thread::_call() } +void Thread::_mmu_exception() +{ + _become_inactive(AWAITS_RESTART); + Cpu::mmu_fault(*regs, _fault); + _fault.ip = regs->ip; + + if (_fault.type == Thread_fault::UNKNOWN) { + Genode::error(*this, " raised unhandled MMU fault ", _fault); + return; + } + + if (_core) + Genode::error(*this, " raised a fault, which should never happen ", + _fault); + + if (_pager) _pager->submit(1); +} + + Thread::Thread(unsigned const priority, unsigned const quota, char const * const label, bool core) : - Cpu_job(priority, quota), _fault_pd(0), _fault_addr(0), - _fault_writes(0), _state(AWAITS_START), + Cpu_job(priority, quota), _state(AWAITS_START), _signal_receiver(0), _label(label), _core(core), regs(core) { } diff --git a/repos/base-hw/src/core/kernel/thread.h b/repos/base-hw/src/core/kernel/thread.h index 39d75e6b5..d7534949a 100644 --- a/repos/base-hw/src/core/kernel/thread.h +++ b/repos/base-hw/src/core/kernel/thread.h @@ -24,10 +24,24 @@ namespace Kernel { + struct Thread_fault; class Thread; class Core_thread; } + +struct Kernel::Thread_fault +{ + enum Type { WRITE, EXEC, PAGE_MISSING, UNKNOWN }; + + addr_t ip = 0; + addr_t addr = 0; + Type type = UNKNOWN; + + void print(Genode::Output &out) const; +}; + + /** * Kernel back-end for userland execution-contexts */ @@ -53,9 +67,7 @@ class Kernel::Thread }; Signal_context * _pager = nullptr; - addr_t _fault_pd; - addr_t _fault_addr; - addr_t _fault_writes; + Thread_fault _fault; State _state; Signal_receiver * _signal_receiver; char const * const _label; @@ -63,7 +75,6 @@ class Kernel::Thread bool _paused = false; bool _cancel_next_await_signal = false; bool const _core = false; - bool _fault_exec = false; /** * Notice that another thread yielded the CPU to this thread @@ -322,11 +333,8 @@ class Kernel::Thread ** Accessors ** ***************/ - char const * label() const { return _label; } - addr_t fault_pd() const { return _fault_pd; } - addr_t fault_addr() const { return _fault_addr; } - addr_t fault_writes() const { return _fault_writes; } - bool fault_exec() const { return _fault_exec; } + char const * label() const { return _label; } + Thread_fault fault() const { return _fault; } }; diff --git a/repos/base-hw/src/core/pager.cc b/repos/base-hw/src/core/pager.cc index 452243556..1a8ab38aa 100644 --- a/repos/base-hw/src/core/pager.cc +++ b/repos/base-hw/src/core/pager.cc @@ -34,9 +34,11 @@ addr_t Ipc_pager::fault_ip() const { return _fault.ip; } addr_t Ipc_pager::fault_addr() const { return _fault.addr; } -bool Ipc_pager::write_fault() const { return _fault.writes; } +bool Ipc_pager::write_fault() const { + return _fault.type == Kernel::Thread_fault::WRITE; } -bool Ipc_pager::exec_fault() const { return _fault.exec; } +bool Ipc_pager::exec_fault() const { + return _fault.type == Kernel::Thread_fault::EXEC; } void Ipc_pager::set_reply_mapping(Mapping m) { _mapping = m; } @@ -67,9 +69,7 @@ void Pager_object::unresolved_page_fault_occurred() Platform_thread * const pt = (Platform_thread *)badge(); if (pt && pt->pd()) warning("page fault, pager_object: pd='", pt->pd()->label(), - "' thread='", pt->label(), - "' ip=", Hex(pt->kernel_object()->regs->ip), - " pf-addr=", Hex(pt->kernel_object()->fault_addr())); + "' thread='", pt->label(), " ", pt->kernel_object()->fault()); } void Pager_object::print(Output &out) const diff --git a/repos/base-hw/src/core/pager.h b/repos/base-hw/src/core/pager.h index 4e8c78563..d25c38aca 100644 --- a/repos/base-hw/src/core/pager.h +++ b/repos/base-hw/src/core/pager.h @@ -80,19 +80,8 @@ class Genode::Ipc_pager { protected: - /** - * Page-fault data that is read from the faulters thread registers - */ - struct Fault_thread_regs - { - addr_t ip; - addr_t addr; - addr_t writes; - addr_t exec; - addr_t signal; - } _fault; - - Mapping _mapping; + Kernel::Thread_fault _fault; + Mapping _mapping; public: diff --git a/repos/base-hw/src/core/region_map_support.cc b/repos/base-hw/src/core/region_map_support.cc index d416fe21a..aa840e127 100644 --- a/repos/base-hw/src/core/region_map_support.cc +++ b/repos/base-hw/src/core/region_map_support.cc @@ -50,10 +50,7 @@ void Pager_entrypoint::entry() continue; } - _fault.ip = pt->kernel_object()->regs->ip; - _fault.addr = pt->kernel_object()->fault_addr(); - _fault.writes = pt->kernel_object()->fault_writes(); - _fault.exec = pt->kernel_object()->fault_exec(); + _fault = pt->kernel_object()->fault(); /* try to resolve fault directly via local region managers */ if (po->pager(*this)) continue; diff --git a/repos/base-hw/src/core/spec/arm/cpu.cc b/repos/base-hw/src/core/spec/arm/cpu.cc index 5c5e854bb..27184097b 100644 --- a/repos/base-hw/src/core/spec/arm/cpu.cc +++ b/repos/base-hw/src/core/spec/arm/cpu.cc @@ -15,9 +15,12 @@ #include #include +#include #include -Genode::Arm_cpu::Context::Context(bool privileged) +using namespace Genode; + +Arm_cpu::Context::Context(bool privileged) { using Psr = Arm_cpu::Psr; @@ -31,14 +34,14 @@ Genode::Arm_cpu::Context::Context(bool privileged) } -using Asid_allocator = Genode::Bit_allocator<256>; +using Asid_allocator = Bit_allocator<256>; static Asid_allocator &alloc() { return *unmanaged_singleton(); } -Genode::Arm_cpu::Mmu_context::Mmu_context(addr_t table) -: cidr((Genode::uint8_t)alloc().alloc()), ttbr0(Ttbr0::init(table)) { } +Arm_cpu::Mmu_context::Mmu_context(addr_t table) +: cidr((uint8_t)alloc().alloc()), ttbr0(Ttbr0::init(table)) { } Genode::Arm_cpu::Mmu_context::~Mmu_context() @@ -47,3 +50,36 @@ Genode::Arm_cpu::Mmu_context::~Mmu_context() Cpu::Tlbiasid::write(id()); alloc().free(id()); } + + +using Thread_fault = Kernel::Thread_fault; + +void Arm_cpu::mmu_fault(Context & c, Thread_fault & fault) +{ + bool prefetch = c.cpu_exception == Context::PREFETCH_ABORT; + fault.addr = prefetch ? Ifar::read() : Dfar::read(); + Fsr::access_t fsr = prefetch ? Ifsr::read() : Dfsr::read(); + + if (!prefetch && Dfsr::Wnr::get(fsr)) { + fault.type = Thread_fault::WRITE; + return; + } + + Cpu::mmu_fault_status(Fsr::Fs::get(fsr), fault); +} + + +void Arm_cpu::mmu_fault_status(Fsr::access_t fsr, Thread_fault & fault) +{ + enum { + FAULT_MASK = 0b11101, + TRANSLATION = 0b00101, + PERMISSION = 0b01101, + }; + + switch(fsr & FAULT_MASK) { + case TRANSLATION: fault.type = Thread_fault::PAGE_MISSING; return; + case PERMISSION: fault.type = Thread_fault::EXEC; return; + default: fault.type = Thread_fault::UNKNOWN; + }; +} diff --git a/repos/base-hw/src/core/spec/arm/cpu_support.h b/repos/base-hw/src/core/spec/arm/cpu_support.h index 2fa009f27..fa5061d17 100644 --- a/repos/base-hw/src/core/spec/arm/cpu_support.h +++ b/repos/base-hw/src/core/spec/arm/cpu_support.h @@ -28,6 +28,8 @@ #include #include +namespace Kernel { struct Thread_fault; } + namespace Genode { using sizet_arithm_t = Genode::uint64_t; struct Arm_cpu; @@ -137,48 +139,10 @@ struct Genode::Arm_cpu : public Hw::Arm_cpu } } - static bool in_fault(Context & c, addr_t & va, addr_t & w, bool & p) - { - /* translation fault on section */ - static constexpr Fsr::access_t section = 5; - /* translation fault on page */ - static constexpr Fsr::access_t page = 7; - /* permission fault on page */ - static constexpr Fsr::access_t permission = 0xf; + static void mmu_fault(Context & c, Kernel::Thread_fault & fault); + static void mmu_fault_status(Fsr::access_t fsr, + Kernel::Thread_fault & fault); - if (c.cpu_exception == Context::PREFETCH_ABORT) { - /* check if fault was caused by a translation miss */ - Ifsr::access_t const fs = Fsr::Fs::get(Ifsr::read()); - - if (fs == permission) { - w = 0; - va = Ifar::read(); - p = true; - return true; - } - - if (fs != section && fs != page) - return false; - - /* fetch fault data */ - w = 0; - va = Ifar::read(); - p = false; - return true; - } else { - /* check if fault is of known type */ - Dfsr::access_t const fs = Fsr::Fs::get(Dfsr::read()); - if (fs != permission && fs != section && fs != page) - return false; - - /* fetch fault data */ - Dfsr::access_t const dfsr = Dfsr::read(); - w = Dfsr::Wnr::get(dfsr); - va = Dfar::read(); - p = false; - return true; - } - } /************* ** Dummies ** diff --git a/repos/base-hw/src/core/spec/arm/kernel/thread.cc b/repos/base-hw/src/core/spec/arm/kernel/thread.cc index 0f7707bc1..722c0f0fc 100644 --- a/repos/base-hw/src/core/spec/arm/kernel/thread.cc +++ b/repos/base-hw/src/core/spec/arm/kernel/thread.cc @@ -50,41 +50,6 @@ void Thread::exception(Cpu & cpu) } -void Thread::_mmu_exception() -{ - _become_inactive(AWAITS_RESTART); - if (Cpu::in_fault(*regs, _fault_addr, _fault_writes, _fault_exec)) { - _fault_pd = (addr_t)_pd->platform_pd(); - - /* - * Core should never raise a page-fault. If this happens, print out an - * error message with debug information. - */ - if (_core) - Genode::error("page fault in core thread (", label(), "): " - "ip=", Genode::Hex(regs->ip), " fault=", Genode::Hex(_fault_addr)); - - if (_pager) _pager->submit(1); - return; - } - - char const *abort_type = "unknown"; - if (regs->cpu_exception == Cpu::Context::DATA_ABORT) - abort_type = "data"; - if (regs->cpu_exception == Cpu::Context::PREFETCH_ABORT) - abort_type = "prefetch"; - - Genode::error(*this, ": raised unhandled ", - abort_type, " abort ", - "DFSR=", Genode::Hex(Cpu::Dfsr::read()), " " - "ISFR=", Genode::Hex(Cpu::Ifsr::read()), " " - "DFAR=", Genode::Hex(Cpu::Dfar::read()), " " - "ip=", Genode::Hex(regs->ip), " " - "sp=", Genode::Hex(regs->sp), " " - "exception=", Genode::Hex(regs->cpu_exception)); -} - - void Kernel::Thread::_call_update_data_region() { Cpu * const cpu = cpu_pool()->cpu(Cpu::executing_id()); diff --git a/repos/base-hw/src/core/spec/cortex_a15/cpu.cc b/repos/base-hw/src/core/spec/cortex_a15/cpu.cc index 422f39429..406bad71c 100644 --- a/repos/base-hw/src/core/spec/cortex_a15/cpu.cc +++ b/repos/base-hw/src/core/spec/cortex_a15/cpu.cc @@ -14,6 +14,7 @@ #include #include +#include #include using Asid_allocator = Genode::Bit_allocator<256>; @@ -33,3 +34,22 @@ Genode::Cpu::Mmu_context::~Mmu_context() Cpu::Tlbiasid::write(id()); alloc().free(id()); } + + +void Genode::Cpu::mmu_fault_status(Genode::Cpu::Fsr::access_t fsr, + Kernel::Thread_fault & fault) +{ + enum { + FAULT_MASK = 0b111100, + TRANSLATION = 0b000100, + PERMISSION = 0b001100, + }; + + using Fault = Kernel::Thread_fault; + + switch(fsr & FAULT_MASK) { + case TRANSLATION: fault.type = Fault::PAGE_MISSING; return; + case PERMISSION: fault.type = Fault::EXEC; return; + default: fault.type = Fault::UNKNOWN; + }; +}; diff --git a/repos/base-hw/src/core/spec/cortex_a15/cpu.h b/repos/base-hw/src/core/spec/cortex_a15/cpu.h index 87605156e..0d725f4b9 100644 --- a/repos/base-hw/src/core/spec/cortex_a15/cpu.h +++ b/repos/base-hw/src/core/spec/cortex_a15/cpu.h @@ -105,61 +105,8 @@ class Genode::Cpu : public Arm_v7_cpu Genode::uint8_t id() const { return Ttbr_64bit::Asid::get(ttbr0); } }; - - /** - * Return if the context is in a page fault due to translation miss - * - * \param va holds the virtual fault-address if call returns 1 - * \param w holds wether it's a write fault if call returns 1 - * \param p holds whether it's a permission fault if call returns 1 - */ - static bool in_fault(Context & c, addr_t & va, addr_t & w, bool & p) - { - /* permission fault on page, 2nd level */ - static constexpr Fsr::access_t permission = 0b1111; - - switch (c.cpu_exception) { - - case Context::PREFETCH_ABORT: - { - /* check if fault was caused by a translation miss */ - Fsr::access_t const fs = Fsr::Fs::get(Ifsr::read()); - if (fs == permission) { - w = 0; - va = Ifar::read(); - p = true; - return true; - } - - if ((fs & 0b11100) != 0b100) return false; - - /* fetch fault data */ - w = 0; - va = Ifar::read(); - p = false; - return true; - } - - case Context::DATA_ABORT: - { - /* check if fault was caused by translation miss */ - Fsr::access_t const fs = Fsr::Fs::get(Dfsr::read()); - if ((fs != permission) && (fs & 0b11100) != 0b100) - return false; - - /* fetch fault data */ - Dfsr::access_t const dfsr = Dfsr::read(); - w = Dfsr::Wnr::get(dfsr); - va = Dfar::read(); - p = false; - return true; - } - - default: - return false; - }; - }; - + static void mmu_fault_status(Fsr::access_t fsr, + Kernel::Thread_fault & fault); /** * Return kernel name of the executing CPU diff --git a/repos/base-hw/src/core/spec/riscv/cpu.cc b/repos/base-hw/src/core/spec/riscv/cpu.cc index dcde9c77b..394d74991 100644 --- a/repos/base-hw/src/core/spec/riscv/cpu.cc +++ b/repos/base-hw/src/core/spec/riscv/cpu.cc @@ -67,3 +67,10 @@ void Genode::Cpu::switch_to(Mmu_context & context) if (user /*&& sptbr != context.sptbr*/) Sptbr::write(context.sptbr); } + + +void Genode::Cpu::mmu_fault(Context & c, Kernel::Thread_fault & f) +{ + f.addr = Genode::Cpu::Sbadaddr::read(); + f.type = Kernel::Thread_fault::PAGE_MISSING; +} diff --git a/repos/base-hw/src/core/spec/riscv/cpu.h b/repos/base-hw/src/core/spec/riscv/cpu.h index 301bf570e..40667c5fc 100644 --- a/repos/base-hw/src/core/spec/riscv/cpu.h +++ b/repos/base-hw/src/core/spec/riscv/cpu.h @@ -24,6 +24,8 @@ #include #include +namespace Kernel { struct Thread_fault; } + namespace Genode { /** @@ -76,6 +78,7 @@ class Genode::Cpu : public Hw::Riscv_cpu static void invalidate_tlb_by_pid(unsigned const pid) { sfence(); } void switch_to(Mmu_context & context); + static void mmu_fault(Context & c, Kernel::Thread_fault & f); static unsigned executing_id() { return 0; } static unsigned primary_id() { return 0; } diff --git a/repos/base-hw/src/core/spec/riscv/kernel/thread.cc b/repos/base-hw/src/core/spec/riscv/kernel/thread.cc index 239166db2..774581edf 100644 --- a/repos/base-hw/src/core/spec/riscv/kernel/thread.cc +++ b/repos/base-hw/src/core/spec/riscv/kernel/thread.cc @@ -45,16 +45,6 @@ void Thread::exception(Cpu&) } -void Thread::_mmu_exception() -{ - _become_inactive(AWAITS_RESTART); - _fault_pd = (addr_t)_pd->platform_pd(); - _fault_addr = Genode::Cpu::Sbadaddr::read(); - - if (_pager) _pager->submit(1); -} - - void Thread::_call_update_pd() { Genode::Cpu::sfence(); 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 62b6acfee..d31e44083 100644 --- a/repos/base-hw/src/core/spec/x86_64/cpu.cc +++ b/repos/base-hw/src/core/spec/x86_64/cpu.cc @@ -13,6 +13,7 @@ /* core includes */ #include +#include #include extern int __tss; @@ -56,3 +57,31 @@ void Genode::Cpu::Gdt::init() uint64_t const base = start; asm volatile ("lgdt %0" :: "m" (Pseudo_descriptor(limit, base))); } + + +void Genode::Cpu::mmu_fault(Context & regs, Kernel::Thread_fault & fault) +{ + using Fault = Kernel::Thread_fault::Type; + + /* + * Intel manual: 6.15 EXCEPTION AND INTERRUPT REFERENCE + * Interrupt 14—Page-Fault Exception (#PF) + */ + enum { + ERR_I = 1UL << 4, + ERR_R = 1UL << 3, + ERR_U = 1UL << 2, + ERR_W = 1UL << 1, + ERR_P = 1UL << 0, + }; + + auto fault_lambda = [] (addr_t err) { + if ((err & ERR_P) && (err & ERR_W)) return Fault::WRITE; + if ((err & ERR_P) && (err & ERR_I)) return Fault::EXEC; + if (err & ERR_P) return Fault::UNKNOWN; + else return Fault::PAGE_MISSING; + }; + + fault.addr = Genode::Cpu::Cr2::read(); + fault.type = fault_lambda(regs.errcode); +} 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 b9bce5b92..961374f1c 100644 --- a/repos/base-hw/src/core/spec/x86_64/cpu.h +++ b/repos/base-hw/src/core/spec/x86_64/cpu.h @@ -29,6 +29,8 @@ /* core includes */ #include +namespace Kernel { struct Thread_fault; } + namespace Genode { class Cpu; using sizet_arithm_t = __uint128_t; @@ -124,6 +126,8 @@ class Genode::Cpu * \param context next CPU context */ inline void switch_to(Context & context, Mmu_context &); + + static void mmu_fault(Context & regs, Kernel::Thread_fault & fault); }; 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 01fbeb626..e7cff13d6 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 @@ -25,40 +25,6 @@ void Kernel::Thread::_call_update_data_region() { } void Kernel::Thread::_call_update_instr_region() { } -/* - * Intel manual: 6.15 EXCEPTION AND INTERRUPT REFERENCE - * Interrupt 14—Page-Fault Exception (#PF) - */ -enum { - ERR_I = 1UL << 4, - ERR_R = 1UL << 3, - ERR_U = 1UL << 2, - ERR_W = 1UL << 1, - ERR_P = 1UL << 0, -}; - - -void Kernel::Thread::_mmu_exception() -{ - _become_inactive(AWAITS_RESTART); - _fault_pd = (addr_t)_pd->platform_pd(); - _fault_addr = Genode::Cpu::Cr2::read(); - _fault_writes = (regs->errcode & ERR_P) && (regs->errcode & ERR_W); - _fault_exec = (regs->errcode & ERR_P) && (regs->errcode & ERR_I); - - /* - * Core should never raise a page-fault. If this happens, print out an - * error message with debug information. - */ - if (_pd == Kernel::core_pd()) - Genode::error("page fault in core thread (", label(), "): " - "ip=", Genode::Hex(regs->ip), " fault=", Genode::Hex(_fault_addr)); - - if (_pager) _pager->submit(1); - return; -} - - void Kernel::Thread::_call_update_pd() { }