Fix higher half kernels by identity mapping context switch fn earlier

We used to reverve two frames to allow the context switch function to identity-map itself after creating the boot info. This works for kernels that use the standard mapping since they don't need to create new level 3/2 page tables for that. For kernels that are mapped differently (e.g. to the higher half), two frames aren't enough and a frame allocation error occurs. While we could fix this by reserving a larger number of frames, this would result in lost frames for normally-mapped kernels. So we instead do the identity-mapping already in the `set_up_mappings` function, where the normal frame allocator still exists. This way, we no longer need the `TwoFrames` workaaround at all.

To ensure that this all works as intended, this PR also adds a new `higher_half` test kernel that maps itself to address `0xFFFF800000000000` via its custom target. By reading the `rip` register, the test kernel can verify that it was indeed loaded to the higher half.

Author: phil-opp

Cargo.lock

@@ -13,6 +13,7 @@ members = [
     "tests/runner",
     "tests/test_kernels/default_settings",
     "tests/test_kernels/map_phys_mem",
+    "tests/test_kernels/higher_half",
 ]
 
 [[bin]]

Cargo.toml

@@ -10,10 +10,9 @@ use level_4_entries::UsedLevel4Entries;
 use parsed_config::CONFIG;
 use usize_conversions::FromUsize;
 use x86_64::{
-    registers,
     structures::paging::{
         FrameAllocator, Mapper, OffsetPageTable, Page, PageTableFlags, PageTableIndex, PhysFrame,
-        Size2MiB, Size4KiB,
+        Size2MiB,
     },
     PhysAddr, VirtAddr,
 };
@@ -90,13 +89,13 @@ where
         system_info.framebuffer_addr,
         system_info.framebuffer_info.byte_len,
     );
-    let (boot_info, two_frames) = create_boot_info(
+    let boot_info = create_boot_info(
         frame_allocator,
         &mut page_tables,
         &mut mappings,
         system_info,
     );
-    switch_to_kernel(page_tables, mappings, boot_info, two_frames);
+    switch_to_kernel(page_tables, mappings, boot_info);
 }
 
 /// Sets up mappings for a kernel stack and the framebuffer.
@@ -153,6 +152,20 @@ where
             .flush();
     }
 
+    // identity-map context switch function, so that we don't get an immediate pagefault
+    // after switching the active page table
+    let context_switch_function = PhysAddr::new(context_switch as *const () as u64);
+    let context_switch_function_start_frame: PhysFrame =
+        PhysFrame::containing_address(context_switch_function);
+    for frame in PhysFrame::range_inclusive(
+        context_switch_function_start_frame,
+        context_switch_function_start_frame + 1,
+    ) {
+        unsafe { kernel_page_table.identity_map(frame, PageTableFlags::PRESENT, frame_allocator) }
+            .unwrap()
+            .flush();
+    }
+
     // map framebuffer
     let framebuffer_virt_addr = if CONFIG.map_framebuffer {
         log::info!("Map framebuffer");
@@ -262,7 +275,7 @@ pub fn create_boot_info<I, D>(
     page_tables: &mut PageTables,
     mappings: &mut Mappings,
     system_info: SystemInfo,
-) -> (&'static mut BootInfo, TwoFrames)
+) -> &'static mut BootInfo
 where
     I: ExactSizeIterator<Item = D> + Clone,
     D: LegacyMemoryRegion,
@@ -310,9 +323,6 @@ where
         (boot_info, memory_regions)
     };
 
-    // reserve two unused frames for context switch
-    let two_frames = TwoFrames::new(&mut frame_allocator);
-
     log::info!("Create Memory Map");
 
     // build memory map
@@ -341,19 +351,17 @@ where
         tls_template: mappings.tls_template.into(),
     });
 
