From 9b585e6c9eaf42906bf583a289a59b15a722e6b7 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 31 Dec 2024 10:06:51 -0800 Subject: [PATCH 1/6] feat(util): add `Deferred` (#499) This commit adds a `Deferred` utility to `mycelium-util` for making drop guards. I wanted to use this to add a quick "`cli` at the top of a scope and `sti` when exiting the scope" utility in the `hal-x86_64` interrupt module. --- util/src/deferred.rs | 51 ++++++++++++++++++++++++++++++++++++++++++++ util/src/lib.rs | 2 ++ 2 files changed, 53 insertions(+) create mode 100644 util/src/deferred.rs diff --git a/util/src/deferred.rs b/util/src/deferred.rs new file mode 100644 index 00000000..b2b689a3 --- /dev/null +++ b/util/src/deferred.rs @@ -0,0 +1,51 @@ +//! Deferred closure execution (a.k.a. scope guards). +//! +//! See the [`Deferred`] type and the [`defer`] function for more information. + +/// Defers execution of a closure until a scope is exited. +/// +/// As seen in "The Go Programming Language".. +pub struct Deferred(Option); + +impl Deferred { + /// Defer execution of `f` until this scope is exited. + #[inline] + pub const fn new(f: F) -> Self { + Self(Some(f)) + } + + /// Cancel the deferred execution.of the closure passed to `defer`. + /// + /// Calling this method will prevent the closure from being executed when + /// this `Deferred` guard is dropped. + #[inline] + pub fn cancel(&mut self) { + self.0 = None; + } +} + +impl Drop for Deferred { + #[inline] + fn drop(&mut self) { + if let Some(f) = self.0.take() { + f() + } + } +} + +impl core::fmt::Debug for Deferred { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_tuple("Deferred") + .field(&format_args!("{}", core::any::type_name::())) + .finish() + } +} + +/// Defer execution of `f` until this scope is exited. +/// +/// This is equivalent to calling `Deferred::new(f)`. See [`Deferred`] for more +/// details. +#[inline(always)] +pub fn defer(f: F) -> Deferred { + Deferred::new(f) +} diff --git a/util/src/lib.rs b/util/src/lib.rs index 24b61dee..f30ca142 100644 --- a/util/src/lib.rs +++ b/util/src/lib.rs @@ -9,6 +9,8 @@ extern crate alloc; #[macro_use] mod macros; +pub use deferred::defer; +pub mod deferred; pub mod error; pub mod fmt; pub mod io; From ec43b14ebaebb2b6064d3f177ea96dd06cdb18b1 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 31 Dec 2024 10:34:44 -0800 Subject: [PATCH 2/6] 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 { From c98232203c5db21c99810d6a6483f54000f54153 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 31 Dec 2024 10:40:11 -0800 Subject: [PATCH 3/6] fix(hal-x86_64): don't log every MSR read (#499) Currently, we have some code for reading MSRs that emits a tracing event for every `rdmsr` instruction you execute. This was a stupid thing to do, and I only did it because I was being excessively cutesy. Now that we read the `FS_GS_BASE` MSR in ISRs to access local data, this can deadlock the kernel :upside_down: So, this commit removes it. --- hal-x86_64/src/cpu/msr.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hal-x86_64/src/cpu/msr.rs b/hal-x86_64/src/cpu/msr.rs index 69547ed5..abfb8126 100644 --- a/hal-x86_64/src/cpu/msr.rs +++ b/hal-x86_64/src/cpu/msr.rs @@ -281,9 +281,7 @@ impl Msr { options(nomem, nostack, preserves_flags) ); } - let result = (hi as u64) << 32 | (lo as u64); - tracing::trace!(rdmsr = %self, value = fmt::hex(result)); - result + (hi as u64) << 32 | (lo as u64) } /// Writes the given raw `u64` value to this MSR. From 36f24ab7f5ece83d3c17731aa9c2540708fc00d8 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 31 Dec 2024 11:06:38 -0800 Subject: [PATCH 4/6] fix(hal-x86_64): fix wrong LAPIC frequency calc (#499) 1 Hz = 1 second. 1 ms = 1/1000 second. So we should multiply here instead of dividing, lol. The divide accidentally makes us consider the LAPIC frequency to be 0, and then we just "don't ever have a timer". Whoopsie. --- hal-x86_64/src/interrupt/apic/local.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hal-x86_64/src/interrupt/apic/local.rs b/hal-x86_64/src/interrupt/apic/local.rs index 516e35df..386340cf 100644 --- a/hal-x86_64/src/interrupt/apic/local.rs +++ b/hal-x86_64/src/interrupt/apic/local.rs @@ -132,15 +132,15 @@ impl LocalApic { let cpuid = CpuId::new(); - if let Some(undivided_freq_khz) = cpuid.get_hypervisor_info().and_then(|hypervisor| { + if let Some(freq_khz) = cpuid.get_hypervisor_info().and_then(|hypervisor| { tracing::trace!("CPUID contains hypervisor info"); let freq = hypervisor.apic_frequency(); - tracing::trace!(hypervisor.apic_frequency = ?freq); + tracing::trace!(hypervisor.apic_frequency_khz = ?freq); NonZeroU32::new(freq?) }) { // the hypervisor info CPUID leaf expresses the frequency in kHz, // and the frequency is not divided by the target timer divisor. - let frequency_hz = undivided_freq_khz.get() / 1000 / Self::TIMER_DIVISOR; + let frequency_hz = (freq_khz.get() * 1000) / Self::TIMER_DIVISOR; tracing::debug!( frequency_hz, "determined APIC frequency from CPUID hypervisor info" @@ -148,6 +148,8 @@ impl LocalApic { return frequency_hz; } + // XXX ELIZA THIS IS TSC FREQUENCY, SO IDK IF THAT'S RIGHT? + /* if let Some(undivided_freq_hz) = cpuid.get_tsc_info().and_then(|tsc| { tracing::trace!("CPUID contains TSC info"); let freq = tsc.nominal_frequency(); @@ -161,6 +163,7 @@ impl LocalApic { ); return frequency_hz; } + */ // CPUID didn't help, so fall back to calibrating the APIC frequency // using the PIT. From 01c40f06e69ee076de2a560e1db360a3824ed567 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 31 Dec 2024 11:57:46 -0800 Subject: [PATCH 5/6] feat(hal-x86_64): crosscheck PIT/CPUID LAPIC freq (#499) This commit changes the local APIC calibration code to attempt PIT calibration even when we find CPUID leaves indicating the frequency. This way, we can cross-check them and see how sloppy the PIT calibration is. It turns out its pretty bad. --- hal-x86_64/src/interrupt/apic/local.rs | 134 +++++++++++++++++-------- 1 file changed, 91 insertions(+), 43 deletions(-) diff --git a/hal-x86_64/src/interrupt/apic/local.rs b/hal-x86_64/src/interrupt/apic/local.rs index 386340cf..f16733b8 100644 --- a/hal-x86_64/src/interrupt/apic/local.rs +++ b/hal-x86_64/src/interrupt/apic/local.rs @@ -127,11 +127,64 @@ impl LocalApic { tracing::info!(base = ?self.base, spurious_vector, "local APIC enabled"); } - fn timer_frequency_hz(&self) -> u32 { + /// Calibrate the timer frequency using the PIT. + #[inline(always)] + fn calibrate_frequency_hz_pit(&self) -> u32 { use register::*; + tracing::debug!("calibrating APIC timer frequency using PIT..."); - let cpuid = CpuId::new(); + // 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 { + // start the timer + + // set timer divisor to 16 + self.write_register(TIMER_DIVISOR, 0b11); + self.write_register( + LVT_TIMER, + LvtTimer::new() + .with(LvtTimer::MODE, TimerMode::OneShot) + .with(LvtTimer::MASKED, false), + ); + // set initial count to -1 + self.write_register(TIMER_INITIAL_COUNT, -1i32 as u32); + } + + // use the PIT to sleep for 10ms + pit.sleep_blocking(Duration::from_millis(10)) + .expect("the PIT should be able to send a 10ms interrupt..."); + + unsafe { + // stop the timer + self.write_register( + LVT_TIMER, + LvtTimer::new() + .with(LvtTimer::MODE, TimerMode::OneShot) + .with(LvtTimer::MASKED, true), + ); + } + + let elapsed_ticks = unsafe { self.register(TIMER_CURRENT_COUNT).read() }; + // since we slept for ten milliseconds, each tick is 10 kHz. we don't + // need to account for the divisor since we ran the timer at that + // divisor already. + let ticks_per_10ms = (-1i32 as u32).wrapping_sub(elapsed_ticks); + tracing::debug!(?ticks_per_10ms); + // convert the frequency to Hz. + let frequency_hz = ticks_per_10ms * 100; + tracing::debug!(frequency_hz, "calibrated local APIC timer using PIT"); + frequency_hz + } + #[inline(always)] + fn calibrate_frequency_hz_cpuid() -> Option { + let cpuid = CpuId::new(); if let Some(freq_khz) = cpuid.get_hypervisor_info().and_then(|hypervisor| { tracing::trace!("CPUID contains hypervisor info"); let freq = hypervisor.apic_frequency(); @@ -143,9 +196,9 @@ impl LocalApic { let frequency_hz = (freq_khz.get() * 1000) / Self::TIMER_DIVISOR; tracing::debug!( frequency_hz, - "determined APIC frequency from CPUID hypervisor info" + "determined APIC timer frequency from CPUID hypervisor info" ); - return frequency_hz; + return Some(frequency_hz); } // XXX ELIZA THIS IS TSC FREQUENCY, SO IDK IF THAT'S RIGHT? @@ -167,51 +220,46 @@ 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); - // set initial count to -1 - self.write_register(TIMER_INITIAL_COUNT, -1i32 as u32); + None + } - // start the timer - self.write_register( - LVT_TIMER, - LvtTimer::new().with(LvtTimer::MODE, TimerMode::OneShot), + fn timer_frequency_hz(&self) -> u32 { + // How sloppy do we expect the PIT frequency calibration to be? + // If the delta between the CPUID frequency and the frequency we + // determined using PIT calibration is > this value, we'll yell about + // it. + const PIT_SLOPPINESS: u32 = 1000; + + // Start out by calibrating the APIC frequency using the PIT. + let pit_frequency_hz = self.calibrate_frequency_hz_pit(); + + // See if we can get something from CPUID. + let Some(cpuid_frequency_hz) = Self::calibrate_frequency_hz_cpuid() else { + tracing::info!( + pit.frequency_hz = pit_frequency_hz, + "CPUID does not indicate APIC timer frequency; using PIT \ + calibration only" ); - } - - // use the PIT to sleep for 10ms - pit.sleep_blocking(Duration::from_millis(10)) - .expect("the PIT should be able to send a 10ms interrupt..."); + return pit_frequency_hz; + }; - unsafe { - // stop the timer - self.write_register( - LVT_TIMER, - LvtTimer::new().with(LvtTimer::MODE, TimerMode::Periodic), + // Cross-check the PIT calibration result and CPUID value. + let distance = if pit_frequency_hz > cpuid_frequency_hz { + pit_frequency_hz - cpuid_frequency_hz + } else { + cpuid_frequency_hz - pit_frequency_hz + }; + if distance > PIT_SLOPPINESS { + tracing::warn!( + pit.frequency_hz = pit_frequency_hz, + cpuid.frequency_hz = cpuid_frequency_hz, + distance, + "APIC timer frequency from PIT calibration differs substantially \ + from CPUID!" ); } - let elapsed_ticks = unsafe { self.register(TIMER_CURRENT_COUNT).read() }; - // since we slept for ten milliseconds, each tick is 10 kHz. we don't - // need to account for the divisor since we ran the timer at that - // divisor already. - let ticks_per_10ms = (-1i32 as u32).wrapping_sub(elapsed_ticks); - tracing::debug!(?ticks_per_10ms); - // convert the frequency to Hz. - let frequency_hz = ticks_per_10ms * 100; - tracing::debug!(frequency_hz, "calibrated local APIC timer using PIT"); - frequency_hz + cpuid_frequency_hz } #[tracing::instrument( From 8d51ac7457cfc75efc0fecee43eb2c9fe70256c5 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 31 Dec 2024 11:59:30 -0800 Subject: [PATCH 6/6] fix(hal-x86_64): remove tracing from PIT sleep (#499) The current code for sleeping using the PIT one-shot timer has a bunch of tracing in it. This adds overhead which makes the sleep duration way less accurate, making PIT calibration for the local APIC timer way sloppier. This commit removes the tracing. PIT calibration accuracy still sucks, but it sucks less now. --- hal-x86_64/src/time/pit.rs | 26 +++----------------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/hal-x86_64/src/time/pit.rs b/hal-x86_64/src/time/pit.rs index df2e5731..6ecaf3a9 100644 --- a/hal-x86_64/src/time/pit.rs +++ b/hal-x86_64/src/time/pit.rs @@ -271,19 +271,14 @@ impl Pit { /// /// [`Controller::init`]: crate::interrupt::Controller::init /// [`interrupt`]: crate::interrupt - #[tracing::instrument( - name = "Pit::sleep_blocking" - level = tracing::Level::DEBUG, - skip(self), - fields(?duration), - err, - )] pub fn sleep_blocking(&mut self, duration: Duration) -> Result<(), PitError> { SLEEPING .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) .map_err(|_| PitError::SleepInProgress)?; self.interrupt_in(duration) .map_err(PitError::InvalidDuration)?; + + // Tracing here is fine, because we are already sleeping... tracing::debug!("started PIT sleep"); // spin until the sleep interrupt fires. @@ -291,11 +286,8 @@ impl Pit { cpu::wait_for_interrupt(); } - tracing::info!(?duration, "slept using PIT channel 0"); - // if we were previously in periodic mode, re-enable it. if let Some(interval) = self.channel0_interval { - tracing::debug!("restarting PIT periodic timer"); self.start_periodic_timer(interval)?; } @@ -366,13 +358,6 @@ impl Pit { /// This configures the PIT in mode 0 (oneshot mode). Once the interrupt has /// fired, in order to use the periodic timer, the pit must be put back into /// periodic mode by calling [`Pit::start_periodic_timer`]. - #[tracing::instrument( - name = "Pit::interrupt_in" - level = tracing::Level::DEBUG, - skip(self), - fields(?duration), - err, - )] pub fn interrupt_in(&mut self, duration: Duration) -> Result<(), InvalidDuration> { let duration_ms = usize::try_from(duration.as_millis()).map_err(|_| { InvalidDuration::new( @@ -388,8 +373,6 @@ impl Pit { ) })?; - tracing::trace!(?duration, duration_ms, target_time, "Pit::interrupt_in"); - let command = Command::new() // use the binary counter .with(Command::BCD_BINARY, false) @@ -410,19 +393,16 @@ impl Pit { } fn set_divisor(&mut self, divisor: u16) { - tracing::trace!(divisor = &fmt::hex(divisor), "Pit::set_divisor"); let low = divisor as u8; let high = (divisor >> 8) as u8; unsafe { self.channel0.writeb(low); // write the low byte - tracing::trace!(lo = &fmt::hex(low), "pit.channel0.writeb(lo)"); self.channel0.writeb(high); // write the high byte - tracing::trace!(hi = &fmt::hex(high), "pit.channel0.writeb(hi)"); } } fn send_command(&self, command: Command) { - tracing::debug!(?command, "Pit::send_command"); + // tracing::debug!(?command, "Pit::send_command"); unsafe { self.command.writeb(command.bits()); }