From 0e1fd09a48d7c10e4337f80cc2f628c26d48224f Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 27 Jan 2023 13:17:30 +0100 Subject: [PATCH 1/6] Test booting with a minimal kernel stack size --- Cargo.lock | 10 +++++++ Cargo.toml | 5 ++++ tests/min_stack.rs | 6 +++++ tests/test_kernels/min_stack/Cargo.toml | 13 +++++++++ .../min_stack/src/bin/basic_boot.rs | 26 ++++++++++++++++++ tests/test_kernels/min_stack/src/lib.rs | 27 +++++++++++++++++++ 6 files changed, 87 insertions(+) create mode 100644 tests/min_stack.rs create mode 100644 tests/test_kernels/min_stack/Cargo.toml create mode 100644 tests/test_kernels/min_stack/src/bin/basic_boot.rs create mode 100644 tests/test_kernels/min_stack/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 24e41cae..1cfa03a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -155,6 +155,7 @@ dependencies = [ "test_kernel_default_settings", "test_kernel_higher_half", "test_kernel_map_phys_mem", + "test_kernel_min_stack", "test_kernel_pie", "test_kernel_ramdisk", ] @@ -885,6 +886,15 @@ dependencies = [ "x86_64", ] +[[package]] +name = "test_kernel_min_stack" +version = "0.1.0" +dependencies = [ + "bootloader_api", + "uart_16550", + "x86_64", +] + [[package]] name = "test_kernel_pie" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index c9bf81c6..a54c1a44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ members = [ "tests/test_kernels/pie", "tests/test_kernels/lto", "tests/test_kernels/ramdisk", + "tests/test_kernels/min_stack", ] exclude = ["examples/basic", "examples/test_framework"] @@ -60,6 +61,7 @@ test_kernel_higher_half = { path = "tests/test_kernels/higher_half", artifact = test_kernel_map_phys_mem = { path = "tests/test_kernels/map_phys_mem", artifact = "bin", target = "x86_64-unknown-none" } test_kernel_pie = { path = "tests/test_kernels/pie", artifact = "bin", target = "x86_64-unknown-none" } test_kernel_ramdisk = { path = "tests/test_kernels/ramdisk", artifact = "bin", target = "x86_64-unknown-none" } +test_kernel_min_stack = { path = "tests/test_kernels/min_stack", artifact = "bin", target = "x86_64-unknown-none" } [profile.dev] panic = "abort" @@ -113,6 +115,9 @@ rustflags = [ "code-model=large", ] +[profile.test.package.test_kernel_min_stack] +opt-level = 2 + [build-dependencies] llvm-tools = "0.1.1" async-process = "1.6.0" diff --git a/tests/min_stack.rs b/tests/min_stack.rs new file mode 100644 index 00000000..d320a6cb --- /dev/null +++ b/tests/min_stack.rs @@ -0,0 +1,6 @@ +use bootloader_test_runner::run_test_kernel; + +#[test] +fn basic_boot() { + run_test_kernel(env!("CARGO_BIN_FILE_TEST_KERNEL_MIN_STACK_basic_boot")); +} diff --git a/tests/test_kernels/min_stack/Cargo.toml b/tests/test_kernels/min_stack/Cargo.toml new file mode 100644 index 00000000..afc7c2d6 --- /dev/null +++ b/tests/test_kernels/min_stack/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "test_kernel_min_stack" +version = "0.1.0" +authors = ["Philipp Oppermann "] +edition = "2021" + +[dependencies] +bootloader_api = { path = "../../../api" } +x86_64 = { version = "0.14.7", default-features = false, features = [ + "instructions", + "inline_asm", +] } +uart_16550 = "0.2.10" diff --git a/tests/test_kernels/min_stack/src/bin/basic_boot.rs b/tests/test_kernels/min_stack/src/bin/basic_boot.rs new file mode 100644 index 00000000..29a6602d --- /dev/null +++ b/tests/test_kernels/min_stack/src/bin/basic_boot.rs @@ -0,0 +1,26 @@ +#![no_std] // don't link the Rust standard library +#![no_main] // disable all Rust-level entry points + +use bootloader_api::{entry_point, BootInfo, BootloaderConfig}; +use core::fmt::Write; +use test_kernel_min_stack::{exit_qemu, serial, QemuExitCode}; + +const BOOTLOADER_CONFIG: BootloaderConfig = { + let mut config = BootloaderConfig::new_default(); + config.kernel_stack_size = 3000; + config +}; +entry_point!(kernel_main, config = &BOOTLOADER_CONFIG); + +fn kernel_main(boot_info: &'static mut BootInfo) -> ! { + writeln!(serial(), "Entered kernel with boot info: {boot_info:?}").unwrap(); + exit_qemu(QemuExitCode::Success); +} + +/// This function is called on panic. +#[panic_handler] +#[cfg(not(test))] +fn panic(info: &core::panic::PanicInfo) -> ! { + let _ = writeln!(serial(), "PANIC: {info}"); + exit_qemu(QemuExitCode::Failed); +} diff --git a/tests/test_kernels/min_stack/src/lib.rs b/tests/test_kernels/min_stack/src/lib.rs new file mode 100644 index 00000000..4e46fdb6 --- /dev/null +++ b/tests/test_kernels/min_stack/src/lib.rs @@ -0,0 +1,27 @@ +#![no_std] + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(u32)] +pub enum QemuExitCode { + Success = 0x10, + Failed = 0x11, +} + +pub fn exit_qemu(exit_code: QemuExitCode) -> ! { + use x86_64::instructions::{nop, port::Port}; + + unsafe { + let mut port = Port::new(0xf4); + port.write(exit_code as u32); + } + + loop { + nop(); + } +} + +pub fn serial() -> uart_16550::SerialPort { + let mut port = unsafe { uart_16550::SerialPort::new(0x3F8) }; + port.init(); + port +} From 9ac6744b5a8007f7d3076232047356591c84e407 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 27 Jan 2023 13:20:20 +0100 Subject: [PATCH 2/6] Fix stack pointer initialization The last page of stack memory was not used before because we initialized the stack pointer with the start address of the inclusive end page of the stack. This commit fixes this by initializing the stack pointer with the exact configured address, aligned to a 16-byte boundary. --- common/src/lib.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/common/src/lib.rs b/common/src/lib.rs index ad92556f..cf3e6c4d 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -210,11 +210,10 @@ where 16, &mut used_entries, ); + let stack_end_addr = stack_start_addr + config.kernel_stack_size; + let stack_start: Page = Page::containing_address(stack_start_addr); - let stack_end = { - let end_addr = stack_start_addr + config.kernel_stack_size; - Page::containing_address(end_addr - 1u64) - }; + let stack_end = Page::containing_address(stack_end_addr - 1u64); for page in Page::range_inclusive(stack_start, stack_end) { let frame = frame_allocator .allocate_frame() @@ -387,7 +386,10 @@ where Mappings { framebuffer: framebuffer_virt_addr, entry_point, - stack_end, + // Use the configured stack size, even if it's not page aligned. However, we + // need to align it down to the next 16-byte boundary because the System V + // ABI requires a 16-byte stack alignment. + stack_top: stack_end_addr.align_down(16u8), used_entries, physical_memory_offset, recursive_index, @@ -404,8 +406,8 @@ where pub struct Mappings { /// The entry point address of the kernel. pub entry_point: VirtAddr, - /// The stack end page of the kernel. - pub stack_end: Page, + /// The (exclusive) end address of the kernel stack. + pub stack_top: VirtAddr, /// Keeps track of used entries in the level 4 page table, useful for finding a free /// virtual memory when needed. pub used_entries: UsedLevel4Entries, @@ -556,7 +558,7 @@ pub fn switch_to_kernel( } = page_tables; let addresses = Addresses { page_table: kernel_level_4_frame, - stack_top: mappings.stack_end.start_address(), + stack_top: mappings.stack_top, entry_point: mappings.entry_point, boot_info, }; From 41c5f0a077cf6879d9bb654fafdeb71f0583628e Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 27 Jan 2023 13:57:28 +0100 Subject: [PATCH 3/6] Create a guard page for the kernel stack --- common/src/lib.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/common/src/lib.rs b/common/src/lib.rs index cf3e6c4d..41e377ef 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -204,12 +204,17 @@ where .expect("no entry point"); log::info!("Entry point at: {:#x}", entry_point.as_u64()); // create a stack - let stack_start_addr = mapping_addr( - config.mappings.kernel_stack, - config.kernel_stack_size, - 16, - &mut used_entries, - ); + let stack_start_addr = { + let guard_page_start = mapping_addr( + config.mappings.kernel_stack, + // allocate an additional page as a guard page + Size4KiB::SIZE + config.kernel_stack_size, + // we need page-alignment because we want a guard page directly below the stack + Size4KiB::SIZE, + &mut used_entries, + ); + guard_page_start + Size4KiB::SIZE + }; let stack_end_addr = stack_start_addr + config.kernel_stack_size; let stack_start: Page = Page::containing_address(stack_start_addr); From e204a565834d536646329e008dc3f508dd75dac7 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 27 Jan 2023 13:59:12 +0100 Subject: [PATCH 4/6] Verify that custom mappings have the required alignment Also: Add a convenience function for allocating page-aligned mappings. --- common/src/lib.rs | 59 ++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/common/src/lib.rs b/common/src/lib.rs index 41e377ef..9c9a4b2a 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -204,20 +204,19 @@ where .expect("no entry point"); log::info!("Entry point at: {:#x}", entry_point.as_u64()); // create a stack - let stack_start_addr = { - let guard_page_start = mapping_addr( + let stack_start = { + // we need page-alignment because we want a guard page directly below the stack + let guard_page = mapping_addr_page_aligned( config.mappings.kernel_stack, // allocate an additional page as a guard page Size4KiB::SIZE + config.kernel_stack_size, - // we need page-alignment because we want a guard page directly below the stack - Size4KiB::SIZE, &mut used_entries, + "kernel stack start", ); - guard_page_start + Size4KiB::SIZE + guard_page + 1 }; - let stack_end_addr = stack_start_addr + config.kernel_stack_size; + let stack_end_addr = stack_start.start_address() + config.kernel_stack_size; - let stack_start: Page = Page::containing_address(stack_start_addr); let stack_end = Page::containing_address(stack_end_addr - 1u64); for page in Page::range_inclusive(stack_start, stack_end) { let frame = frame_allocator @@ -266,13 +265,12 @@ where let framebuffer_start_frame: PhysFrame = PhysFrame::containing_address(framebuffer.addr); let framebuffer_end_frame = PhysFrame::containing_address(framebuffer.addr + framebuffer.info.byte_len - 1u64); - let start_page = Page::from_start_address(mapping_addr( + let start_page = mapping_addr_page_aligned( config.mappings.framebuffer, u64::from_usize(framebuffer.info.byte_len), - Size4KiB::SIZE, &mut used_entries, - )) - .expect("the framebuffer address must be page aligned"); + "framebuffer", + ); for (i, frame) in PhysFrame::range_inclusive(framebuffer_start_frame, framebuffer_end_frame).enumerate() { @@ -293,19 +291,17 @@ where }; let ramdisk_slice_len = system_info.ramdisk_len; let ramdisk_slice_start = if let Some(ramdisk_address) = system_info.ramdisk_addr { - let ramdisk_address_start = mapping_addr( + let start_page = mapping_addr_page_aligned( config.mappings.ramdisk_memory, system_info.ramdisk_len, - Size4KiB::SIZE, &mut used_entries, + "ramdisk start", ); let physical_address = PhysAddr::new(ramdisk_address); let ramdisk_physical_start_page: PhysFrame = PhysFrame::containing_address(physical_address); let ramdisk_page_count = (system_info.ramdisk_len - 1) / Size4KiB::SIZE; let ramdisk_physical_end_page = ramdisk_physical_start_page + ramdisk_page_count; - let start_page = Page::from_start_address(ramdisk_address_start) - .expect("the ramdisk start address must be page aligned"); let flags = PageTableFlags::PRESENT | PageTableFlags::WRITABLE; for (i, frame) in @@ -321,7 +317,7 @@ where ), }; } - Some(ramdisk_address_start) + Some(start_page.start_address()) } else { None }; @@ -335,7 +331,8 @@ where let size = max_phys.as_u64(); let alignment = Size2MiB::SIZE; - let offset = mapping_addr(mapping, size, alignment, &mut used_entries); + let offset = mapping_addr(mapping, size, alignment, &mut used_entries) + .expect("start addraess for physical memory mapping must be 2MiB-page-aligned"); for frame in PhysFrame::range_inclusive(start_frame, end_frame) { let page = Page::containing_address(offset + frame.start_address().as_u64()); @@ -465,11 +462,8 @@ where u64::from_usize(combined.size()), u64::from_usize(combined.align()), &mut mappings.used_entries, - ); - assert!( - boot_info_addr.is_aligned(u64::from_usize(combined.align())), - "boot info addr is not properly aligned" - ); + ) + .expect("boot info addr is not properly aligned"); let memory_map_regions_addr = boot_info_addr + memory_regions_offset; let memory_map_regions_end = boot_info_addr + combined.size(); @@ -614,15 +608,32 @@ struct Addresses { boot_info: &'static mut BootInfo, } +fn mapping_addr_page_aligned( + mapping: Mapping, + size: u64, + used_entries: &mut UsedLevel4Entries, + kind: &str, +) -> Page { + match mapping_addr(mapping, size, Size4KiB::SIZE, used_entries) { + Ok(addr) => Page::from_start_address(addr).unwrap(), + Err(addr) => panic!("{kind} address must be page-aligned (is `{addr:?})`"), + } +} + fn mapping_addr( mapping: Mapping, size: u64, alignment: u64, used_entries: &mut UsedLevel4Entries, -) -> VirtAddr { - match mapping { +) -> Result { + let addr = match mapping { Mapping::FixedAddress(addr) => VirtAddr::new(addr), Mapping::Dynamic => used_entries.get_free_address(size, alignment), + }; + if addr.is_aligned(alignment) { + Ok(addr) + } else { + Err(addr) } } From 4c7d8bf2d9c4905d515a47651ca44b1181b83aef Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 27 Jan 2023 13:59:38 +0100 Subject: [PATCH 5/6] Improve documentation for kernel stack configuration options --- api/src/config.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/api/src/config.rs b/api/src/config.rs index 7b91a4bd..19dd318e 100644 --- a/api/src/config.rs +++ b/api/src/config.rs @@ -20,8 +20,10 @@ pub struct BootloaderConfig { /// The size of the stack that the bootloader should allocate for the kernel (in bytes). /// /// The bootloader starts the kernel with a valid stack pointer. This setting defines - /// the stack size that the bootloader should allocate and map. The stack is created - /// with a guard page, so a stack overflow will lead to a page fault. + /// the stack size that the bootloader should allocate and map. + /// + /// The stack is created with a additional guard page, so a stack overflow will lead to + /// a page fault. pub kernel_stack_size: u64, /// Configuration for the frame buffer that can be used by the kernel to display pixels @@ -413,6 +415,14 @@ impl Default for ApiVersion { #[non_exhaustive] pub struct Mappings { /// Configures how the kernel stack should be mapped. + /// + /// If a fixed address is set, it must be page aligned. + /// + /// Note that the first page of the kernel stack is intentionally left unmapped + /// to act as a guard page. This ensures that a page fault occurs on a stack + /// overflow. For example, setting the kernel stack address to + /// `FixedAddress(0xf_0000_0000)` will result in a guard page at address + /// `0xf_0000_0000` and the kernel stack starting at address `0xf_0000_1000`. pub kernel_stack: Mapping, /// Specifies where the [`crate::BootInfo`] struct should be placed in virtual memory. pub boot_info: Mapping, From e8d379672863e51bbb4e255b52dfab9fd2847a42 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 27 Jan 2023 14:08:03 +0100 Subject: [PATCH 6/6] Fix typos --- common/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/src/lib.rs b/common/src/lib.rs index 9c9a4b2a..44aa3883 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -332,7 +332,7 @@ where let size = max_phys.as_u64(); let alignment = Size2MiB::SIZE; let offset = mapping_addr(mapping, size, alignment, &mut used_entries) - .expect("start addraess for physical memory mapping must be 2MiB-page-aligned"); + .expect("start address for physical memory mapping must be 2MiB-page-aligned"); for frame in PhysFrame::range_inclusive(start_frame, end_frame) { let page = Page::containing_address(offset + frame.start_address().as_u64()); @@ -388,7 +388,7 @@ where Mappings { framebuffer: framebuffer_virt_addr, entry_point, - // Use the configured stack size, even if it's not page aligned. However, we + // Use the configured stack size, even if it's not page-aligned. However, we // need to align it down to the next 16-byte boundary because the System V // ABI requires a 16-byte stack alignment. stack_top: stack_end_addr.align_down(16u8),