Add RoAfterInit to replace most static mut

Author: ALSchwalm

mythril/src/apic.rs

@@ -6,6 +6,8 @@ use crate::{declare_per_core, get_per_core, get_per_core_mut};
 use raw_cpuid::CpuId;
 use x86::msr;
 
+use core::fmt;
+
 /// APIC base physical address mask.
 const IA32_APIC_BASE_MASK: u64 = 0xffff_f000;
 /// xAPIC global enable mask
@@ -103,6 +105,32 @@ pub unsafe fn get_local_apic_mut() -> &'static mut LocalApic {
         .expect("Attempt to get local APIC before initialization")
 }
 
+/// A representation of a APIC ID
+#[derive(Copy, Clone, Debug, Ord, PartialEq, PartialOrd, Eq)]
+pub struct ApicId {
+    /// The raw ID as an integer
+    pub raw: u32,
+}
+
+impl ApicId {
+    /// Returns whether this is the BSP core
+    pub fn is_bsp(&self) -> bool {
+        self.raw == 0
+    }
+}
+
+impl From<u32> for ApicId {
+    fn from(value: u32) -> Self {
+        ApicId { raw: value }
+    }
+}
+
+impl fmt::Display for ApicId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "0x{:x}", self.raw)
+    }
+}
+
 /// Structure defining the interface for a local x2APIC
 #[derive(Debug)]
 pub struct LocalApic {
@@ -188,8 +216,10 @@ impl LocalApic {
     }
 
     /// The APIC ID
-    pub fn id(&self) -> u32 {
-        unsafe { msr::rdmsr(msr::IA32_X2APIC_APICID) as u32 }
+    pub fn id(&self) -> ApicId {
+        ApicId {
+            raw: unsafe { msr::rdmsr(msr::IA32_X2APIC_APICID) as u32 },
+        }
     }
 
     /// The Logical APIC ID
@@ -249,15 +279,15 @@ impl LocalApic {
     /// Set the Interrupt Command Register
     pub fn send_ipi(
         &mut self,
-        dst: u32,
+        dst: ApicId,
         dst_short: DstShorthand,
         trigger: TriggerMode,
         level: Level,
         dst_mode: DstMode,
         delivery_mode: DeliveryMode,
         vector: u8,
     ) {
-        let mut icr: u64 = (dst as u64) << 32;
+        let mut icr: u64 = (dst.raw as u64) << 32;
         icr |= (dst_short as u64) << 18;
         icr |= (trigger as u64) << 15;
         icr |= (level as u64) << 14;

mythril/src/ioapic.rs

@@ -14,6 +14,7 @@
 
 use crate::acpi::madt::{Ics, MADT};
 use crate::error::{Error, Result};
+use crate::lock::ro_after_init::RoAfterInit;
 use core::convert::TryFrom;
 use core::fmt;
 use core::ptr;
@@ -40,16 +41,14 @@ mod reg {
 
 const MAX_IOAPIC_COUNT: usize = 16;
 
-// Interactions with individual IoApics are thread-safe, and this
-// vector is only initialized by the BSP, so no mutex is needed.
-// TODO(alschwalm): Remove the Option once ArrayVec::new is a const fn
-static mut IOAPICS: Option<ArrayVec<[IoApic; MAX_IOAPIC_COUNT]>> = None;
+static IOAPICS: RoAfterInit<ArrayVec<[IoApic; MAX_IOAPIC_COUNT]>> =
+    RoAfterInit::uninitialized();
 
 // Get the IoApic and redirection table entry index corresponding to a given GSI.
 // Returns None if there is no such IoApic
 // TODO(alschwalm): Support InterruptSourceOverride
-fn ioapic_for_gsi(gsi: u32) -> Option<(&'static mut IoApic, u8)> {
-    for ioapic in unsafe { IOAPICS.as_mut().unwrap() }.iter_mut() {
+fn ioapic_for_gsi(gsi: u32) -> Option<(&'static IoApic, u8)> {
+    for ioapic in IOAPICS.iter() {
         let entries = ioapic.max_redirection_entry() as u32;
         let base = ioapic.gsi_base;
         if gsi > base && gsi < base + entries {
@@ -102,7 +101,7 @@ pub unsafe fn init_ioapics(madt: &MADT) -> Result<()> {
     }) {
         ioapics.push(ioapic);
     }
-    IOAPICS = Some(ioapics);
+    RoAfterInit::init(&IOAPICS, ioapics);
     Ok(())
 }
 
@@ -115,6 +114,11 @@ pub struct IoApic {
     pub gsi_base: u32,
 }
 
+// IoApics are actually Send/Sync. This will not be correctly derived
+// because raw pointers are not send (even when protected by a mutex).
+unsafe impl Send for IoApic {}
+unsafe impl Sync for IoApic {}
+
 impl IoApic {
     /// Create a new raw IoApic structure from the given
     /// base address and global system interrupt base.

mythril/src/kmain.rs

@@ -29,7 +29,7 @@ extern "C" {
 
 // Temporary helper function to create a vm for a single core
 fn default_vm(
-    core: u32,
+    core: percore::CoreId,
     mem: u64,
     info: &BootInfo,
     add_uart: bool,
@@ -132,7 +132,8 @@ fn default_vm(
     .unwrap();
     device_map.register_device(fw_cfg_builder.build()).unwrap();
 
-    vm::VirtualMachine::new(core, config, info).expect("Failed to create vm")
+    vm::VirtualMachine::new(core.raw, config, info)
+        .expect("Failed to create vm")
 }
 
 #[no_mangle]
@@ -201,7 +202,7 @@ unsafe fn kmain(mut boot_info: BootInfo) -> ! {
             // TODO(dlrobertson): Check the flags to ensure we can acutally
             // use this APIC.
             Ok(acpi::madt::Ics::LocalApic { apic_id, .. }) => {
-                Some(apic_id as u32)
+                Some(apic::ApicId::from(apic_id as u32))
             }
             _ => None,
         })
@@ -219,10 +220,10 @@ unsafe fn kmain(mut boot_info: BootInfo) -> ! {
     for apic_id in apic_ids.iter() {
         builder
             .insert_machine(default_vm(
-                *apic_id,
+                percore::CoreId::from(apic_id.raw),
                 256,
                 &boot_info,
-                *apic_id == 0,
+                apic_id.is_bsp(),
             ))
             .expect("Failed to insert new vm");
     }
@@ -232,7 +233,7 @@ unsafe fn kmain(mut boot_info: BootInfo) -> ! {
     debug!("AP_STARTUP address: 0x{:x}", AP_STARTUP_ADDR);
 
     for (idx, apic_id) in apic_ids.into_iter().enumerate() {
-        if apic_id == local_apic.id() as u32 {
+        if apic_id == local_apic.id() {
             continue;
         }
 

mythril/src/lib.rs

@@ -36,6 +36,7 @@ pub mod interrupt;
 pub mod ioapic;
 pub mod kmain;
 pub mod linux;
+pub mod lock;
 pub mod logger;
 pub mod memory;
 pub mod multiboot2;

mythril/src/lock/mod.rs

@@ -0,0 +1 @@
+pub mod ro_after_init;

mythril/src/lock/ro_after_init.rs

@@ -0,0 +1,37 @@
+use core::{cell::UnsafeCell, ops::Deref};
+
+pub struct RoAfterInit<T> {
+    data: UnsafeCell<Option<T>>,
+}
+
+impl<T> RoAfterInit<T> {
+    pub const fn uninitialized() -> Self {
+        RoAfterInit {
+            data: UnsafeCell::new(None),
+        }
+    }
+
+    pub unsafe fn init(this: &Self, val: T) {
+        *this.data.get() = Some(val);
+    }
+
+    pub fn is_initialized(this: &Self) -> bool {
+        unsafe { &*this.data.get() }.is_some()
+    }
+}
+
+// BspOnce is Send/Sync because the contents are immutable after init
+unsafe impl<T: Send> Send for RoAfterInit<T> {}
+unsafe impl<T: Send + Sync> Sync for RoAfterInit<T> {}
+
+impl<T> Deref for RoAfterInit<T> {
+    type Target = T;
+
+    fn deref(&self) -> &T {
+        unsafe {
+            (*self.data.get())
+                .as_ref()
+                .expect("Attempt to use BspOnce before init")
+        }
+    }
+}

mythril/src/percore.rs

@@ -8,9 +8,12 @@
 //! invocation of `init_sections` by the BSP.
 
 use crate::error::Result;
+use crate::lock::ro_after_init::RoAfterInit;
 use alloc::vec::Vec;
+use core::fmt;
 
-static mut AP_PER_CORE_SECTIONS: Option<Vec<u8>> = None;
+static AP_PER_CORE_SECTIONS: RoAfterInit<Vec<u8>> =
+    RoAfterInit::uninitialized();
 
 extern "C" {
     // The _value_ of the first/last byte of the .per_core section. The
@@ -31,11 +34,9 @@ unsafe fn per_core_address(symbol_addr: *const u8, core: usize) -> *const u8 {
     }
     let section_len = per_core_section_len();
     let offset = symbol_addr as u64 - (&PER_CORE_START as *const _ as u64);
-    let ap_sections = AP_PER_CORE_SECTIONS
-        .as_ref()
-        .expect("Per-core sections not initialized");
 
-    &ap_sections[(section_len * (core - 1)) + offset as usize] as *const u8
+    &AP_PER_CORE_SECTIONS[(section_len * (core - 1)) + offset as usize]
+        as *const u8
 }
 
 /// Initialize the per-core sections
@@ -53,35 +54,53 @@ pub unsafe fn init_sections(ncores: usize) -> Result<()> {
         ap_sections.extend_from_slice(per_core_section);
     }
 
-    AP_PER_CORE_SECTIONS = Some(ap_sections);
-
+    RoAfterInit::init(&AP_PER_CORE_SECTIONS, ap_sections);
     Ok(())
 }
 
+/// The sequential index of a core
+#[derive(Copy, Clone, Debug, Ord, PartialEq, PartialOrd, Eq)]
+pub struct CoreId {
+    /// The raw ID as an integer
+    pub raw: u32,
+}
+
+impl From<u32> for CoreId {
+    fn from(value: u32) -> Self {
+        CoreId { raw: value }
+    }
+}
+
+impl fmt::Display for CoreId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "0x{:x}", self.raw)
+    }
+}
+
 /// Get this current core's sequential index
-pub fn read_core_idx() -> u64 {
+pub fn read_core_id() -> CoreId {
     unsafe {
         let value: u64;
         llvm_asm!("mov [%fs], %rax"
                   : "={rax}"(value)
                   ::: "volatile");
-        value >> 3 // Shift away the RPL and TI bits (they will always be 0)
+        ((value >> 3) as u32).into() // Shift away the RPL and TI bits (they will always be 0)
     }
 }
 
 #[doc(hidden)]
 pub unsafe fn get_pre_core_impl<T>(t: &T) -> &T {
     core::mem::transmute(per_core_address(
         t as *const T as *const u8,
-        read_core_idx() as usize,
+        read_core_id().raw as usize,
     ))
 }
 
 #[doc(hidden)]
 pub unsafe fn get_pre_core_mut_impl<T>(t: &mut T) -> &mut T {
     core::mem::transmute(per_core_address(
         t as *const T as *const u8,
-        read_core_idx() as usize,
+        read_core_id().raw as usize,
     ))
 }
 

mythril/src/physdev/com.rs

@@ -51,7 +51,6 @@ impl Uart8250 {
     pub fn new(base: u16) -> Result<Self> {
         let mut uart = Self { base };
         uart.write_ier(IerFlags::RECV_DATA_AVAIL_INTERRUPT);
-        info!("{:?}", uart.read_ier());
         Ok(uart)
     }