From 2f5fdf04d72ee78746f6addbbbc12d45eb17ab2f Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 30 Jan 2023 08:38:58 +0100 Subject: [PATCH 1/5] Test that boot config is actually loaded (through a sentinel value) --- api/src/info.rs | 4 ++++ bios/stage-4/src/main.rs | 2 +- common/config/src/lib.rs | 3 +++ common/src/lib.rs | 6 +++++- tests/config_file.rs | 12 ++++++------ ...c_boot_broken_config_file.rs => custom_config.rs} | 1 + .../src/bin/{basic_boot.rs => no_config.rs} | 1 + uefi/src/main.rs | 5 +++-- 8 files changed, 24 insertions(+), 10 deletions(-) rename tests/test_kernels/config_file/src/bin/{basic_boot_broken_config_file.rs => custom_config.rs} (91%) rename tests/test_kernels/config_file/src/bin/{basic_boot.rs => no_config.rs} (93%) diff --git a/api/src/info.rs b/api/src/info.rs index 771948de..cdb48722 100644 --- a/api/src/info.rs +++ b/api/src/info.rs @@ -56,6 +56,9 @@ pub struct BootInfo { pub ramdisk_addr: Optional, /// Ramdisk image size, set to 0 if addr is None pub ramdisk_len: u64, + + #[doc(hidden)] + pub _test_sentinel: u64, } impl BootInfo { @@ -73,6 +76,7 @@ impl BootInfo { tls_template: Optional::None, ramdisk_addr: Optional::None, ramdisk_len: 0, + _test_sentinel: 0, } } } diff --git a/bios/stage-4/src/main.rs b/bios/stage-4/src/main.rs index f51f2252..cc7f091d 100644 --- a/bios/stage-4/src/main.rs +++ b/bios/stage-4/src/main.rs @@ -173,7 +173,7 @@ pub extern "C" fn _start(info: &mut BiosInfo) -> ! { ramdisk_len: info.ramdisk.len, }; - load_and_switch_to_kernel(kernel, frame_allocator, page_tables, system_info); + load_and_switch_to_kernel(kernel, config, frame_allocator, page_tables, system_info); } fn init_logger( diff --git a/common/config/src/lib.rs b/common/config/src/lib.rs index 2286ad16..267cb64f 100644 --- a/common/config/src/lib.rs +++ b/common/config/src/lib.rs @@ -22,6 +22,8 @@ pub struct BootConfig { /// /// Enabled by default. pub serial_logging: bool, + + pub _test_sentinel: u64, } impl Default for BootConfig { @@ -31,6 +33,7 @@ impl Default for BootConfig { log_level: Default::default(), frame_buffer_logging: true, serial_logging: true, + _test_sentinel: 0, } } } diff --git a/common/src/lib.rs b/common/src/lib.rs index 97778db1..3a0e3163 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -8,7 +8,7 @@ use bootloader_api::{ info::{FrameBuffer, FrameBufferInfo, MemoryRegion, TlsTemplate}, BootInfo, BootloaderConfig, }; -use bootloader_boot_config::LevelFilter; +use bootloader_boot_config::{BootConfig, LevelFilter}; use core::{alloc::Layout, arch::asm, mem::MaybeUninit, slice}; use level_4_entries::UsedLevel4Entries; use usize_conversions::FromUsize; @@ -125,6 +125,7 @@ impl<'a> Kernel<'a> { /// directly to these functions, so see their docs for more info. pub fn load_and_switch_to_kernel( kernel: Kernel, + boot_config: BootConfig, mut frame_allocator: LegacyFrameAllocator, mut page_tables: PageTables, system_info: SystemInfo, @@ -144,6 +145,7 @@ where ); let boot_info = create_boot_info( &config, + &boot_config, frame_allocator, &mut page_tables, &mut mappings, @@ -435,6 +437,7 @@ pub struct Mappings { /// are taken from the given `frame_allocator`. pub fn create_boot_info( config: &BootloaderConfig, + boot_config: &BootConfig, mut frame_allocator: LegacyFrameAllocator, page_tables: &mut PageTables, mappings: &mut Mappings, @@ -539,6 +542,7 @@ where .map(|addr| addr.as_u64()) .into(); info.ramdisk_len = mappings.ramdisk_slice_len; + info._test_sentinel = boot_config._test_sentinel; info }); diff --git a/tests/config_file.rs b/tests/config_file.rs index af5c0f31..69f805dc 100644 --- a/tests/config_file.rs +++ b/tests/config_file.rs @@ -3,25 +3,25 @@ use bootloader_test_runner::run_test_kernel_internal; use bootloader::BootConfig; #[test] -fn basic_boot() { - let config: BootConfig = Default::default(); +fn default_config() { run_test_kernel_internal( - env!("CARGO_BIN_FILE_TEST_KERNEL_CONFIG_FILE_basic_boot"), + env!("CARGO_BIN_FILE_TEST_KERNEL_CONFIG_FILE_no_config"), + None, None, - Some(&config), ); } #[test] -fn custom_options_boot() { +fn custom_boot_config() { let config = BootConfig { frame_buffer: Default::default(), log_level: Default::default(), frame_buffer_logging: false, serial_logging: true, + _test_sentinel: 0xb001b001b001, }; run_test_kernel_internal( - env!("CARGO_BIN_FILE_TEST_KERNEL_CONFIG_FILE_basic_boot_broken_config_file"), + env!("CARGO_BIN_FILE_TEST_KERNEL_CONFIG_FILE_custom_config"), None, Some(&config), ); diff --git a/tests/test_kernels/config_file/src/bin/basic_boot_broken_config_file.rs b/tests/test_kernels/config_file/src/bin/custom_config.rs similarity index 91% rename from tests/test_kernels/config_file/src/bin/basic_boot_broken_config_file.rs rename to tests/test_kernels/config_file/src/bin/custom_config.rs index 5232aa80..425e268a 100644 --- a/tests/test_kernels/config_file/src/bin/basic_boot_broken_config_file.rs +++ b/tests/test_kernels/config_file/src/bin/custom_config.rs @@ -9,6 +9,7 @@ entry_point!(kernel_main); fn kernel_main(boot_info: &'static mut BootInfo) -> ! { writeln!(serial(), "Entered kernel with boot info: {boot_info:?}").unwrap(); + assert_eq!(boot_info._test_sentinel, 0xb001b001b001); exit_qemu(QemuExitCode::Success); } diff --git a/tests/test_kernels/config_file/src/bin/basic_boot.rs b/tests/test_kernels/config_file/src/bin/no_config.rs similarity index 93% rename from tests/test_kernels/config_file/src/bin/basic_boot.rs rename to tests/test_kernels/config_file/src/bin/no_config.rs index 5232aa80..d0898bde 100644 --- a/tests/test_kernels/config_file/src/bin/basic_boot.rs +++ b/tests/test_kernels/config_file/src/bin/no_config.rs @@ -9,6 +9,7 @@ entry_point!(kernel_main); fn kernel_main(boot_info: &'static mut BootInfo) -> ! { writeln!(serial(), "Entered kernel with boot info: {boot_info:?}").unwrap(); + assert_eq!(boot_info._test_sentinel, 0); exit_qemu(QemuExitCode::Success); } diff --git a/uefi/src/main.rs b/uefi/src/main.rs index 56aea62c..e5c5b7ce 100644 --- a/uefi/src/main.rs +++ b/uefi/src/main.rs @@ -105,7 +105,7 @@ fn main_inner(image: Handle, mut st: SystemTable) -> Status { config.frame_buffer.minimum_framebuffer_width = kernel.config.frame_buffer.minimum_framebuffer_width; } - let framebuffer = init_logger(image, &st, config); + let framebuffer = init_logger(image, &st, &config); unsafe { *SYSTEM_TABLE.get() = None; @@ -193,6 +193,7 @@ fn main_inner(image: Handle, mut st: SystemTable) -> Status { bootloader_x86_64_common::load_and_switch_to_kernel( kernel, + config, frame_allocator, page_tables, system_info, @@ -473,7 +474,7 @@ fn create_page_tables( fn init_logger( image_handle: Handle, st: &SystemTable, - config: BootConfig, + config: &BootConfig, ) -> Option { let gop_handle = st .boot_services() From 510a44bc357b6443a820e4ef4fbc41ee3b098c4c Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 30 Jan 2023 08:39:33 +0100 Subject: [PATCH 2/5] Use correct file name when loading boot config file --- bios/stage-2/src/main.rs | 2 +- uefi/src/main.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bios/stage-2/src/main.rs b/bios/stage-2/src/main.rs index d07957c8..1321519c 100644 --- a/bios/stage-2/src/main.rs +++ b/bios/stage-2/src/main.rs @@ -115,7 +115,7 @@ fn start(disk_number: u16, partition_table_start: *const u8) -> ! { } let config_file_start = ramdisk_start.wrapping_add(ramdisk_len.try_into().unwrap()); let config_file_len = try_load_file( - "config.json", + "boot.json", config_file_start, &mut fs, &mut disk, diff --git a/uefi/src/main.rs b/uefi/src/main.rs index e5c5b7ce..e917c2a8 100644 --- a/uefi/src/main.rs +++ b/uefi/src/main.rs @@ -219,7 +219,7 @@ fn load_config_file( st: &mut SystemTable, boot_mode: BootMode, ) -> Option<&'static mut [u8]> { - load_file_from_boot_method(image, st, "config.json\0", boot_mode) + load_file_from_boot_method(image, st, "boot.json\0", boot_mode) } fn load_kernel( From c87d04133c24d846cacb4a708b7a4e13b22f8897 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 30 Jan 2023 08:40:02 +0100 Subject: [PATCH 3/5] Include boot config when using TFTP booting --- src/uefi/mod.rs | 1 + src/uefi/pxe.rs | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/src/uefi/mod.rs b/src/uefi/mod.rs index 69024718..030ea9bb 100644 --- a/src/uefi/mod.rs +++ b/src/uefi/mod.rs @@ -68,6 +68,7 @@ impl UefiBoot { bootloader_path, self.kernel.as_path(), self.ramdisk.as_deref(), + self.config.as_deref(), out_path, ) .context("failed to create UEFI PXE tftp folder")?; diff --git a/src/uefi/pxe.rs b/src/uefi/pxe.rs index 84c7b0d6..4eac7f8c 100644 --- a/src/uefi/pxe.rs +++ b/src/uefi/pxe.rs @@ -1,11 +1,13 @@ use std::path::Path; use anyhow::Context; +use bootloader_boot_config::BootConfig; pub fn create_uefi_tftp_folder( bootloader_path: &Path, kernel_binary: &Path, ramdisk_path: Option<&Path>, + boot_config: Option<&str>, out_path: &Path, ) -> anyhow::Result<()> { std::fs::create_dir_all(out_path) @@ -39,5 +41,10 @@ pub fn create_uefi_tftp_folder( })?; } + if let Some(config) = boot_config { + let to = out_path.join("boot.json"); + std::fs::write(to, config).context("failed to write boot.json")?; + } + Ok(()) } From 38aa26639eaeeffcafb8d6ddb12ed2025110dfc7 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 30 Jan 2023 08:40:38 +0100 Subject: [PATCH 4/5] Fix: Load kernel first, then use same boot mode for config file We previously tried to always load the config file from disk. --- uefi/src/main.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/uefi/src/main.rs b/uefi/src/main.rs index e917c2a8..d0cee33b 100644 --- a/uefi/src/main.rs +++ b/uefi/src/main.rs @@ -73,6 +73,15 @@ fn main_inner(image: Handle, mut st: SystemTable) -> Status { } let mut boot_mode = BootMode::Disk; + + let mut kernel = load_kernel(image, &mut st, boot_mode); + if kernel.is_none() { + // Try TFTP boot + boot_mode = BootMode::Tftp; + kernel = load_kernel(image, &mut st, boot_mode); + } + let kernel = kernel.expect("Failed to load kernel"); + let config_file = load_config_file(image, &mut st, boot_mode); let mut error_loading_config: Option = None; let mut config: BootConfig = match config_file @@ -87,14 +96,6 @@ fn main_inner(image: Handle, mut st: SystemTable) -> Status { } }; - let mut kernel = load_kernel(image, &mut st, boot_mode); - if kernel.is_none() { - // Try TFTP boot - boot_mode = BootMode::Tftp; - kernel = load_kernel(image, &mut st, boot_mode); - } - let kernel = kernel.expect("Failed to load kernel"); - #[allow(deprecated)] if config.frame_buffer.minimum_framebuffer_height.is_none() { config.frame_buffer.minimum_framebuffer_height = From f03065e9c740e4c9c69d17d3ec740b67c8fd38e1 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 30 Jan 2023 08:49:23 +0100 Subject: [PATCH 5/5] Fix: Consider config file when initializing frame allocator We still considered the ramdisk file as the last used frame when initializing the frame allocator, so we overwrote the config file. This commit fixes this by setting the `last_used_addr` correctly when creating the `BiosInfo`. --- bios/common/src/lib.rs | 1 + bios/stage-2/src/main.rs | 1 + bios/stage-4/src/main.rs | 9 +-------- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/bios/common/src/lib.rs b/bios/common/src/lib.rs index 6d34e559..166e12df 100644 --- a/bios/common/src/lib.rs +++ b/bios/common/src/lib.rs @@ -9,6 +9,7 @@ pub struct BiosInfo { pub kernel: Region, pub ramdisk: Region, pub config_file: Region, + pub last_used_addr: u64, pub framebuffer: BiosFramebufferInfo, pub memory_map_addr: u32, pub memory_map_len: u16, diff --git a/bios/stage-2/src/main.rs b/bios/stage-2/src/main.rs index 1321519c..b5b07d65 100644 --- a/bios/stage-2/src/main.rs +++ b/bios/stage-2/src/main.rs @@ -161,6 +161,7 @@ fn start(disk_number: u16, partition_table_start: *const u8) -> ! { start: config_file_start as u64, len: config_file_len, }, + last_used_addr: config_file_start as u64 + config_file_len - 1, memory_map_addr: memory_map.as_mut_ptr() as u32, memory_map_len: memory_map.len().try_into().unwrap(), framebuffer: BiosFramebufferInfo { diff --git a/bios/stage-4/src/main.rs b/bios/stage-4/src/main.rs index cc7f091d..260dd622 100644 --- a/bios/stage-4/src/main.rs +++ b/bios/stage-4/src/main.rs @@ -54,14 +54,7 @@ pub extern "C" fn _start(info: &mut BiosInfo) -> ! { PhysAddr::new(info.kernel.start) }; let kernel_size = info.kernel.len; - let next_free_frame = match info.ramdisk.len { - 0 => PhysFrame::containing_address(kernel_start + kernel_size - 1u64) + 1, - _ => { - PhysFrame::containing_address(PhysAddr::new( - info.ramdisk.start + info.ramdisk.len - 1u64, - )) + 1 - } - }; + let next_free_frame = PhysFrame::containing_address(PhysAddr::new(info.last_used_addr)) + 1; let mut frame_allocator = LegacyFrameAllocator::new_starting_at( next_free_frame, memory_map.iter().copied().map(MemoryRegion),