Fix higher half kernels by identity mapping context switch fn earlier

Cargo.toml

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

Changelog.md

@@ -1,6 +1,8 @@
 # Unreleased
 
 - Fix build on latest Rust nightly by updating to `uefi` v0.9.0 ([#162](https://github.com/rust-osdev/bootloader/pull/162))
+- Fix higher half kernels by identity mapping context switch fn earlier ([#161](https://github.com/rust-osdev/bootloader/pull/161))
+  - Also: improve reporting of mapping errors
 
 # 0.10.3 – 2021-05-05
 

src/binary/mod.rs

@@ -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.
@@ -148,9 +147,27 @@ where
             .allocate_frame()
             .expect("frame allocation failed when mapping a kernel stack");
         let flags = PageTableFlags::PRESENT | PageTableFlags::WRITABLE;
-        unsafe { kernel_page_table.map_to(page, frame, flags, frame_allocator) }
-            .unwrap()
-            .flush();
+        match unsafe { kernel_page_table.map_to(page, frame, flags, frame_allocator) } {
+            Ok(tlb) => tlb.flush(),
+            Err(err) => panic!("failed to map page {:?}: {:?}", page, err),
+        }
+    }
+
+    // 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,
+    ) {
+        match unsafe {
+            kernel_page_table.identity_map(frame, PageTableFlags::PRESENT, frame_allocator)
+        } {
+            Ok(tlb) => tlb.flush(),
+            Err(err) => panic!("failed to identity map frame {:?}: {:?}", frame, err),
+        }
     }
 
     // map framebuffer
@@ -166,9 +183,13 @@ where
         {
             let page = start_page + u64::from_usize(i);
             let flags = PageTableFlags::PRESENT | PageTableFlags::WRITABLE;
-            unsafe { kernel_page_table.map_to(page, frame, flags, frame_allocator) }
-                .unwrap()
-                .flush();
+            match unsafe { kernel_page_table.map_to(page, frame, flags, frame_allocator) } {
+                Ok(tlb) => tlb.flush(),
+                Err(err) => panic!(
+                    "failed to map page {:?} to frame {:?}: {:?}",
+                    page, frame, err
+                ),
+            }
         }
         let framebuffer_virt_addr = start_page.start_address();
         Some(framebuffer_virt_addr)
@@ -189,9 +210,13 @@ where
         for frame in PhysFrame::range_inclusive(start_frame, end_frame) {
             let page = Page::containing_address(offset + frame.start_address().as_u64());
             let flags = PageTableFlags::PRESENT | PageTableFlags::WRITABLE;
-            unsafe { kernel_page_table.map_to(page, frame, flags, frame_allocator) }
-                .unwrap()
-                .ignore();
+            match unsafe { kernel_page_table.map_to(page, frame, flags, frame_allocator) } {
+                Ok(tlb) => tlb.ignore(),
+                Err(err) => panic!(
+                    "failed to map page {:?} to frame {:?}: {:?}",
+                    page, frame, err
+                ),
+            };
         }
 
         Some(offset)
@@ -262,7 +287,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,
@@ -286,21 +311,23 @@ where
             let frame = frame_allocator
                 .allocate_frame()
                 .expect("frame allocation for boot info failed");
-            unsafe {
+            match unsafe {
                 page_tables
                     .kernel
                     .map_to(page, frame, flags, &mut frame_allocator)
+            } {
+                Ok(tlb) => tlb.flush(),
+                Err(err) => panic!("failed to map page {:?}: {:?}", page, err),
             }
-            .unwrap()
-            .flush();
             // we need to be able to access it too
-            unsafe {
+            match unsafe {
                 page_tables
                     .bootloader
                     .map_to(page, frame, flags, &mut frame_allocator)
+            } {
+                Ok(tlb) => tlb.flush(),
+                Err(err) => panic!("failed to map page {:?}: {:?}", page, err),
             }
-            .unwrap()
-            .flush();
         }
 
         let boot_info: &'static mut MaybeUninit<BootInfo> =
@@ -310,9 +337,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 +365,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 +391,7 @@ pub fn switch_to_kernel(
     );
 
     unsafe {
-        context_switch(addresses, kernel_page_table, two_frames);
+        context_switch(addresses);
     }
 }
 
@@ -388,32 +410,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 +431,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

tests/higher_half.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/test_kernels/higher_half/.cargo/config.toml

@@ -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/.gitignore

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

tests/test_kernels/higher_half/Cargo.toml

@@ -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/src/bin/basic_boot.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) -> ! {
+    exit_qemu(QemuExitCode::Success);
+}
+
+/// This function is called on panic.
+#[panic_handler]
+fn panic(_info: &PanicInfo) -> ! {
+    exit_qemu(QemuExitCode::Failed);
+}