diff --git a/capsules/core/src/alarm.rs b/capsules/core/src/alarm.rs index 9c23dd244e..cbf5d1f7ef 100644 --- a/capsules/core/src/alarm.rs +++ b/capsules/core/src/alarm.rs @@ -8,7 +8,7 @@ use core::cell::Cell; use kernel::grant::{AllowRoCount, AllowRwCount, Grant, UpcallCount}; -use kernel::hil::time::{self, Alarm, Frequency, Ticks, Ticks32}; +use kernel::hil::time::{self, Alarm, Ticks, Ticks32}; use kernel::syscall::{CommandReturn, SyscallDriver}; use kernel::{ErrorCode, ProcessId}; @@ -186,26 +186,37 @@ impl<'a, A: Alarm<'a>> SyscallDriver for AlarmDriver<'a, A> { if let Expiration::Disabled = td.expiration { self.num_armed.set(self.num_armed.get() + 1); } + td.expiration = Expiration::Enabled { reference: reference as u32, dt: dt as u32, }; - ( - CommandReturn::success_u32(reference.wrapping_add(dt) as u32), - true, - ) + + reference.wrapping_add(dt) as u32 }; + let now = self.alarm.now(); match cmd_type { 0 => (CommandReturn::success(), false), 1 => { - // Get clock frequency - let freq = ::frequency(); - (CommandReturn::success_u32(freq), false) + // Get clock frequency. We return a frequency scaled by + // the amount of padding we add to the `ticks` value + // returned in command 2 ("capture time"), such that + // userspace knows when the timer will wrap and can + // accurately determine the duration of a single tick. + let scaled_freq = + ::u32_left_justified_scale_freq::(); + (CommandReturn::success_u32(scaled_freq), false) } 2 => { - // capture time - (CommandReturn::success_u32(now.into_u32()), false) + // Capture time. We pad the underlying timer's ticks to + // wrap at exactly `(2 ** 32) - 1`. This predictable + // wrapping value allows userspace to build long running + // timers beyond `2 ** now.width()` ticks. + ( + CommandReturn::success_u32(now.into_u32_left_justified()), + false, + ) } 3 => { // Stop @@ -227,17 +238,63 @@ impl<'a, A: Alarm<'a>> SyscallDriver for AlarmDriver<'a, A> { (CommandReturn::failure(ErrorCode::NOSUPPORT), false) } 5 => { - // Set relative expiration + // Set relative expiration. We provided userspace a + // potentially padded version of our in-kernel Ticks + // object, and as such we have to invert that operation + // here. let reference = now.into_u32() as usize; - let dt = data; + + // We do not want to switch back to userspace *before* + // the timer fires. As such, when userspace gives us + // reference and ticks values with a precision + // unrepresentible using our Ticks object, we round up: + let dt = if data & ((1 << A::Ticks::u32_padding()) - 1) != 0 { + // By right-shifting, we would decrease the + // requested dt value. Add one to compensate: + (data >> A::Ticks::u32_padding()) + 1 + } else { + // dt does not need to be shifted or contains no + // unrepresentable precision: + data >> A::Ticks::u32_padding() + }; + // if previously unarmed, but now will become armed - rearm(reference, dt) + ( + CommandReturn::success_u32( + rearm(reference, dt) << A::Ticks::u32_padding(), + ), + true, + ) } 6 => { - // Set absolute expiration with reference point - let reference = data; - let dt = data2; - rearm(reference, dt) + // Set absolute expiration with reference point. We + // provided userspace a potentially padded version of + // our in-kernel Ticks object, and as such we have to + // invert that operation here. + // + // We do not want to switch back to userspace *before* + // the timer fires. As such, when userspace gives us + // reference and ticks values with a precision + // unrepresentible using our Ticks object, we round + // `reference` down, and `dt` up (ensuring that the + // timer cannot fire earlier than requested). + let reference = data >> A::Ticks::u32_padding(); + let dt = if data2 & ((1 << A::Ticks::u32_padding()) - 1) != 0 { + // By right-shifting, we would decrease the + // requested dt value. Add one to compensate: + (data2 >> A::Ticks::u32_padding()) + 1 + } else { + // dt does not need to be shifted or contains no + // unrepresentable precision: + data2 >> A::Ticks::u32_padding() + }; + + ( + CommandReturn::success_u32( + rearm(reference, dt) << A::Ticks::u32_padding(), + ), + true, + ) } _ => (CommandReturn::failure(ErrorCode::NOSUPPORT), false), } @@ -260,7 +317,7 @@ impl<'a, A: Alarm<'a>> SyscallDriver for AlarmDriver<'a, A> { impl<'a, A: Alarm<'a>> time::AlarmClient for AlarmDriver<'a, A> { fn alarm(&self) { - let now: Ticks32 = Ticks32::from(self.alarm.now().into_u32()); + let now: Ticks32 = Ticks32::from(self.alarm.now().into_u32_left_justified()); self.app_alarms.each(|_processid, alarm, upcalls| { if let Expiration::Enabled { reference, dt } = alarm.expiration { // Now is not within reference, reference + ticks; this timer @@ -275,7 +332,7 @@ impl<'a, A: Alarm<'a>> time::AlarmClient for AlarmDriver<'a, A> { .schedule_upcall( ALARM_CALLBACK_NUM, ( - now.into_u32() as usize, + now.into_u32_left_justified() as usize, reference.wrapping_add(dt) as usize, 0, ), diff --git a/kernel/src/hil/time.rs b/kernel/src/hil/time.rs index bb90b461e5..c2ba877e06 100644 --- a/kernel/src/hil/time.rs +++ b/kernel/src/hil/time.rs @@ -23,11 +23,51 @@ use core::fmt; /// clients to know when wraparound will occur. pub trait Ticks: Clone + Copy + From + fmt::Debug + Ord + PartialOrd + Eq { + /// Width of the actual underlying timer in bits. + /// + /// The maximum value that *will* be attained by this timer should + /// be `(2 ** width) - 1`. In other words, the timer will wrap at + /// exactly `width` bits, and then continue counting at `0`. + /// + /// The return value is a `u32`, in accordance with the bit widths + /// specified using the BITS associated const on Rust integer + /// types. + fn width() -> u32; + /// Converts the type into a `usize`, stripping the higher bits /// it if it is larger than `usize` and filling the higher bits /// with 0 if it is smaller than `usize`. fn into_usize(self) -> usize; + /// The amount of bits required to left-justify this ticks value + /// range (filling the lower bits with `0`) for it wrap at `(2 ** + /// usize::BITS) - 1` bits. For timers with a `width` larger than + /// usize, this value will be `0` (i.e., they can simply be + /// truncated to usize::BITS bits). + fn usize_padding() -> u32 { + usize::BITS.saturating_sub(Self::width()) + } + + /// Converts the type into a `usize`, left-justified and + /// right-padded with `0` such that it is guaranteed to wrap at + /// `(2 ** usize::BITS) - 1`. If it is larger than usize::BITS + /// bits, any higher bits are stripped. + /// + /// The resulting tick rate will possibly be higher (multiplied by + /// `2 ** usize_padding()`). Use `usize_left_justified_scale_freq` + /// to convert the underlying timer's frequency into the padded + /// ticks frequency in Hertz. + fn into_usize_left_justified(self) -> usize { + self.into_usize() << Self::usize_padding() + } + + /// Convert the generic [`Frequency`] argument into a frequency + /// (Hertz) describing a left-justified ticks value as returned by + /// [`Ticks::into_usize_left_justified`]. + fn usize_left_justified_scale_freq() -> u32 { + F::frequency() << Self::usize_padding() + } + /// Converts the type into a `u32`, stripping the higher bits /// it if it is larger than `u32` and filling the higher bits /// with 0 if it is smaller than `u32`. Included as a simple @@ -35,6 +75,39 @@ pub trait Ticks: Clone + Copy + From + fmt::Debug + Ord + PartialOrd + Eq { /// are 32 bits. fn into_u32(self) -> u32; + /// The amount of bits required to left-justify this ticks value + /// range (filling the lower bits with `0`) for it wrap at `(2 ** + /// 32) - 1` bits. For timers with a `width` larger than 32, this + /// value will be `0` (i.e., they can simply be truncated to + /// 32-bits). + /// + /// The return value is a `u32`, in accordance with the bit widths + /// specified using the BITS associated const on Rust integer + /// types. + fn u32_padding() -> u32 { + u32::BITS.saturating_sub(Self::width()) + } + + /// Converts the type into a `u32`, left-justified and + /// right-padded with `0` such that it is guaranteed to wrap at + /// `(2 ** 32) - 1`. If it is larger than 32-bits, any higher bits + /// are stripped. + /// + /// The resulting tick rate will possibly be higher (multiplied by + /// `2 ** u32_padding()`). Use `u32_left_justified_scale_freq` to + /// convert the underlying timer's frequency into the padded ticks + /// frequency in Hertz. + fn into_u32_left_justified(self) -> u32 { + self.into_u32() << Self::u32_padding() + } + + /// Convert the generic [`Frequency`] argument into a frequency + /// (Hertz) describing a left-justified ticks value as returned by + /// [`Ticks::into_u32_left_justified`]. + fn u32_left_justified_scale_freq() -> u32 { + F::frequency() << Self::u32_padding() + } + /// Add two values, wrapping around on overflow using standard /// unsigned arithmetic. fn wrapping_add(self, other: Self) -> Self; @@ -397,6 +470,10 @@ impl From for Ticks32 { } impl Ticks for Ticks32 { + fn width() -> u32 { + 32 + } + fn into_usize(self) -> usize { self.0 as usize } @@ -471,13 +548,21 @@ impl Eq for Ticks32 {} #[derive(Clone, Copy, Debug)] pub struct Ticks24(u32); +impl Ticks24 { + pub const MASK: u32 = 0x00FFFFFF; +} + impl From for Ticks24 { fn from(val: u32) -> Self { - Ticks24(val) + Ticks24(val & Self::MASK) } } impl Ticks for Ticks24 { + fn width() -> u32 { + 24 + } + fn into_usize(self) -> usize { self.0 as usize } @@ -487,11 +572,11 @@ impl Ticks for Ticks24 { } fn wrapping_add(self, other: Self) -> Self { - Ticks24(self.0.wrapping_add(other.0) & 0x00FFFFFF) + Ticks24(self.0.wrapping_add(other.0) & Self::MASK) } fn wrapping_sub(self, other: Self) -> Self { - Ticks24(self.0.wrapping_sub(other.0) & 0x00FFFFFF) + Ticks24(self.0.wrapping_sub(other.0) & Self::MASK) } fn within_range(self, start: Self, end: Self) -> bool { @@ -500,7 +585,7 @@ impl Ticks for Ticks24 { /// Returns the maximum value of this type, which should be (2^width)-1. fn max_value() -> Self { - Ticks24(0x00FFFFFF) + Ticks24(Self::MASK) } /// Returns the half the maximum value of this type, which should be (2^width-1). @@ -571,6 +656,10 @@ impl Ticks16 { } impl Ticks for Ticks16 { + fn width() -> u32 { + 16 + } + fn into_usize(self) -> usize { self.0 as usize } @@ -664,6 +753,10 @@ impl From for Ticks64 { } impl Ticks for Ticks64 { + fn width() -> u32 { + 64 + } + fn into_usize(self) -> usize { self.0 as usize }