From 2d44e8112b5a861bec52566add8c472ad8fedef0 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Sun, 27 Sep 2020 16:06:25 -0700 Subject: [PATCH] [kernel] Remove 'allowed' page table flag The allowed flag was janky and easy to get lost when doing page table manipulation. All allocation goes throug vm_area now, so 'allowed' can be dropped. --- src/kernel/objects/vm_area.cpp | 1 - src/kernel/page_table.cpp | 24 +---------------- src/kernel/page_table.h | 11 +------- src/kernel/vm_space.cpp | 48 +++++----------------------------- src/kernel/vm_space.h | 7 ----- 5 files changed, 8 insertions(+), 83 deletions(-) diff --git a/src/kernel/objects/vm_area.cpp b/src/kernel/objects/vm_area.cpp index d1a25f5..60d3f36 100644 --- a/src/kernel/objects/vm_area.cpp +++ b/src/kernel/objects/vm_area.cpp @@ -91,7 +91,6 @@ vm_area_shared::commit(uintptr_t phys, uintptr_t offset, size_t count) .phys = phys}); n = 1; - // Try to expand to abutting similar areas if (o > 0 && m_mappings[o-1].end() == offset && diff --git a/src/kernel/page_table.cpp b/src/kernel/page_table.cpp index 38ee0a6..5753e09 100644 --- a/src/kernel/page_table.cpp +++ b/src/kernel/page_table.cpp @@ -100,27 +100,6 @@ page_table::iterator::next(level l) } } -bool -page_table::iterator::allowed() const -{ - level d = depth(); - while (true) { - if (entry(d) & flag::allowed) return true; - else if (d == level::pml4) return false; - --d; - } -} - -void -page_table::iterator::allow(level at, bool allowed) -{ - for (level l = level::pdp; l <= at; ++l) - ensure_table(l); - - if (allowed) entry(at) |= flag::allowed; - else entry(at) &= ~flag::allowed; -} - bool page_table::iterator::operator!=(const iterator &o) const { @@ -159,7 +138,7 @@ page_table::iterator::ensure_table(level l) uintptr_t phys = reinterpret_cast(table) & ~page_offset; uint64_t &parent = entry(l - 1); - flag flags = table_flags | (parent & flag::allowed); + flag flags = table_flags; if (m_index[0] < memory::pml4e_kernel) flags |= flag::user; @@ -185,7 +164,6 @@ page_table::get(int i, uint16_t *flags) const void page_table::set(int i, page_table *p, uint16_t flags) { - if (entries[i] & flag::allowed) flags |= flag::allowed; entries[i] = (reinterpret_cast(p) - page_offset) | (flags & 0xfff); diff --git a/src/kernel/page_table.h b/src/kernel/page_table.h index 27e916f..5e3f010 100644 --- a/src/kernel/page_table.h +++ b/src/kernel/page_table.h @@ -28,10 +28,7 @@ struct page_table page = 0x0080, /// Entry is a large page pte_mtrr2 = 0x0080, /// MTRR selector bit 2 on PT entries global = 0x0100, /// Entry is not PCID-specific - mtrr2 = 0x1000, /// MTRR selector bit 2 on PD and PDP entries - - // jsix-defined - allowed = 0x0800 /// Allocation here is allowed + mtrr2 = 0x1000 /// MTRR selector bit 2 on PD and PDP entries }; /// Helper for getting the next level value @@ -103,12 +100,6 @@ struct page_table /// Increment iteration to the next entry aligned to the given level void next(level l); - /// Check if allocation is allowed at the current location - bool allowed() const; - - /// Mark allocation allowed at the given depth for the current location - void allow(level at, bool allowed); - /// Increment iteration to the next entry at the deepest level inline void increment() { next(level::page); } diff --git a/src/kernel/vm_space.cpp b/src/kernel/vm_space.cpp index 0e6dd65..a3969e0 100644 --- a/src/kernel/vm_space.cpp +++ b/src/kernel/vm_space.cpp @@ -197,7 +197,6 @@ vm_space::clear(const vm_area &vma, uintptr_t offset, size_t count, bool free) while (count--) { uint64_t &e = it.entry(page_table::level::pt); - bool allowed = (e & page_table::flag::allowed); uintptr_t phys = e & ~0xfffull; if (e & page_table::flag::present) { @@ -212,7 +211,7 @@ vm_space::clear(const vm_area &vma, uintptr_t offset, size_t count, bool free) fa.free(e & ~0xfffull, 1); } - e = 0 | (allowed ? page_table::flag::allowed : page_table::flag::none); + e = 0; ++it; } @@ -229,24 +228,6 @@ vm_space::lookup(const vm_area &vma, uintptr_t offset) return base + offset; } -void -vm_space::allow(uintptr_t start, size_t length, bool allow) -{ - using level = page_table::level; - kassert((start & 0xfff) == 0, "non-page-aligned address"); - kassert((length & 0xfff) == 0, "non-page-aligned length"); - - const uintptr_t end = start + length; - page_table::iterator it {start, m_pml4}; - - while (it.vaddress() < end) { - level d = it.align(); - while (it.end(d) > end) ++d; - it.allow(d-1, allow); - it.next(d); - } -} - bool vm_space::active() const { @@ -276,8 +257,6 @@ vm_space::handle_fault(uintptr_t addr, fault_type fault) { uintptr_t page = addr & ~0xfffull; - page_table::iterator it {addr, m_pml4}; - // TODO: Handle more fult types if (fault && fault_type::present) return false; @@ -285,33 +264,18 @@ vm_space::handle_fault(uintptr_t addr, fault_type fault) uintptr_t base = 0; vm_area *area = get(addr, &base); - if ((!area || !area->allowed(page-base)) && !it.allowed()) + if (!area || !area->allowed(page-base)) return false; uintptr_t phys = 0; size_t n = frame_allocator::get().allocate(1, &phys); kassert(n, "Failed to allocate a new page during page fault"); - page_table::flag flags = - page_table::flag::present | - page_table::flag::write | - (area - ? page_table::flag::none - : page_table::flag::allowed) | - (is_kernel() - ? page_table::flag::global - : page_table::flag::user); + if (area->flags() && vm_flags::zero) + kutil::memset(memory::to_virtual(phys), 0, memory::frame_size); - if (area) { - if (area->flags() && vm_flags::zero) - kutil::memset(memory::to_virtual(phys), 0, memory::frame_size); - - uintptr_t offset = page - base; - area->commit(phys, offset, 1); - return true; - } - - it.entry(page_table::level::pt) = phys | flags; + uintptr_t offset = page - base; + area->commit(phys, offset, 1); return true; } diff --git a/src/kernel/vm_space.h b/src/kernel/vm_space.h index 7124453..0d229a0 100644 --- a/src/kernel/vm_space.h +++ b/src/kernel/vm_space.h @@ -64,13 +64,6 @@ public: /// Look up the address of a given VMA's offset uintptr_t lookup(const vm_area &vma, uintptr_t offset); - /// Mark whether allocation is allowed or not in a range of - /// virtual memory. - /// \arg start The starting virtual address of the area - /// \arg length The length in bytes of the area - /// \arg allow True if allocation should be allowed - void allow(uintptr_t start, size_t length, bool allow); - /// Check if this space is the current active space bool active() const;