From 83b330bf2baa1371973b51f75a9cdf46349bf440 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Sun, 24 May 2020 22:06:24 -0700 Subject: [PATCH] [kernel] Use constants for known pml4e indices There were a few lingering bugs due to places where 510/511 were hard-coded as the kernel-space PML4 entries. These are now constants defined in kernel_memory.h instead. Tags: boot memory paging --- src/boot/main.cpp | 73 ++++++++------------------------- src/boot/paging.cpp | 2 +- src/include/kernel_memory.h | 9 ++++ src/kernel/memory_bootstrap.cpp | 7 +++- src/kernel/page_manager.cpp | 51 +++++++++++------------ src/kernel/page_table.h | 2 +- 6 files changed, 57 insertions(+), 87 deletions(-) diff --git a/src/boot/main.cpp b/src/boot/main.cpp index 7a96fda..7a923c2 100644 --- a/src/boot/main.cpp +++ b/src/boot/main.cpp @@ -17,67 +17,21 @@ #include "kernel_args.h" -/* -#define KERNEL_HEADER_MAGIC 0x600db007 -#define KERNEL_HEADER_VERSION 1 - -#pragma pack(push, 1) -struct kernel_header { - uint32_t magic; - uint16_t version; - uint16_t length; - - uint8_t major; - uint8_t minor; - uint16_t patch; - uint32_t gitsha; -}; -#pragma pack(pop) -*/ +namespace kernel { +#include "kernel_memory.h" +} namespace boot { constexpr int max_modules = 10; // Max modules to allocate room for -/* - -EFI_STATUS -detect_debug_mode(EFI_RUNTIME_SERVICES *run, kernel_args *header) { - CHAR16 var_name[] = L"debug"; - - EFI_STATUS status; - uint8_t debug = 0; - UINTN var_size = sizeof(debug); - -#ifdef __JSIX_SET_DEBUG_UEFI_VAR__ - debug = __JSIX_SET_DEBUG_UEFI_VAR__; - uint32_t attrs = - EFI_VARIABLE_NON_VOLATILE | - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS; - status = run->SetVariable( - var_name, - &guid_jsix_vendor, - attrs, - var_size, - &debug); - CHECK_EFI_STATUS_OR_RETURN(status, "detect_debug_mode::SetVariable"); -#endif - - status = run->GetVariable( - var_name, - &guid_jsix_vendor, - nullptr, - &var_size, - &debug); - CHECK_EFI_STATUS_OR_RETURN(status, "detect_debug_mode::GetVariable"); - - if (debug) - header->flags |= JSIX_FLAG_DEBUG; - - return EFI_SUCCESS; +/// Change a pointer to point to the higher-half linear-offset version +/// of the address it points to. +template +void change_pointer(T *&pointer) +{ + pointer = offset_ptr(pointer, kernel::memory::page_offset); } -*/ /// Allocate space for kernel args. Allocates enough space from pool /// memory for the args header and `max_modules` module headers. @@ -160,8 +114,6 @@ bootloader_main_uefi( args->acpi_table = hw::find_acpi_table(st); memory::mark_pointer_fixup(&args->runtime_services); - for (unsigned i = 0; i < args->num_modules; ++i) - memory::mark_pointer_fixup(reinterpret_cast(&args->modules[i])); fs::file disk = fs::get_boot_volume(image, bs); load_module(disk, args, L"initrd", L"initrd.img", kernel::args::mod_type::initrd); @@ -171,6 +123,12 @@ bootloader_main_uefi( paging::allocate_tables(args, bs); *kentry = loader::load(kernel->location, kernel->size, args, bs); + + for (unsigned i = 0; i < args->num_modules; ++i) { + kernel::args::module &mod = args->modules[i]; + change_pointer(mod.location); + } + return args; } @@ -197,6 +155,7 @@ efi_main(uefi::handle image_handle, uefi::system_table *st) L"Failed to exit boot services"); memory::virtualize(args->pml4, map, st->runtime_services); + change_pointer(args->pml4); hw::setup_cr4(); kentry(args); diff --git a/src/boot/paging.cpp b/src/boot/paging.cpp index 056b720..7c0e07b 100644 --- a/src/boot/paging.cpp +++ b/src/boot/paging.cpp @@ -165,7 +165,7 @@ add_current_mappings(page_table *new_pml4) asm volatile ( "mov %%cr3, %0" : "=r" (old_pml4) ); // Only copy mappings in the lower half - for (int i = 0; i < 256; ++i) { + for (int i = 0; i < ::memory::pml4e_kernel; ++i) { uint64_t entry = old_pml4->entries[i]; if (entry & 1) new_pml4->entries[i] = entry; diff --git a/src/include/kernel_memory.h b/src/include/kernel_memory.h index d449ce6..1edcf1e 100644 --- a/src/include/kernel_memory.h +++ b/src/include/kernel_memory.h @@ -28,6 +28,15 @@ namespace memory { /// Start of the kernel heap constexpr uintptr_t heap_start = page_offset - kernel_max_heap; + /// First kernel space PML4 entry + constexpr unsigned pml4e_kernel = 256; + + /// First offset-mapped space PML4 entry + constexpr unsigned pml4e_offset = 384; + + /// Number of page_table entries + constexpr unsigned table_entries = 512; + /// Helper to determine if a physical address can be accessed /// through the page_offset area. inline bool page_mappable(uintptr_t a) { return (a & page_offset) == 0; } diff --git a/src/kernel/memory_bootstrap.cpp b/src/kernel/memory_bootstrap.cpp index 43d1618..4e413f9 100644 --- a/src/kernel/memory_bootstrap.cpp +++ b/src/kernel/memory_bootstrap.cpp @@ -17,6 +17,9 @@ using memory::heap_start; using memory::kernel_max_heap; using memory::kernel_offset; using memory::page_offset; +using memory::pml4e_kernel; +using memory::pml4e_offset; +using memory::table_entries; using namespace kernel; @@ -38,7 +41,7 @@ void walk_page_table( constexpr size_t huge_page_size = (1ull<<30); constexpr size_t large_page_size = (1ull<<21); - for (unsigned i = 0; i < 512; ++i) { + for (unsigned i = 0; i < table_entries; ++i) { page_table *next = table->get(i); if (!next) { kspace.commit(current_start, current_bytes); @@ -95,7 +98,7 @@ memory_initialize(args::header *kargs) size_t current_bytes = 0; // TODO: Should we exclude the top of this area? (eg, buffers, stacks, etc) - for (unsigned i = 256; i < 384; ++i) { + for (unsigned i = pml4e_kernel; i < pml4e_offset; ++i) { page_table *pdp = kpml4->get(i); if (!pdp) { diff --git a/src/kernel/page_manager.cpp b/src/kernel/page_manager.cpp index da5d0d3..082693b 100644 --- a/src/kernel/page_manager.cpp +++ b/src/kernel/page_manager.cpp @@ -13,6 +13,8 @@ using memory::kernel_max_heap; using memory::kernel_offset; using memory::page_offset; using memory::page_mappable; +using memory::pml4e_kernel; +using memory::table_entries; page_manager g_page_manager(g_frame_allocator, 0); extern kutil::vm_space g_kernel_space; @@ -58,7 +60,7 @@ page_manager::create_process_map() page_table *table = get_table_page(); kutil::memset(table, 0, frame_size/2); - for (unsigned i = 256; i < 512; ++i) + for (unsigned i = pml4e_kernel; i < table_entries; ++i) table->entries[i] = m_kernel_pml4->entries[i]; // Create the initial user stack @@ -74,9 +76,6 @@ page_manager::create_process_map() uintptr_t page_manager::copy_page(uintptr_t orig) { - bool paged_orig = false; - bool paged_copy = false; - uintptr_t copy = 0; size_t n = m_frames.allocate(1, ©); kassert(n, "copy_page could not allocate page"); @@ -99,14 +98,14 @@ page_manager::copy_table(page_table *from, page_table::level lvl, page_table_ind log::debug(logs::paging, "Page manager copying level %d table at %016lx to %016lx.", lvl, from, to); if (lvl == page_table::level::pml4) { - to->entries[510] = m_kernel_pml4->entries[510]; - to->entries[511] = m_kernel_pml4->entries[511]; + for (unsigned i = pml4e_kernel; i < table_entries; ++i) + to->entries[i] = m_kernel_pml4->entries[i]; } const int max = - lvl == page_table::level::pml4 ? - 510 : - 512; + lvl == page_table::level::pml4 + ? pml4e_kernel + : table_entries; unsigned pages_copied = 0; uintptr_t from_addr = 0; @@ -242,9 +241,9 @@ void page_manager::unmap_table(page_table *table, page_table::level lvl, bool free, page_table_indices index) { const int max = - lvl == page_table::level::pml4 ? - 510 : - 512; + lvl == page_table::level::pml4 + ? pml4e_kernel + : table_entries; uintptr_t free_start = 0; uintptr_t free_start_virt = 0; @@ -344,7 +343,7 @@ page_manager::check_needs_page(page_table *table, unsigned index, bool user) if ((table->entries[index] & 0x1) == 1) return; page_table *new_table = get_table_page(); - for (int i=0; i<512; ++i) new_table->entries[i] = 0; + for (int i=0; ientries[i] = 0; table->entries[index] = pt_to_phys(new_table) | (user ? user_table_flags : sys_table_flags); } @@ -361,23 +360,23 @@ page_manager::page_in(page_table *pml4, uintptr_t phys_addr, uintptr_t virt_addr uint64_t flags = user ? user_table_flags : sys_table_flags; - for (; idx[0] < 512; idx[0] += 1) { + for (; idx[0] < table_entries; idx[0] += 1) { check_needs_page(tables[0], idx[0], user); tables[1] = tables[0]->get(idx[0]); - for (; idx[1] < 512; idx[1] += 1, idx[2] = 0, idx[3] = 0) { + for (; idx[1] < table_entries; idx[1] += 1, idx[2] = 0, idx[3] = 0) { check_needs_page(tables[1], idx[1], user); tables[2] = tables[1]->get(idx[1]); - for (; idx[2] < 512; idx[2] += 1, idx[3] = 0) { + for (; idx[2] < table_entries; idx[2] += 1, idx[3] = 0) { if (large && idx[3] == 0 && - count >= 512 && + count >= table_entries && tables[2]->get(idx[2]) == nullptr) { // Do a 2MiB page instead tables[2]->entries[idx[2]] = phys_addr | flags | 0x80; - phys_addr += frame_size * 512; - count -= 512; + phys_addr += frame_size * table_entries; + count -= table_entries; if (count == 0) return; continue; } @@ -385,7 +384,7 @@ page_manager::page_in(page_table *pml4, uintptr_t phys_addr, uintptr_t virt_addr check_needs_page(tables[2], idx[2], user); tables[3] = tables[2]->get(idx[2]); - for (; idx[3] < 512; idx[3] += 1) { + for (; idx[3] < table_entries; idx[3] += 1) { tables[3]->entries[idx[3]] = phys_addr | flags; phys_addr += frame_size; if (--count == 0) return; @@ -407,16 +406,16 @@ page_manager::page_out(page_table *pml4, uintptr_t virt_addr, size_t count, bool uintptr_t free_start = 0; unsigned free_count = 0; - for (; idx[0] < 512; idx[0] += 1) { + for (; idx[0] < table_entries; idx[0] += 1) { tables[1] = tables[0]->get(idx[0]); - for (; idx[1] < 512; idx[1] += 1) { + for (; idx[1] < table_entries; idx[1] += 1) { tables[2] = tables[1]->get(idx[1]); - for (; idx[2] < 512; idx[2] += 1) { + for (; idx[2] < table_entries; idx[2] += 1) { tables[3] = tables[2]->get(idx[2]); - for (; idx[3] < 512; idx[3] += 1) { + for (; idx[3] < table_entries; idx[3] += 1) { uintptr_t entry = tables[3]->entries[idx[3]] & ~0xfffull; if (!found || entry != free_start + free_count * frame_size) { if (found && free) m_frames.free(free_start, free_count); @@ -447,7 +446,7 @@ page_table::dump(page_table::level lvl, bool recurse) console *cons = console::get(); cons->printf("\nLevel %d page table @ %lx:\n", lvl, this); - for (int i=0; i<512; ++i) { + for (int i=0; i(static_cast(l) + 1); } - uint64_t entries[512]; + uint64_t entries[memory::table_entries]; inline page_table * get(int i, uint16_t *flags = nullptr) const { uint64_t entry = entries[i];