From 555862ad4444ab54eac65ab3f17e77ba784cd6e8 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 31 Dec 2024 10:34:44 -0800 Subject: [PATCH] feat(hal-x86_64): put local APIC in local storage (#499) As described in #495, each CPU core in a SMP x86 system has its own local APIC. Currently, the mycelium HAL's interrupt code will just only ever know about the boot processor's local APIC. This is wrong. This commit reworks this to use the `%GS` local storage mechanism to store the state of the local APIC separately for each CPU core. I've added code for initializing the local APIC state on non-boot processors, which we will call when we bring up other cores. Fixes #495 --- hal-x86_64/src/interrupt.rs | 77 ++++++++++++++++++++++---- hal-x86_64/src/interrupt/apic/local.rs | 55 +++++++++++++++++- util/src/deferred.rs | 4 +- 3 files changed, 123 insertions(+), 13 deletions(-) diff --git a/hal-x86_64/src/interrupt.rs b/hal-x86_64/src/interrupt.rs index a4eaf012..43cfadf7 100644 --- a/hal-x86_64/src/interrupt.rs +++ b/hal-x86_64/src/interrupt.rs @@ -2,6 +2,7 @@ use crate::{cpu, mm, segment, time, VAddr}; use core::{arch::asm, marker::PhantomData, time::Duration}; use hal_core::interrupt::Control; use hal_core::interrupt::{ctx, Handlers}; +use hal_core::mem::page; use mycelium_util::{ bits, fmt, sync::{ @@ -44,7 +45,8 @@ pub type Isr = extern "x86-interrupt" fn(&mut Context); #[derive(Debug)] pub enum PeriodicTimerError { Pit(time::PitError), - Apic(time::InvalidDuration), + InvalidDuration(time::InvalidDuration), + Apic(apic::local::LocalApicError), } #[derive(Debug)] @@ -68,8 +70,8 @@ enum InterruptModel { /// [I/O]: apic::IoApic /// [apics]: apic Apic { - local: apic::LocalApic, io: apic::IoApicSet, + local: apic::local::Handle, }, } @@ -192,6 +194,16 @@ impl IsaInterrupt { ]; } +#[must_use] +fn disable_scoped() -> impl Drop + Send + Sync { + unsafe { + crate::cpu::intrinsics::cli(); + } + mycelium_util::defer(|| unsafe { + crate::cpu::intrinsics::sti(); + }) +} + impl Controller { // const DEFAULT_IOAPIC_BASE_PADDR: u64 = 0xFEC00000; @@ -224,6 +236,36 @@ impl Controller { } } + fn local_apic_handle(&self) -> Result<&apic::local::Handle, apic::local::LocalApicError> { + match self.model { + InterruptModel::Pic(_) => Err(apic::local::LocalApicError::NoApic), + InterruptModel::Apic { ref local, .. } => Ok(local), + } + } + + pub fn with_local_apic( + &self, + f: impl FnOnce(&LocalApic) -> T, + ) -> Result { + self.local_apic_handle()?.with(f) + } + + /// This should *not* be called by the boot processor + pub fn initialize_local_apic( + &self, + frame_alloc: &A, + pagectrl: &mut impl page::Map, + ) -> Result<(), apic::local::LocalApicError> + where + A: page::Alloc, + { + let _deferred = disable_scoped(); + let hdl = self.local_apic_handle()?; + unsafe { + hdl.initialize(frame_alloc, pagectrl, Idt::LOCAL_APIC_SPURIOUS as u8); + } + Ok(()) + } /// # Safety /// /// Calling this when there isn't actually an ISA interrupt pending can do @@ -231,13 +273,14 @@ impl Controller { pub unsafe fn end_isa_irq(&self, irq: IsaInterrupt) { match self.model { InterruptModel::Pic(ref pics) => pics.lock().end_interrupt(irq), - InterruptModel::Apic { ref local, .. } => local.end_interrupt(), + InterruptModel::Apic { ref local, .. } => local.with(|apic| unsafe { apic.end_interrupt() }) + .expect("interrupts should not be handled on this core until the local APIC is initialized") } } pub fn enable_hardware_interrupts( acpi: Option<&acpi::InterruptModel>, - frame_alloc: &impl hal_core::mem::page::Alloc, + frame_alloc: &impl page::Alloc, ) -> &'static Self { let mut pics = pic::CascadedPic::new(); // regardless of whether APIC or PIC interrupt handling will be used, @@ -267,13 +310,16 @@ impl Controller { // configure the I/O APIC(s) let io = IoApicSet::new(apic_info, frame_alloc, &mut pagectrl, Idt::ISA_BASE as u8); - // enable the local APIC - let local = LocalApic::new(&mut pagectrl, frame_alloc); - local.enable(Idt::LOCAL_APIC_SPURIOUS as u8); + // configure and initialize the local APIC on the boot processor + let local = apic::local::Handle::new(); + unsafe { + local.initialize(frame_alloc, &mut pagectrl, Idt::LOCAL_APIC_SPURIOUS as u8); + } let model = InterruptModel::Apic { local, io }; tracing::trace!(interrupt_model = ?model); + INTERRUPT_CONTROLLER.init(Self { model }) } model => { @@ -334,8 +380,11 @@ impl Controller { .start_periodic_timer(interval) .map_err(PeriodicTimerError::Pit), InterruptModel::Apic { ref local, .. } => local - .start_periodic_timer(interval, Idt::LOCAL_APIC_TIMER as u8) - .map_err(PeriodicTimerError::Apic), + .with(|apic| { + apic.start_periodic_timer(interval, Idt::LOCAL_APIC_TIMER as u8) + .map_err(PeriodicTimerError::InvalidDuration) + }) + .map_err(PeriodicTimerError::Apic)?, } } } @@ -650,7 +699,15 @@ mod isr { unsafe { match INTERRUPT_CONTROLLER.get_unchecked().model { InterruptModel::Pic(_) => unreachable!(), - InterruptModel::Apic { ref local, .. } => local.end_interrupt(), + InterruptModel::Apic { ref local, .. } => { + match local.with(|apic| apic.end_interrupt()) { + Ok(_) => {} + Err(e) => unreachable!( + "the local APIC timer will not have fired if the \ + local APIC is uninitialized on this core! {e:?}", + ), + } + } } } } diff --git a/hal-x86_64/src/interrupt/apic/local.rs b/hal-x86_64/src/interrupt/apic/local.rs index c5c3d18d..516e35df 100644 --- a/hal-x86_64/src/interrupt/apic/local.rs +++ b/hal-x86_64/src/interrupt/apic/local.rs @@ -1,10 +1,10 @@ use super::{PinPolarity, TriggerMode}; use crate::{ - cpu::{FeatureNotSupported, Msr}, + cpu::{local, FeatureNotSupported, Msr}, mm::{self, page, size::Size4Kb, PhysPage, VirtPage}, time::{Duration, InvalidDuration}, }; -use core::{convert::TryInto, marker::PhantomData, num::NonZeroU32}; +use core::{cell::RefCell, convert::TryInto, marker::PhantomData, num::NonZeroU32}; use hal_core::{PAddr, VAddr}; use mycelium_util::fmt; use raw_cpuid::CpuId; @@ -24,6 +24,9 @@ pub struct LocalApicRegister { _ty: PhantomData, } +#[derive(Debug)] +pub(in crate::interrupt) struct Handle(local::LocalKey>>); + pub trait RegisterAccess { type Access; type Target; @@ -32,6 +35,16 @@ pub trait RegisterAccess { ) -> Volatile<&'static mut Self::Target, Self::Access>; } +#[derive(Debug, Eq, PartialEq)] +pub enum LocalApicError { + /// The system is configured to use the PIC interrupt model rather than the + /// APIC interrupt model. + NoApic, + + /// The local APIC is uninitialized. + Uninitialized, +} + impl LocalApic { const BASE_PADDR_MASK: u64 = 0xffff_ffff_f000; @@ -293,6 +306,44 @@ impl LocalApic { } } +impl Handle { + pub(in crate::interrupt) const fn new() -> Self { + Self(local::LocalKey::new(|| RefCell::new(None))) + } + + pub(in crate::interrupt) fn with( + &self, + f: impl FnOnce(&LocalApic) -> T, + ) -> Result { + self.0.with(|apic| { + Ok(f(apic + .borrow() + .as_ref() + .ok_or(LocalApicError::Uninitialized)?)) + }) + } + + pub(in crate::interrupt) unsafe fn initialize( + &self, + frame_alloc: &A, + pagectrl: &mut impl page::Map, + spurious_vector: u8, + ) where + A: page::Alloc, + { + self.0.with(|slot| { + let mut slot = slot.borrow_mut(); + if slot.is_some() { + // already initialized, bail. + return; + } + let apic = LocalApic::new(pagectrl, frame_alloc); + apic.enable(spurious_vector); + *slot = Some(apic); + }) + } +} + pub mod register { use super::*; use mycelium_util::bits::{bitfield, enum_from_bits}; diff --git a/util/src/deferred.rs b/util/src/deferred.rs index b2b689a3..73bdf157 100644 --- a/util/src/deferred.rs +++ b/util/src/deferred.rs @@ -4,7 +4,9 @@ /// Defers execution of a closure until a scope is exited. /// -/// As seen in "The Go Programming Language".. +/// As seen in "The Go Programming Language". +#[must_use = "dropping a `Deferred` guard immediately executes \ + the deferred function"] pub struct Deferred(Option); impl Deferred {