-    (boot_info, two_frames)
+    boot_info
 }
 
 /// Switches to the kernel address space and jumps to the kernel entry point.
 pub fn switch_to_kernel(
     page_tables: PageTables,
     mappings: Mappings,
     boot_info: &'static mut BootInfo,
-    two_frames: TwoFrames,
 ) -> ! {
     let PageTables {
         kernel_level_4_frame,
-        kernel: kernel_page_table,
         ..
     } = page_tables;
     let addresses = Addresses {
@@ -369,7 +377,7 @@ pub fn switch_to_kernel(
     );
 
     unsafe {
-        context_switch(addresses, kernel_page_table, two_frames);
+        context_switch(addresses);
     }
 }
 
@@ -388,32 +396,7 @@ pub struct PageTables {
 }
 
 /// Performs the actual context switch.
-///
-/// This function uses the given `frame_allocator` to identity map itself in the kernel-level
-/// page table. This is required to avoid a page fault after the context switch. Since this
-/// function is relatively small, only up to two physical frames are required from the frame
-/// allocator, so the [`TwoFrames`] type can be used here.
-unsafe fn context_switch(
-    addresses: Addresses,
-    mut kernel_page_table: OffsetPageTable,
-    mut frame_allocator: impl FrameAllocator<Size4KiB>,
-) -> ! {
-    // identity-map current and next frame, so that we don't get an immediate pagefault
-    // after switching the active page table
-    let current_addr = PhysAddr::new(registers::read_rip().as_u64());
-    let current_frame: PhysFrame = PhysFrame::containing_address(current_addr);
-    for frame in PhysFrame::range_inclusive(current_frame, current_frame + 1) {
-        unsafe {
-            kernel_page_table.identity_map(frame, PageTableFlags::PRESENT, &mut frame_allocator)
-        }
-        .unwrap()
-        .flush();
-    }
-
-    // we don't need the kernel page table anymore
-    mem::drop(kernel_page_table);
-
-    // do the context switch
+unsafe fn context_switch(addresses: Addresses) -> ! {
     unsafe {
         asm!(
             "mov cr3, {}; mov rsp, {}; push 0; jmp {}",
@@ -434,36 +417,6 @@ struct Addresses {
     boot_info: &'static mut crate::boot_info::BootInfo,
 }
 
-/// Used for reversing two physical frames for identity mapping the context switch function.
-///
-/// In order to prevent a page fault, the context switch function must be mapped identically in
-/// both address spaces. The context switch function is small, so this mapping requires only
-/// two physical frames (one frame is not enough because the linker might place the function
-/// directly before a page boundary). Since the frame allocator no longer exists when the
-/// context switch function is invoked, we use this type to reserve two physical frames
-/// beforehand.
-pub struct TwoFrames {
-    frames: [Option<PhysFrame>; 2],
-}
-
-impl TwoFrames {
-    /// Creates a new instance by allocating two physical frames from the given frame allocator.
-    pub fn new(frame_allocator: &mut impl FrameAllocator<Size4KiB>) -> Self {
-        TwoFrames {
-            frames: [
-                Some(frame_allocator.allocate_frame().unwrap()),
-                Some(frame_allocator.allocate_frame().unwrap()),
-            ],
-        }
-    }
-}
-
-unsafe impl FrameAllocator<Size4KiB> for TwoFrames {
-    fn allocate_frame(&mut self) -> Option<PhysFrame<Size4KiB>> {
-        self.frames.iter_mut().find_map(|f| f.take())
-    }
-}
-
 fn boot_info_location(used_entries: &mut UsedLevel4Entries) -> VirtAddr {
     CONFIG
         .boot_info_address

src/binary/mod.rs

@@ -0,0 +1,32 @@
+use std::process::Command;
+
+#[test]
+fn basic_boot() {
+    run_test_binary("basic_boot");
+}
+
+#[test]
+fn should_panic() {
+    run_test_binary("should_panic");
+}
+
+#[test]
+fn check_boot_info() {
+    run_test_binary("check_boot_info");
+}
+
+#[test]
+fn verify_higher_half() {
+    run_test_binary("verify_higher_half");
+}
+
+fn run_test_binary(bin_name: &str) {
+    let mut cmd = Command::new(env!("CARGO"));
+    cmd.current_dir("tests/test_kernels/higher_half");
+    cmd.arg("run");
+    cmd.arg("--bin").arg(bin_name);
+    cmd.arg("--target").arg("x86_64-higher_half.json");
+    cmd.arg("-Zbuild-std=core");
+    cmd.arg("-Zbuild-std-features=compiler-builtins-mem");
+    assert!(cmd.status().unwrap().success());
+}

tests/higher_half.rs

@@ -0,0 +1,10 @@
+[unstable]
+# TODO: Uncomment once https://github.com/rust-lang/cargo/issues/8643 is merged
+# build-std = ["core"]
+
+[build]
+# TODO: Uncomment once https://github.com/rust-lang/cargo/issues/8643 is merged
+# target = "x86_64-example-kernel.json"
+
+[target.'cfg(target_os = "none")']
+runner = "cargo run --manifest-path ../../runner/Cargo.toml"

tests/test_kernels/higher_half/.cargo/config.toml

@@ -0,0 +1 @@
+target

tests/test_kernels/higher_half/.gitignore

@@ -0,0 +1,10 @@
+[package]
+name = "test_kernel_higher_half"
+version = "0.1.0"
+authors = ["Philipp Oppermann <[email protected]>"]
+edition = "2018"
+
+[dependencies]
+bootloader = { path = "../../.." }
+x86_64 = { version = "0.13.2", default-features = false, features = ["instructions", "inline_asm"] }
+uart_16550 = "0.2.10"

tests/test_kernels/higher_half/Cargo.toml

@@ -0,0 +1,18 @@
+#![no_std] // don't link the Rust standard library
+#![no_main] // disable all Rust-level entry points
+
+use bootloader::{entry_point, BootInfo};
+use core::panic::PanicInfo;
+use test_kernel_higher_half::{exit_qemu, QemuExitCode};
+
+entry_point!(kernel_main);
+
+fn kernel_main(_boot_info: &'static mut BootInfo) -> ! {
+    exit_qemu(QemuExitCode::Success);
+}
+
+/// This function is called on panic.
+#[panic_handler]
+fn panic(_info: &PanicInfo) -> ! {
+    exit_qemu(QemuExitCode::Failed);
+}

tests/test_kernels/higher_half/src/bin/basic_boot.rs

@@ -0,0 +1,68 @@
+#![no_std] // don't link the Rust standard library
+#![no_main] // disable all Rust-level entry points
+
+use bootloader::{boot_info::PixelFormat, entry_point, BootInfo};
+use core::panic::PanicInfo;
+use test_kernel_higher_half::{exit_qemu, QemuExitCode};
+
+entry_point!(kernel_main);
+
+fn kernel_main(boot_info: &'static mut BootInfo) -> ! {
+    // check memory regions
+    assert!(boot_info.memory_regions.len() > 4);
+
+    // check framebuffer
+    let framebuffer = boot_info.framebuffer.as_ref().unwrap();
+    assert_eq!(framebuffer.info().byte_len, framebuffer.buffer().len());
+    if ![640, 1024].contains(&framebuffer.info().horizontal_resolution) {
+        panic!(
+            "unexpected horizontal_resolution `{}`",
+            framebuffer.info().horizontal_resolution
+        );
+    }
+    if ![480, 768].contains(&framebuffer.info().vertical_resolution) {
+        panic!(
+            "unexpected vertical_resolution `{}`",
+            framebuffer.info().vertical_resolution
+        );
+    }
+    if ![3, 4].contains(&framebuffer.info().bytes_per_pixel) {
+        panic!(
+            "unexpected bytes_per_pixel `{}`",
+            framebuffer.info().bytes_per_pixel
+        );
+    }
+    if ![640, 1024].contains(&framebuffer.info().stride) {
+        panic!("unexpected stride `{}`", framebuffer.info().stride);
+    }
+    assert_eq!(framebuffer.info().pixel_format, PixelFormat::BGR);
+    assert_eq!(
+        framebuffer.buffer().len(),
+        framebuffer.info().stride
+            * framebuffer.info().vertical_resolution
+            * framebuffer.info().bytes_per_pixel
+    );
+
+    // check defaults for optional features
+    assert_eq!(boot_info.physical_memory_offset.into_option(), None);
+    assert_eq!(boot_info.recursive_index.into_option(), None);
+
+    // check rsdp_addr
+    let rsdp = boot_info.rsdp_addr.into_option().unwrap();
+    assert!(rsdp > 0x000E0000);
+    assert!(rsdp < 0x000FFFFF);
+
+    // the test kernel has no TLS template
+    assert_eq!(boot_info.tls_template.into_option(), None);
+
+    exit_qemu(QemuExitCode::Success);
+}
+
+/// This function is called on panic.
+#[panic_handler]
+fn panic(info: &PanicInfo) -> ! {
+    use core::fmt::Write;
+
+    let _ = writeln!(test_kernel_higher_half::serial(), "PANIC: {}", info);
+    exit_qemu(QemuExitCode::Failed);
+}

tests/test_kernels/higher_half/src/bin/check_boot_info.rs

@@ -0,0 +1,18 @@
+#![no_std] // don't link the Rust standard library
+#![no_main] // disable all Rust-level entry points
+
+use bootloader::{entry_point, BootInfo};
+use core::panic::PanicInfo;
+use test_kernel_higher_half::{exit_qemu, QemuExitCode};
+
+entry_point!(kernel_main);
+
+fn kernel_main(_boot_info: &'static mut BootInfo) -> ! {
+    panic!();
+}
+
+/// This function is called on panic.
+#[panic_handler]
+fn panic(_info: &PanicInfo) -> ! {
+    exit_qemu(QemuExitCode::Success);
+}