[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

@@ -17,67 +17,21 @@
 
 #include "kernel_args.h"
 
-/*
+namespace kernel {
-#define KERNEL_HEADER_MAGIC   0x600db007
+#include "kernel_memory.h"
-#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 boot {
 
 constexpr int max_modules = 10; // Max modules to allocate room for
 
-/*
+/// Change a pointer to point to the higher-half linear-offset version
-
+/// of the address it points to.
-EFI_STATUS
+template <typename T>
-detect_debug_mode(EFI_RUNTIME_SERVICES *run, kernel_args *header) {
+void change_pointer(T *&pointer)
-	CHAR16 var_name[] = L"debug";
+{
-
+	pointer = offset_ptr<T>(pointer, kernel::memory::page_offset);
-	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;
 }
-*/
 
 /// 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<void**>(&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);

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;

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; }

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) {

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, &copy);
 	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];
+		for (unsigned i = pml4e_kernel; i < table_entries; ++i)
-		to->entries[511] = m_kernel_pml4->entries[511];
+			to->entries[i] = m_kernel_pml4->entries[i];
 	}
 
 	const int max =
-		lvl == page_table::level::pml4 ?
+		lvl == page_table::level::pml4
-			510 :
+			? pml4e_kernel
-			512;
+			: 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 ?
+		lvl == page_table::level::pml4
-			510 :
+			? pml4e_kernel
-			512;
+			: 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; i<table_entries; ++i) new_table->entries[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;
+					phys_addr += frame_size * table_entries;
-					count -= 512;
+					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<table_entries; ++i) {
 		uint64_t ent = entries[i];
 
 		if ((ent & 0x1) == 0)
@@ -465,7 +464,7 @@ page_table::dump(page_table::level lvl, bool recurse)
 	}
 
 	if (lvl != level::pt && recurse) {
-		for (int i=0; i<=512; ++i) {
+		for (int i=0; i<=table_entries; ++i) {
 			if (is_large_page(lvl, i))
 				continue;
 

src/kernel/page_table.h

@@ -15,7 +15,7 @@ struct page_table
 		return static_cast<level>(static_cast<unsigned>(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];