Rewrite timer subsystem

Author: andre-richter

07_timestamps/README.md

@@ -52,6 +52,14 @@ _start:
 	ADR_REL	x0, __boot_core_stack_end_exclusive
 	mov	sp, x0
 
+	// Read the CPU's timer counter frequency and store it in ARCH_TIMER_COUNTER_FREQUENCY.
+	// Abort if the frequency read back as 0.
+	ADR_REL	x1, ARCH_TIMER_COUNTER_FREQUENCY // provided by aarch64/time.rs
+	mrs	x2, CNTFRQ_EL0
+	cmp	x2, xzr
+	b.eq	.L_parking_loop
+	str	w2, [x1]
+
 	// Jump to Rust code.
 	b	_start_rust
 

07_timestamps/src/_arch/aarch64/cpu/boot.s

@@ -11,111 +11,152 @@
 //!
 //! crate::time::arch_time
 
-use crate::{time, warn};
-use core::time::Duration;
+use crate::warn;
+use core::{
+    num::{NonZeroU128, NonZeroU32, NonZeroU64},
+    ops::{Add, Div},
+    time::Duration,
+};
 use cortex_a::{asm::barrier, registers::*};
-use tock_registers::interfaces::{ReadWriteable, Readable, Writeable};
+use tock_registers::interfaces::Readable;
 
 //--------------------------------------------------------------------------------------------------
 // Private Definitions
 //--------------------------------------------------------------------------------------------------
 
-const NS_PER_S: u64 = 1_000_000_000;
+const NANOSEC_PER_SEC: NonZeroU64 = NonZeroU64::new(1_000_000_000).unwrap();
 
-/// ARMv8 Generic Timer.
-struct GenericTimer;
+#[derive(Copy, Clone, PartialOrd, PartialEq)]
+struct GenericTimerCounterValue(u64);
 
 //--------------------------------------------------------------------------------------------------
 // Global instances
 //--------------------------------------------------------------------------------------------------
 
-static TIME_MANAGER: GenericTimer = GenericTimer;
+/// Boot assembly code overwrites this value with the value of CNTFRQ_EL0 before any Rust code is
+/// executed. This given value here is just a (safe) dummy.
+#[no_mangle]
+static ARCH_TIMER_COUNTER_FREQUENCY: NonZeroU32 = NonZeroU32::MIN;
 
 //--------------------------------------------------------------------------------------------------
 // Private Code
 //--------------------------------------------------------------------------------------------------
 
