From d164cbac8c54f42a19a7fc38365eecc74ce9e755 Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Wed, 8 Nov 2017 15:17:15 +0100 Subject: [PATCH] hw: do not change x86 paging attributes on fly Instead of changing the attributes (e.g., Xd bit) of the top-level page-tables, set them to allow everything. Only leafs of the paging hierarchy are set according to the paging attributes given by core. Otherwise, top-level page- table attributes are changed during lifetime, which requires a TLB flush operation (not intended in the semantic of the kernel/core). This led to problems when using the non-executable features introduced by issue #1723 in the recent past. --- .../base-hw/src/bootstrap/spec/x86_64/board.h | 2 -- repos/base-hw/src/core/spec/x86_64/cpu.cc | 8 ++--- .../src/core/spec/x86_64/kernel/thread.cc | 4 ++- .../src/core/spec/x86_64/translation_table.h | 8 ----- .../src/lib/hw/spec/x86_64/page_table.h | 31 +++++-------------- 5 files changed, 15 insertions(+), 38 deletions(-) diff --git a/repos/base-hw/src/bootstrap/spec/x86_64/board.h b/repos/base-hw/src/bootstrap/spec/x86_64/board.h index 4c3ed5157..8f54f6645 100644 --- a/repos/base-hw/src/bootstrap/spec/x86_64/board.h +++ b/repos/base-hw/src/bootstrap/spec/x86_64/board.h @@ -20,8 +20,6 @@ #include #include -void Hw::Pml4_table::_invalidate_range(addr_t vo, size_t size) {} - namespace Bootstrap { struct Pic {}; using Cpu = Hw::X86_64_cpu; 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 d31e44083..0d1344832 100644 --- a/repos/base-hw/src/core/spec/x86_64/cpu.cc +++ b/repos/base-hw/src/core/spec/x86_64/cpu.cc @@ -76,10 +76,10 @@ void Genode::Cpu::mmu_fault(Context & regs, Kernel::Thread_fault & fault) }; 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; + if (!(err & ERR_P)) return Fault::PAGE_MISSING; + if (err & ERR_W) return Fault::WRITE; + if (err & ERR_I) return Fault::EXEC; + else return Fault::UNKNOWN; }; fault.addr = Genode::Cpu::Cr2::read(); 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 e7cff13d6..5f69a79eb 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,7 +25,9 @@ void Kernel::Thread::_call_update_data_region() { } void Kernel::Thread::_call_update_instr_region() { } -void Kernel::Thread::_call_update_pd() { } +void Kernel::Thread::_call_update_pd() { + Genode::Cpu::Cr3::write(Genode::Cpu::Cr3::read()); +} extern void * __tss_client_context_ptr; diff --git a/repos/base-hw/src/core/spec/x86_64/translation_table.h b/repos/base-hw/src/core/spec/x86_64/translation_table.h index d87744c66..dc7543cf8 100644 --- a/repos/base-hw/src/core/spec/x86_64/translation_table.h +++ b/repos/base-hw/src/core/spec/x86_64/translation_table.h @@ -15,13 +15,5 @@ #define _CORE__SPEC__X86_64__TRANSLATION_TABLE_H_ #include -#include - -void Hw::Pml4_table::_invalidate_range(addr_t vo, size_t size) -{ - /* FIXME: do not necessarily flush the whole TLB */ - Genode::Cpu::Cr3::write(Genode::Cpu::Cr3::read()); - -} #endif /* _CORE__SPEC__X86_64__TRANSLATION_TABLE_H_ */ diff --git a/repos/base-hw/src/lib/hw/spec/x86_64/page_table.h b/repos/base-hw/src/lib/hw/spec/x86_64/page_table.h index d879c14a6..37fe511af 100644 --- a/repos/base-hw/src/lib/hw/spec/x86_64/page_table.h +++ b/repos/base-hw/src/lib/hw/spec/x86_64/page_table.h @@ -100,17 +100,6 @@ namespace Hw D::clear(value); return value; } - - /** - * Merge access rights of descriptor with given flags. - */ - static void merge_access_rights(access_t &desc, - Page_flags const &flags) - { - Rw::set(desc, Rw::get(desc) | flags.writeable); - Us::set(desc, Us::get(desc) | !flags.privileged); - Xd::set(desc, Xd::get(desc) & !flags.executable); - } }; } @@ -341,10 +330,11 @@ class Hw::Page_directory struct Mt : Base::template Bitset_2 { }; - static typename Base::access_t create(Page_flags const &flags, - addr_t const pa) + static typename Base::access_t create(addr_t const pa) { /* XXX: Set memory type depending on active PAT */ + static Page_flags flags { RW, EXEC, USER, NO_GLOBAL, + RAM, Genode::CACHED }; return Base::create(flags) | Pa::masked(pa); } }; @@ -388,12 +378,10 @@ class Hw::Page_directory /* create and link next level table */ ENTRY & table = alloc.construct(); - desc = (access_t) Td::create(flags, alloc.phys_addr(table)); + desc = (access_t) Td::create(alloc.phys_addr(table)); } else if (Descriptor::maps_page(desc)) { throw Double_insertion(); - } else { - Descriptor::merge_access_rights(desc, flags); } /* insert translation */ @@ -528,9 +516,11 @@ class Hw::Pml4_table struct Pa : Bitfield<12, SIZE_LOG2> { }; /* physical address */ struct Mt : Genode::Bitset_2 { }; /* memory type */ - static access_t create(Page_flags const &flags, addr_t const pa) + static access_t create(addr_t const pa) { /* XXX: Set memory type depending on active PAT */ + static Page_flags flags { RW, EXEC, USER, NO_GLOBAL, + RAM, Genode::CACHED }; return Common_descriptor::create(flags) | Pa::masked(pa); } }; @@ -556,9 +546,7 @@ class Hw::Pml4_table if (!Descriptor::present(desc)) { /* create and link next level table */ ENTRY & table = alloc.construct(); - desc = Descriptor::create(flags, alloc.phys_addr(table)); - } else { - Descriptor::merge_access_rights(desc, flags); + desc = Descriptor::create(alloc.phys_addr(table)); } /* insert translation */ @@ -622,8 +610,6 @@ class Hw::Pml4_table / (1UL << alignment); } - inline void _invalidate_range(addr_t vo, size_t size); - public: static constexpr size_t MIN_PAGE_SIZE_LOG2 = SIZE_LOG2_4KB; @@ -680,7 +666,6 @@ class Hw::Pml4_table void remove_translation(addr_t vo, size_t size, Allocator & alloc) { _range_op(vo, 0, size, Remove_func(alloc)); - _invalidate_range(vo, size); } } __attribute__((aligned(1 << ALIGNM_LOG2)));