Guard the lower 1MB of memory

This commit fixes the review changes in #317.
Most of the credit should go to Jason Couture <jasonc@alertr.info>

Co-authored-by: Jason Couture <jasonc@alertr.info>

Author: Wasabi375

Cargo.lock

@@ -26,6 +26,7 @@ members = [
     "tests/test_kernels/lto",
     "tests/test_kernels/ramdisk",
     "tests/test_kernels/min_stack",
+    "tests/test_kernels/lower_memory_free",
 ]
 exclude = ["examples/basic", "examples/test_framework"]
 
@@ -67,6 +68,7 @@ test_kernel_pie = { path = "tests/test_kernels/pie", artifact = "bin", target =
 test_kernel_ramdisk = { path = "tests/test_kernels/ramdisk", artifact = "bin", target = "x86_64-unknown-none" }
 test_kernel_config_file = { path = "tests/test_kernels/config_file", artifact = "bin", target = "x86_64-unknown-none" }
 test_kernel_min_stack = { path = "tests/test_kernels/min_stack", artifact = "bin", target = "x86_64-unknown-none" }
+test_kernel_lower_memory_free = { path = "tests/test_kernels/lower_memory_free", artifact = "bin", target = "x86_64-unknown-none" }
 
 [profile.dev]
 panic = "abort"

Cargo.toml

@@ -28,8 +28,12 @@ pub struct LegacyFrameAllocator<I, D> {
     memory_map: I,
     current_descriptor: Option<D>,
     next_frame: PhysFrame,
+    min_frame: PhysFrame,
 }
 
+/// Start address of the first frame that is not part of the lower 1MB of frames
+const LOWER_MEMORY_END_PAGE: u64 = 0x100_000;
+
 impl<I, D> LegacyFrameAllocator<I, D>
 where
     I: ExactSizeIterator<Item = D> + Clone,
@@ -40,20 +44,26 @@ where
     /// Skips the frame at physical address zero to avoid potential problems. For example
     /// identity-mapping the frame at address zero is not valid in Rust, because Rust's `core`
     /// library assumes that references can never point to virtual address `0`.  
+    /// Also skips the lower 1MB of frames, there are use cases that require lower conventional memory access (Such as SMP SIPI).
     pub fn new(memory_map: I) -> Self {
         // skip frame 0 because the rust core library does not see 0 as a valid address
-        let start_frame = PhysFrame::containing_address(PhysAddr::new(0x1000));
+        // Also skip at least the lower 1MB of frames, there are use cases that require lower conventional memory access (Such as SMP SIPI).
+        let start_frame = PhysFrame::containing_address(PhysAddr::new(LOWER_MEMORY_END_PAGE));
         Self::new_starting_at(start_frame, memory_map)
     }
 
     /// Creates a new frame allocator based on the given legacy memory regions. Skips any frames
-    /// before the given `frame`.
+    /// before the given `frame` or `0x10000`(1MB) whichever is higher, there are use cases that require
+    /// lower conventional memory access (Such as SMP SIPI).
     pub fn new_starting_at(frame: PhysFrame, memory_map: I) -> Self {
+        let lower_mem_end = PhysFrame::containing_address(PhysAddr::new(LOWER_MEMORY_END_PAGE));
+        let frame = core::cmp::max(frame, lower_mem_end);
         Self {
             original: memory_map.clone(),
             memory_map,
             current_descriptor: None,
             next_frame: frame,
+            min_frame: frame,
         }
     }
 
@@ -71,6 +81,7 @@ where
         if self.next_frame <= end_frame {
             let ret = self.next_frame;
             self.next_frame += 1;
+
             Some(ret)
         } else {
             None
@@ -125,14 +136,44 @@ where
             let next_free = self.next_frame.start_address();
             let kind = match descriptor.kind() {
                 MemoryRegionKind::Usable => {
-                    if end <= next_free {
+                    if end <= next_free && start >= self.min_frame.start_address() {
                         MemoryRegionKind::Bootloader
                     } else if descriptor.start() >= next_free {
                         MemoryRegionKind::Usable
+                    } else if end <= self.min_frame.start_address() {
+                        // treat regions before min_frame as usable
+                        // this allows for access to the lower 1MB of frames
+                        MemoryRegionKind::Usable
+                    } else if end <= next_free {
+                        // part of the region is used -> add it separately
+                        // first part of the region is in lower 1MB, later part is used
+                        let free_region = MemoryRegion {
+                            start: descriptor.start().as_u64(),
+                            end: self.min_frame.start_address().as_u64(),
+                            kind: MemoryRegionKind::Usable,
+                        };
+                        Self::add_region(free_region, regions, &mut next_index);
+
+                        // add bootloader part normally
+                        start = self.min_frame.start_address();
+                        MemoryRegionKind::Bootloader
                     } else {
+                        if start < self.min_frame.start_address() {
+                            // part of the region is in lower memory
+                            let lower_region = MemoryRegion {
+                                start: start.as_u64(),
+                                end: self.min_frame.start_address().as_u64(),
+                                kind: MemoryRegionKind::Usable,
+                            };
+                            Self::add_region(lower_region, regions, &mut next_index);
+
+                            start = self.min_frame.start_address();
+                        }
+
                         // part of the region is used -> add it separately
+                        // first part of the region is used, later part is free
                         let used_region = MemoryRegion {
-                            start: descriptor.start().as_u64(),
+                            start: start.as_u64(),
                             end: next_free.as_u64(),
                             kind: MemoryRegionKind::Bootloader,
                         };

common/src/legacy_memory_region.rs

@@ -0,0 +1,7 @@
+use bootloader_test_runner::run_test_kernel;
+#[test]
+fn lower_memory_free() {
+    run_test_kernel(env!(
+        "CARGO_BIN_FILE_TEST_KERNEL_LOWER_MEMORY_FREE_lower_memory_free"
+    ));
+}

tests/lower_memory_free.rs

@@ -0,0 +1,12 @@
+[package]
+name = "test_kernel_lower_memory_free"
+version = "0.1.0"
+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"

tests/test_kernels/lower_memory_free/Cargo.toml

@@ -0,0 +1,65 @@
+#![no_std] // don't link the Rust standard library
+#![no_main] // disable all Rust-level entry points
+
+use bootloader_api::{
+    config::Mapping, entry_point, info::MemoryRegionKind, BootInfo, BootloaderConfig,
+};
+use test_kernel_lower_memory_free::{exit_qemu, QemuExitCode};
+
+const LOWER_MEMORY_END_PAGE: u64 = 0x0010_0000;
+const WRITE_TEST_UNTIL: u64 = 0x4000_0000;
+
+pub const BOOTLOADER_CONFIG: BootloaderConfig = {
+    let mut config = BootloaderConfig::new_default();
+    config.mappings.physical_memory = Some(Mapping::FixedAddress(0x0000_4000_0000_0000));
+    config
+};
+
+entry_point!(kernel_main, config = &BOOTLOADER_CONFIG);
+
+fn kernel_main(boot_info: &'static mut BootInfo) -> ! {
+    use core::fmt::Write;
+    use test_kernel_lower_memory_free::serial;
+
+    let phys_mem_offset = boot_info.physical_memory_offset.into_option().unwrap();
+
+    let mut count = 0;
+    for region in boot_info.memory_regions.iter() {
+        writeln!(
+            serial(),
+            "Region: {:016x}-{:016x} - {:?}",
+            region.start,
+            region.end,
+            region.kind
+        )
+        .unwrap();
+        if region.kind == MemoryRegionKind::Usable && region.start < LOWER_MEMORY_END_PAGE {
+            let end = core::cmp::min(region.end, LOWER_MEMORY_END_PAGE);
+            let pages = (end - region.start) / 4096;
+            count += pages;
+        }
+        if region.kind == MemoryRegionKind::Usable && region.start < WRITE_TEST_UNTIL {
+            let end = core::cmp::min(region.end, WRITE_TEST_UNTIL);
+            // ensure region is actually writable
+            let addr = phys_mem_offset + region.start;
+            let size = end - region.start;
+            unsafe {
+                core::ptr::write_bytes(addr as *mut u8, 0xff, size as usize);
+            }
+        }
+    }
+
+    writeln!(serial(), "Free lower memory page count: {}", count).unwrap();
+    assert!(count > 0x10); // 0x10 chosen arbirarily, we need _some_ free conventional memory, but not all of it. Some, especially on BIOS, may be reserved for hardware.
+    exit_qemu(QemuExitCode::Success);
+}
+
+/// This function is called on panic.
+#[panic_handler]
+#[cfg(not(test))]
+fn panic(info: &core::panic::PanicInfo) -> ! {
+    use core::fmt::Write;
+
+    let _ = writeln!(test_kernel_lower_memory_free::serial(), "PANIC: {}", info);
+    exit_qemu(QemuExitCode::Failed);
+}

tests/test_kernels/lower_memory_free/src/bin/lower_memory_free.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
+}