From d75b9c22d444a1e81fd2838542bf43866824e408 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Fri, 8 Jan 2021 22:40:30 -0800 Subject: [PATCH] [boot] Don't use custom UEFI memory types The UEFI spec specifically calls out memory types with the high bit set as being available for OS loaders' custom use. However, it seems many UEFI firmware implementations don't handle this well. (Virtualbox, and the firmware on my Intel NUC and Dell XPS laptop to name a few.) So sadly since we can't rely on this feature of UEFI in all cases, we can't use it at all. Instead, treat _all_ memory tagged as EfiLoaderData as possibly containing data that's been passed to the OS by the bootloader and don't free it yet. This will need to be followed up with a change that copies anything we need to save and frees this memory. See: https://github.com/kiznit/rainbow-os/blob/master/boot/machine/efi/README.md --- src/boot/loader.cpp | 2 +- src/boot/main.cpp | 5 ++--- src/boot/memory.cpp | 37 ++++++--------------------------- src/boot/memory.h | 22 -------------------- src/boot/paging.cpp | 2 +- src/include/kernel_args.h | 5 +---- src/kernel/memory_bootstrap.cpp | 1 + 7 files changed, 12 insertions(+), 62 deletions(-) diff --git a/src/boot/loader.cpp b/src/boot/loader.cpp index 1418d0c..3b5d748 100644 --- a/src/boot/loader.cpp +++ b/src/boot/loader.cpp @@ -82,7 +82,7 @@ load_program( try_or_raise( bs->allocate_pages(uefi::allocate_type::any_pages, - memory::program_type, num_pages, &pages), + uefi::memory_type::loader_data, num_pages, &pages), L"Failed allocating space for program"); bs->set_mem(pages, total_size, 0); diff --git a/src/boot/main.cpp b/src/boot/main.cpp index 004087a..3f74ee4 100644 --- a/src/boot/main.cpp +++ b/src/boot/main.cpp @@ -67,7 +67,7 @@ allocate_args_structure( max_programs * sizeof(args::program); // The program structures try_or_raise( - bs->allocate_pool(memory::args_type, args_size, + bs->allocate_pool(uefi::memory_type::loader_data, args_size, reinterpret_cast(&args)), L"Could not allocate argument memory"); @@ -119,9 +119,8 @@ uefi_preboot(uefi::handle image, uefi::system_table *st) fs::file disk = fs::get_boot_volume(image, bs); - const uefi::memory_type mod_type = memory::module_type; buffer symbols = loader::load_file(disk, L"symbol table", L"symbol_table.dat", - memory::module_type); + uefi::memory_type::loader_data); add_module(args, args::mod_type::symbol_table, symbols); for (auto &desc : program_list) { diff --git a/src/boot/memory.cpp b/src/boot/memory.cpp index 4e3c3e3..e283ed9 100644 --- a/src/boot/memory.cpp +++ b/src/boot/memory.cpp @@ -39,19 +39,10 @@ static const wchar_t *memory_type_names[] = { static const wchar_t * memory_type_name(uefi::memory_type t) { - if (t < uefi::memory_type::max_memory_type) { + if (t < uefi::memory_type::max_memory_type) return memory_type_names[static_cast(t)]; - } - switch(t) { - /* - case args_type: return L"jsix kernel args"; - case module_type: return L"jsix bootloader module"; - case program_type: return L"jsix kernel or program code"; - case table_type: return L"jsix page tables"; - */ - default: return L"Bad Type Value"; - } + return L"Bad Type Value"; } void @@ -141,7 +132,7 @@ build_kernel_mem_map(kernel::args::header *args, uefi::boot_services *bs) try_or_raise( bs->allocate_pages( uefi::allocate_type::any_pages, - module_type, + uefi::memory_type::loader_data, bytes_to_pages(map_size), reinterpret_cast(&kernel_map)), L"Error allocating kernel memory map module space"); @@ -166,13 +157,15 @@ build_kernel_mem_map(kernel::args::header *args, uefi::boot_services *bs) continue; case uefi::memory_type::loader_code: - case uefi::memory_type::loader_data: case uefi::memory_type::boot_services_code: case uefi::memory_type::boot_services_data: case uefi::memory_type::conventional_memory: type = mem_type::free; break; + case uefi::memory_type::loader_data: + type = mem_type::pending; + case uefi::memory_type::runtime_services_code: case uefi::memory_type::runtime_services_data: type = mem_type::uefi_runtime; @@ -191,24 +184,6 @@ build_kernel_mem_map(kernel::args::header *args, uefi::boot_services *bs) type = mem_type::persistent; break; - /* - case args_type: - type = mem_type::args; - break; - - case module_type: - type = mem_type::module; - break; - - case program_type: - type = mem_type::program; - break; - - case table_type: - type = mem_type::table; - break; - */ - default: error::raise( uefi::status::invalid_parameter, diff --git a/src/boot/memory.h b/src/boot/memory.h index e1c5715..d8b8ed0 100644 --- a/src/boot/memory.h +++ b/src/boot/memory.h @@ -18,28 +18,6 @@ inline constexpr size_t bytes_to_pages(size_t bytes) { return ((bytes - 1) / page_size) + 1; } -/// \defgroup memory_types -/// Custom UEFI memory type values used for data being passed to the kernel -/// @{ - -/// Memory containing the kernel args structure -constexpr uefi::memory_type args_type = - static_cast(0x80000000); - -/// Memory containing any loaded modules to be passed to the kernel -constexpr uefi::memory_type module_type = - static_cast(0x80000001); - -/// Memory containing loaded kernel or program code and data sections -constexpr uefi::memory_type program_type = - static_cast(0x80000002); - -/// Memory containing page tables set up by the loader -constexpr uefi::memory_type table_type = - static_cast(0x80000003); - -/// @} - /// \defgroup pointer_fixup /// Memory virtualization pointer fixup functions. Handles changing affected pointers /// when calling UEFI's `set_virtual_address_map` function to change the location of diff --git a/src/boot/paging.cpp b/src/boot/paging.cpp index 475b88a..21f693d 100644 --- a/src/boot/paging.cpp +++ b/src/boot/paging.cpp @@ -202,7 +202,7 @@ allocate_tables(kernel::args::header *args, uefi::boot_services *bs) try_or_raise( bs->allocate_pages( uefi::allocate_type::any_pages, - memory::table_type, + uefi::memory_type::loader_data, tables_needed, &addr), L"Error allocating page table pages."); diff --git a/src/include/kernel_args.h b/src/include/kernel_args.h index b793edd..36c81c6 100644 --- a/src/include/kernel_args.h +++ b/src/include/kernel_args.h @@ -44,10 +44,7 @@ struct program { enum class mem_type : uint32_t { free, - args, - program, - module, - table, + pending, acpi, uefi_runtime, mmio, diff --git a/src/kernel/memory_bootstrap.cpp b/src/kernel/memory_bootstrap.cpp index 4d621aa..114a5ce 100644 --- a/src/kernel/memory_bootstrap.cpp +++ b/src/kernel/memory_bootstrap.cpp @@ -110,6 +110,7 @@ memory_initialize_pre_ctors(args::header *kargs) const size_t count = kargs->map_count; for (unsigned i = 0; i < count; ++i) { // TODO: use entry attributes + // TODO: copy anything we need from "pending" memory and free it args::mem_entry &e = entries[i]; if (e.type == args::mem_type::free) g_frame_allocator.free(e.start, e.pages);