improve maitake timer wheel

hal-x86_64/Cargo.toml

@@ -13,6 +13,7 @@ alloc = []
 [dependencies]
 acpi = "4.1.1"
 hal-core = { path = "../hal-core" }
+maitake = { path = "../maitake" }
 mycelium-util = { path = "../util" }
 mycelium-trace = { path = "../trace" }
 mycotest = { path = "../mycotest"}

hal-x86_64/src/interrupt.rs

@@ -1,4 +1,4 @@
-use crate::{cpu, mm, segment, PAddr, VAddr};
+use crate::{cpu, mm, segment, time, PAddr, VAddr};
 use core::{arch::asm, marker::PhantomData, time::Duration};
 use hal_core::interrupt::Control;
 use hal_core::interrupt::{ctx, Handlers};
@@ -37,6 +37,12 @@ pub struct CodeFault<'a> {
 /// An interrupt service routine.
 pub type Isr<T> = extern "x86-interrupt" fn(&mut Context<T>);
 
+#[derive(Debug)]
+pub enum PeriodicTimerError {
+    Pit(time::PitError),
+    Apic(time::InvalidDuration),
+}
+
 #[derive(Debug)]
 #[repr(C)]
 pub struct Interrupt<T = ()> {
@@ -229,6 +235,7 @@ impl Controller {
         // `sti` may not be called until the interrupt controller static is
         // fully initialized, as an interrupt that occurs before it is
         // initialized may attempt to access the static to finish the interrupt!
+        core::sync::atomic::fence(core::sync::atomic::Ordering::SeqCst);
         unsafe {
             crate::cpu::intrinsics::sti();
         }
@@ -238,15 +245,15 @@ impl Controller {
 
     /// Starts a periodic timer which fires the `timer_tick` interrupt of the
     /// provided [`Handlers`] every time `interval` elapses.
-    pub fn start_periodic_timer(
-        &self,
-        interval: Duration,
-    ) -> Result<(), crate::time::InvalidDuration> {
+    pub fn start_periodic_timer(&self, interval: Duration) -> Result<(), PeriodicTimerError> {
         match self.model {
-            InterruptModel::Pic(_) => crate::time::PIT.lock().start_periodic_timer(interval),
-            InterruptModel::Apic { ref local, .. } => {
-                local.start_periodic_timer(interval, Idt::LOCAL_APIC_TIMER as u8)
-            }
+            InterruptModel::Pic(_) => crate::time::PIT
+                .lock()
+                .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),
         }
     }
 }
@@ -388,14 +395,8 @@ impl hal_core::interrupt::Control for Idt {
         }
 
         extern "x86-interrupt" fn pit_timer_isr<H: Handlers<Registers>>(_regs: Registers) {
-            use core::sync::atomic::Ordering;
-            // if we weren't trying to do a PIT sleep, handle the timer tick
-            // instead.
-            let was_sleeping = crate::time::pit::SLEEPING
-                .compare_exchange(true, false, Ordering::AcqRel, Ordering::Acquire)
-                .is_ok();
-            if !was_sleeping {
-                H::timer_tick();
+            if crate::time::Pit::handle_interrupt() {
+                H::timer_tick()
             } else {
                 tracing::trace!("PIT sleep completed");
             }

hal-x86_64/src/interrupt/apic/local.rs

@@ -152,6 +152,15 @@ impl LocalApic {
         // CPUID didn't help, so fall back to calibrating the APIC frequency
         // using the PIT.
         tracing::debug!("calibrating APIC timer frequency using PIT...");
+
+        // lock the PIT now, before actually starting the timer IRQ, so that we
+        // don't include any time spent waiting for the PIT lock.
+        //
+        // since we only run this code on startup, before any other cores have
+        // been started, this probably never actually waits for a lock. but...we
+        // should do it the right way, anyway.
+        let mut pit = crate::time::PIT.lock();
+
         unsafe {
             // set timer divisor to 16
             self.write_register(TIMER_DIVISOR, 0b11);
@@ -166,9 +175,7 @@ impl LocalApic {
         }
 
         // use the PIT to sleep for 10ms
-        crate::time::PIT
-            .lock()
-            .sleep_blocking(Duration::from_millis(10))
+        pit.sleep_blocking(Duration::from_millis(10))
             .expect("the PIT should be able to send a 10ms interrupt...");
 
         unsafe {

hal-x86_64/src/time.rs

@@ -2,7 +2,7 @@
 pub(crate) mod pit;
 mod tsc;
 pub use self::{
-    pit::{Pit, PIT},
+    pit::{Pit, PitError, PIT},
     tsc::Rdtsc,
 };
 use core::fmt;

hal-x86_64/src/time/pit.rs

@@ -86,6 +86,17 @@ pub struct Pit {
     channel0_interval: Option<Duration>,
 }
 
+/// Errors returned by [`Pit::start_periodic_timer`] and [`Pit::sleep_blocking`].
+#[derive(Debug)]
+pub enum PitError {
+    /// The periodic timer is already running.
+    AlreadyRunning,
+    /// A [`Pit::sleep_blocking`] call is in progress.
+    SleepInProgress,
+    /// The provided duration was invalid.
+    InvalidDuration(InvalidDuration),
+}
+
 bitfield! {
     struct Command<u8> {
         /// BCD/binary mode.
@@ -202,7 +213,7 @@ enum_from_bits! {
 pub static PIT: Mutex<Pit> = Mutex::new(Pit::new());
 
 /// Are we currently sleeping on an interrupt?
-pub(crate) static SLEEPING: AtomicBool = AtomicBool::new(false);
+static SLEEPING: AtomicBool = AtomicBool::new(false);
 
 impl Pit {
     /// The PIT's base frequency runs at roughly 1.193182 MHz, for [extremely
@@ -267,11 +278,12 @@ impl Pit {
         fields(?duration),
         err,
     )]
-    pub fn sleep_blocking(&mut self, duration: Duration) -> Result<(), InvalidDuration> {
+    pub fn sleep_blocking(&mut self, duration: Duration) -> Result<(), PitError> {
         SLEEPING
             .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire)
-            .expect("if another CPU core is sleeping, it should be holding the lock on the PIT, preventing us from starting a sleep!");
-        self.interrupt_in(duration)?;
+            .map_err(|_| PitError::SleepInProgress)?;
+        self.interrupt_in(duration)
+            .map_err(PitError::InvalidDuration)?;
         tracing::debug!("started PIT sleep");
 
         // spin until the sleep interrupt fires.
@@ -306,21 +318,20 @@ impl Pit {
         fields(?interval),
         err,
     )]
-    pub fn start_periodic_timer(&mut self, interval: Duration) -> Result<(), InvalidDuration> {
-        debug_assert!(
-            !SLEEPING.load(Ordering::Acquire),
-            "tried to start a periodic timer while a sleep was in progress!"
-        );
+    pub fn start_periodic_timer(&mut self, interval: Duration) -> Result<(), PitError> {
+        if SLEEPING.load(Ordering::Acquire) {
+            return Err(PitError::SleepInProgress);
+        }
 
         let interval_ms = usize::try_from(interval.as_millis()).map_err(|_| {
-            InvalidDuration::new(
+            PitError::invalid_duration(
                 interval,
                 "PIT periodic timer interval as milliseconds would exceed a `usize`",
             )
         })?;
         let interval_ticks = Self::TICKS_PER_MS * interval_ms;
         let divisor = u16::try_from(interval_ticks).map_err(|_| {
-            InvalidDuration::new(interval, "PIT channel 0 divisor would exceed a `u16`")
+            PitError::invalid_duration(interval, "PIT channel 0 divisor would exceed a `u16`")
         })?;
 
         // store the periodic timer interval so we can reset later.
@@ -394,6 +405,10 @@ impl Pit {
         Ok(())
     }
 
+    pub(crate) fn handle_interrupt() -> bool {
+        SLEEPING.swap(false, Ordering::AcqRel)
+    }
+
     fn set_divisor(&mut self, divisor: u16) {
         tracing::trace!(divisor = &fmt::hex(divisor), "Pit::set_divisor");
         let low = divisor as u8;
@@ -413,3 +428,21 @@ impl Pit {
         }
     }
 }
+
+// === impl PitError ===
+
+impl PitError {
+    fn invalid_duration(duration: Duration, msg: &'static str) -> Self {
+        Self::InvalidDuration(InvalidDuration::new(duration, msg))
+    }
+}
+
+impl fmt::Display for PitError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::AlreadyRunning => write!(f, "PIT periodic timer is already running"),
+            Self::SleepInProgress => write!(f, "a PIT sleep is currently in progress"),
+            Self::InvalidDuration(e) => write!(f, "invalid PIT duration: {e}"),
+        }
+    }
+}

hal-x86_64/src/time/tsc.rs

@@ -1,4 +1,9 @@
 use crate::cpu::{intrinsics, FeatureNotSupported};
+use core::{
+    sync::atomic::{AtomicU32, Ordering::*},
+    time::Duration,
+};
+use maitake::time::Clock;
 use raw_cpuid::CpuId;
 
 #[derive(Copy, Clone, Debug)]
@@ -22,11 +27,74 @@ impl Rdtsc {
 
     /// Reads the current value of the timestamp counter.
     #[inline(always)]
+    #[must_use]
     pub fn read_timestamp(&self) -> u64 {
         unsafe {
             // safety: if an instance of this type was constructed, then we
             // checked that `rdtsc` is supported via `cpuid`.
             intrinsics::rdtsc()
         }
     }
+
+    /// Returns a [`maitake::time::Clock`] defining a clock that uses `rdtsc`
+    /// timestamps to produce `maitake` ticks.
+    pub fn into_maitake_clock(self) -> Result<Clock, &'static str> {
+        // TODO(eliza): fix this
+        #![allow(unreachable_code)]
+        return Err("calibration routine doesn't really work yet, sorry!");
+
+        const NOT_YET_CALIBRATED: u32 = u32::MAX;
+
+        // 50ms is stolen from Linux, so i'm assuming its a reasonable duration
+        // for PIT-based calibration:
+        // https://github.com/torvalds/linux/blob/4ae004a9bca8bef118c2b4e76ee31c7df4514f18/arch/x86/kernel/tsc.c#L688-L839
+        const PIT_SLEEP_DURATION: Duration = Duration::from_millis(50);
+
+        static MAITAKE_TICK_SHIFT: AtomicU32 = AtomicU32::new(NOT_YET_CALIBRATED);
+
+        fn now() -> u64 {
+            let rdtsc = unsafe { intrinsics::rdtsc() };
+            let shift = MAITAKE_TICK_SHIFT.load(Relaxed);
+            rdtsc >> shift
+        }
+
+        tracing::info!("calibrating RDTSC Maitake clock from PIT...");
+        let mut pit = super::PIT.lock();
+
+        let t0 = self.read_timestamp();
+        pit.sleep_blocking(PIT_SLEEP_DURATION)
+            .expect("PIT sleep failed for some reason");
+        let t1 = self.read_timestamp();
+
+        let elapsed_cycles = t1 - t0;
+        tracing::debug!(t0, t1, elapsed_cycles, "slept for {PIT_SLEEP_DURATION:?}");
+
+        let mut shift = 0;
+        loop {
+            assert!(
+                shift < 64,
+                "shifted away all the precision in the timestamp counter! \
+                this definitely should never happen..."
+            );
+            let elapsed_ticks = elapsed_cycles >> shift;
+            tracing::debug!(shift, elapsed_ticks, "trying a new tick shift");
+
+            let elapsed_ticks: u32 = elapsed_ticks.try_into().expect(
+                "there is no way that a 50ms sleep duration is more than \
+                    u32::MAX rdtsc cycles...right?",
+            );
+
+            let tick_duration = PIT_SLEEP_DURATION / elapsed_ticks;
+            if tick_duration.as_nanos() > 0 {
+                // Ladies and gentlemen...we got him!
+                tracing::info!(?tick_duration, shift, "calibrated RDTSC Maitake clock");
+                MAITAKE_TICK_SHIFT
+                    .compare_exchange(NOT_YET_CALIBRATED, shift, AcqRel, Acquire)
+                    .map_err(|_| "RDTSC Maitake clock has already been calibrated")?;
+                return Ok(Clock::new(tick_duration, now).named("CLOCK_RDTSC"));
+            } else {
+                shift += 1;
+            }
+        }
+    }
 }

maitake/src/loom.rs

@@ -4,6 +4,7 @@ pub(crate) use self::inner::*;
 #[cfg(loom)]
 mod inner {
     #![allow(dead_code)]
+    pub(crate) use loom::thread_local;
 
     #[cfg(feature = "alloc")]
     pub(crate) mod alloc {
@@ -101,6 +102,10 @@ mod inner {
 #[cfg(not(loom))]
 mod inner {
     #![allow(dead_code, unused_imports)]
+
+    #[cfg(test)]
+    pub(crate) use std::thread_local;
+
     pub(crate) mod sync {
         #[cfg(feature = "alloc")]
         pub use alloc::sync::*;

maitake/src/time.rs

@@ -43,14 +43,18 @@
 //! [wheel]: http://www.cs.columbia.edu/~nahum/w6998/papers/sosp87-timing-wheels.pdf
 //! [driving-timers]: Timer#driving-timers
 #![warn(missing_docs, missing_debug_implementations)]
+pub mod clock;
 pub mod timeout;
 mod timer;
 
 use crate::util;
 
 #[doc(inline)]
-pub use self::timeout::Timeout;
-pub use self::timer::{set_global_timer, sleep::Sleep, AlreadyInitialized, Timer, TimerError, Turn};
+pub use self::{
+    clock::{Clock, Instant},
+    timeout::Timeout,
+    timer::{set_global_timer, sleep::Sleep, AlreadyInitialized, Timer, TimerError, Turn},
+};
 pub use core::time::Duration;
 
 use core::future::Future;