-impl GenericTimer {
-    #[inline(always)]
-    fn read_cntpct(&self) -> u64 {
-        // Prevent that the counter is read ahead of time due to out-of-order execution.
-        unsafe { barrier::isb(barrier::SY) };
-        CNTPCT_EL0.get()
-    }
+impl GenericTimerCounterValue {
+    pub const MAX: Self = GenericTimerCounterValue(u64::MAX);
 }
 
-//--------------------------------------------------------------------------------------------------
-// Public Code
-//--------------------------------------------------------------------------------------------------
+impl Add for GenericTimerCounterValue {
+    type Output = Self;
 
-/// Return a reference to the time manager.
-pub fn time_manager() -> &'static impl time::interface::TimeManager {
-    &TIME_MANAGER
+    fn add(self, other: Self) -> Self {
+        GenericTimerCounterValue(self.0.wrapping_add(other.0))
+    }
 }
 
-//------------------------------------------------------------------------------
-// OS Interface Code
-//------------------------------------------------------------------------------
+impl From<GenericTimerCounterValue> for Duration {
+    fn from(counter_value: GenericTimerCounterValue) -> Self {
+        if counter_value.0 == 0 {
+            return Duration::ZERO;
+        }
 
-impl time::interface::TimeManager for GenericTimer {
-    fn resolution(&self) -> Duration {
-        Duration::from_nanos(NS_PER_S / (CNTFRQ_EL0.get() as u64))
+        // Read volatile is needed here to prevent the compiler from optimizing
+        // ARCH_TIMER_COUNTER_FREQUENCY away.
+        //
+        // This is safe, because all the safety requirements as stated in read_volatile()'s
+        // documentation are fulfilled.
+        let frequency: NonZeroU64 =
+            unsafe { core::ptr::read_volatile(&ARCH_TIMER_COUNTER_FREQUENCY) }.into();
+
+        // Div<NonZeroU64> implementation for u64 cannot panic.
+        let secs = counter_value.0.div(frequency);
+
+        // This is safe, because frequency can never be greater than u32::MAX, which means the
+        // largest theoretical value for sub_second_counter_value is (u32::MAX - 1). Therefore,
+        // (sub_second_counter_value * NANOSEC_PER_SEC) cannot overflow an u64.
+        //
+        // The subsequent division ensures the result fits into u32, since the max result is smaller
+        // than NANOSEC_PER_SEC. Therefore, just cast it to u32 using `as`.
+        let sub_second_counter_value = counter_value.0 % frequency;
+        let nanos = unsafe { sub_second_counter_value.unchecked_mul(u64::from(NANOSEC_PER_SEC)) }
+            .div(frequency) as u32;
+
+        Duration::new(secs, nanos)
     }
+}
 
-    fn uptime(&self) -> Duration {
-        let current_count: u64 = self.read_cntpct() * NS_PER_S;
-        let frq: u64 = CNTFRQ_EL0.get() as u64;
+fn max_duration() -> Duration {
+    Duration::from(GenericTimerCounterValue::MAX)
+}
 
-        Duration::from_nanos(current_count / frq)
-    }
+impl TryFrom<Duration> for GenericTimerCounterValue {
+    type Error = &'static str;
 
-    fn spin_for(&self, duration: Duration) {
-        // Instantly return on zero.
-        if duration.as_nanos() == 0 {
-            return;
+    fn try_from(duration: Duration) -> Result<Self, Self::Error> {
+        if duration < resolution() {
+            return Ok(GenericTimerCounterValue(0));
         }
 
-        // Calculate the register compare value.
-        let frq = CNTFRQ_EL0.get();
-        let x = match frq.checked_mul(duration.as_nanos() as u64) {
-            #[allow(unused_imports)]
-            None => {
-                warn!("Spin duration too long, skipping");
-                return;
-            }
-            Some(val) => val,
-        };
-        let tval = x / NS_PER_S;
-
-        // Check if it is within supported bounds.
-        let warn: Option<&str> = if tval == 0 {
-            Some("smaller")
-        // The upper 32 bits of CNTP_TVAL_EL0 are reserved.
-        } else if tval > u32::max_value().into() {
-            Some("bigger")
-        } else {
-            None
-        };
-
-        #[allow(unused_imports)]
-        if let Some(w) = warn {
-            warn!(
-                "Spin duration {} than architecturally supported, skipping",
-                w
-            );
-            return;
+        if duration > max_duration() {
+            return Err("Conversion error. Duration too big");
         }
 
-        // Set the compare value register.
-        CNTP_TVAL_EL0.set(tval);
+        // This is safe, because all the safety requirements as stated in read_volatile()'s
+        // documentation are fulfilled.
+        let frequency: u128 =
+            unsafe { u32::from(core::ptr::read_volatile(&ARCH_TIMER_COUNTER_FREQUENCY)) as u128 };
+        let duration: u128 = duration.as_nanos();
 
-        // Kick off the counting.                       // Disable timer interrupt.
-        CNTP_CTL_EL0.modify(CNTP_CTL_EL0::ENABLE::SET + CNTP_CTL_EL0::IMASK::SET);
+        // This is safe, because frequency can never be greater than u32::MAX, and
+        // (Duration::MAX.as_nanos() * u32::MAX) < u128::MAX.
+        let counter_value =
+            unsafe { duration.unchecked_mul(frequency) }.div(NonZeroU128::from(NANOSEC_PER_SEC));
 
-        // ISTATUS will be '1' when cval ticks have passed. Busy-check it.
-        while !CNTP_CTL_EL0.matches_all(CNTP_CTL_EL0::ISTATUS::SET) {}
-
-        // Disable counting again.
-        CNTP_CTL_EL0.modify(CNTP_CTL_EL0::ENABLE::CLEAR);
+        // Since we checked above that we are <= max_duration(), just cast to u64.
+        Ok(GenericTimerCounterValue(counter_value as u64))
     }
 }
+
+#[inline(always)]
+fn read_cntpct() -> GenericTimerCounterValue {
+    // Prevent that the counter is read ahead of time due to out-of-order execution.
+    unsafe { barrier::isb(barrier::SY) };
+    let cnt = CNTPCT_EL0.get();
+
+    GenericTimerCounterValue(cnt)
+}
+
+//--------------------------------------------------------------------------------------------------
+// Public Code
+//--------------------------------------------------------------------------------------------------
+
+/// The timer's resolution.
+pub fn resolution() -> Duration {
+    Duration::from(GenericTimerCounterValue(1))
+}
+
+/// The uptime since power-on of the device.
+///
+/// This includes time consumed by firmware and bootloaders.
+pub fn uptime() -> Duration {
+    read_cntpct().into()
+}
+
+/// Spin for a given duration.
+pub fn spin_for(duration: Duration) {
+    let curr_counter_value = read_cntpct();
+
+    let counter_value_delta: GenericTimerCounterValue = match duration.try_into() {
+        Err(msg) => {
+            warn!("spin_for: {}. Skipping", msg);
+            return;
+        }
+        Ok(val) => val,
+    };
+    let counter_value_target = curr_counter_value + counter_value_delta;
+
+    // Busy wait.
+    //
+    // Read CNTPCT_EL0 directly to avoid the ISB that is part of [`read_cntpct`].
+    while GenericTimerCounterValue(CNTPCT_EL0.get()) < counter_value_target {}
+}

07_timestamps/src/_arch/aarch64/time.rs

@@ -140,7 +140,7 @@ impl GPIOInner {
     /// Disable pull-up/down on pins 14 and 15.
     #[cfg(feature = "bsp_rpi3")]
     fn disable_pud_14_15_bcm2837(&mut self) {
-        use crate::{time, time::interface::TimeManager};
+        use crate::time;
         use core::time::Duration;
 
         // The Linux 2837 GPIO driver waits 1 µs between the steps.

07_timestamps/src/bsp/device_driver/bcm/bcm2xxx_gpio.rs

@@ -108,9 +108,12 @@
 
 #![allow(clippy::upper_case_acronyms)]
 #![feature(asm_const)]
+#![feature(const_option)]
 #![feature(format_args_nl)]
+#![feature(nonzero_min_max)]
 #![feature(panic_info_message)]
 #![feature(trait_alias)]
+#![feature(unchecked_math)]
 #![no_main]
 #![no_std]
 
@@ -148,7 +151,6 @@ unsafe fn kernel_init() -> ! {
 fn kernel_main() -> ! {
     use core::time::Duration;
     use driver::interface::DriverManager;
-    use time::interface::TimeManager;
 
     info!(
         "{} version {}",

07_timestamps/src/main.rs

@@ -42,8 +42,6 @@ fn panic_prevent_reenter() {
 
 #[panic_handler]
 fn panic(info: &PanicInfo) -> ! {
-    use crate::time::interface::TimeManager;
-
     // Protect against panic infinite loops if any of the following code panics itself.
     panic_prevent_reenter();
 

07_timestamps/src/panic_wait.rs

@@ -39,8 +39,6 @@ macro_rules! println {
 #[macro_export]
 macro_rules! info {
     ($string:expr) => ({
-        use $crate::time::interface::TimeManager;
-
         let timestamp = $crate::time::time_manager().uptime();
 
         $crate::print::_print(format_args_nl!(
@@ -50,8 +48,6 @@ macro_rules! info {
         ));
     });
     ($format_string:expr, $($arg:tt)*) => ({
-        use $crate::time::interface::TimeManager;
-
         let timestamp = $crate::time::time_manager().uptime();
 
         $crate::print::_print(format_args_nl!(
@@ -67,8 +63,6 @@ macro_rules! info {
 #[macro_export]
 macro_rules! warn {
     ($string:expr) => ({
-        use $crate::time::interface::TimeManager;
-
         let timestamp = $crate::time::time_manager().uptime();
 
         $crate::print::_print(format_args_nl!(
@@ -78,8 +72,6 @@ macro_rules! warn {
         ));
     });
     ($format_string:expr, $($arg:tt)*) => ({
-        use $crate::time::interface::TimeManager;
-
         let timestamp = $crate::time::time_manager().uptime();
 
         $crate::print::_print(format_args_nl!(

07_timestamps/src/print.rs

@@ -8,30 +8,50 @@
 #[path = "_arch/aarch64/time.rs"]
 mod arch_time;
 
+use core::time::Duration;
+
 //--------------------------------------------------------------------------------------------------
-// Architectural Public Reexports
+// Public Definitions
 //--------------------------------------------------------------------------------------------------
-pub use arch_time::time_manager;
+
+/// Provides time management functions.
+pub struct TimeManager;
 
 //--------------------------------------------------------------------------------------------------
-// Public Definitions
+// Global instances
 //--------------------------------------------------------------------------------------------------
 
-/// Timekeeping interfaces.
-pub mod interface {
-    use core::time::Duration;
+static TIME_MANAGER: TimeManager = TimeManager::new();
 
-    /// Time management functions.
-    pub trait TimeManager {
-        /// The timer's resolution.
-        fn resolution(&self) -> Duration;
+//--------------------------------------------------------------------------------------------------
+// Public Code
+//--------------------------------------------------------------------------------------------------
 
-        /// The uptime since power-on of the device.
-        ///
-        /// This includes time consumed by firmware and bootloaders.
-        fn uptime(&self) -> Duration;
+/// Return a reference to the global TimeManager.
+pub fn time_manager() -> &'static TimeManager {
+    &TIME_MANAGER
+}
+
+impl TimeManager {
+    /// Create an instance.
+    pub const fn new() -> Self {
+        Self
+    }
+
+    /// The timer's resolution.
+    pub fn resolution(&self) -> Duration {
+        arch_time::resolution()
+    }
+
+    /// The uptime since power-on of the device.
+    ///
+    /// This includes time consumed by firmware and bootloaders.
+    pub fn uptime(&self) -> Duration {
+        arch_time::uptime()
+    }
 
-        /// Spin for a given duration.
-        fn spin_for(&self, duration: Duration);
+    /// Spin for a given duration.
+    pub fn spin_for(&self, duration: Duration) {
+        arch_time::spin_for(duration)
     }
 }

07_timestamps/src/time.rs

@@ -52,6 +52,14 @@ _start:
 	ADR_REL	x0, __boot_core_stack_end_exclusive
 	mov	sp, x0
 
+	// Read the CPU's timer counter frequency and store it in ARCH_TIMER_COUNTER_FREQUENCY.
+	// Abort if the frequency read back as 0.
+	ADR_REL	x1, ARCH_TIMER_COUNTER_FREQUENCY // provided by aarch64/time.rs
+	mrs	x2, CNTFRQ_EL0
+	cmp	x2, xzr
+	b.eq	.L_parking_loop
+	str	w2, [x1]
+
 	// Jump to Rust code.
 	b	_start_rust