Add NotificationBits type and use it in sys_recv_notification

Author: mkeeter

drv/psc-seq-server/src/main.rs

@@ -470,9 +470,7 @@ fn main() -> ! {
         // Wait for a pin change or timer.
         let n = sys_recv_notification(sleep_notifications);
         // If the timer bit is set _and the timer has actually fired_...
-        if n & notifications::TIMER_MASK != 0
-            && sys_get_timer().deadline.is_none()
-        {
+        if n.has_timer_fired(notifications::TIMER_MASK) {
             // Reset our timer forward.
             sys_set_timer(
                 Some(now.saturating_add(POLL_MS)),

drv/stm32h7-sprot-server/src/main.rs

@@ -389,7 +389,7 @@ impl<S: SpiServer> Io<S> {
             .unwrap_lite();
 
         // Determine the deadline after which we'll give up, and start the clock.
-        let expected_wake = set_timer_relative(max_sleep, TIMER_MASK);
+        set_timer_relative(max_sleep, TIMER_MASK);
 
         let mut irq_fired = false;
         while self.is_rot_irq_asserted() != desired {
@@ -406,21 +406,21 @@ impl<S: SpiServer> Io<S> {
             // notification bit, because it's possible *both* notifications were
             // posted before we were scheduled again, and if the IRQ did fire,
             // we'd prefer to honor that.
-            irq_fired = notif & ROT_IRQ_MASK != 0
-                && self
-                    .sys
+            irq_fired = notif.check_condition(ROT_IRQ_MASK, || {
+                self.sys
                     // If the IRQ hasn't fired, leave it enabled, otherwise,
                     // if it has fired, don't re-enable the IRQ.
                     .gpio_irq_control(ROT_IRQ_MASK, IrqControl::Check)
                     // Sys task shouldn't panic.
-                    .unwrap_lite();
+                    .unwrap_lite()
+            });
             if irq_fired {
                 break;
             }
 
             // If the timer notification was posted, and the GPIO IRQ
             // notification wasn't, we've waited for the timeout. Too bad!
-            if notif & TIMER_MASK != 0 && sys_get_timer().now >= expected_wake {
+            if notif.has_timer_fired(TIMER_MASK) {
                 // Disable the IRQ, so that we don't get the notification later
                 // while in `recv`.
                 self.sys

drv/stm32xx-i2c/src/lib.rs

@@ -1199,30 +1199,27 @@ fn wfi_raw(event_mask: u32, timeout: I2cTimeout) -> I2cControlResult {
     loop {
         let received = sys_recv_notification(event_mask | TIMER_NOTIFICATION);
 
-        // If the event arrived _and_ our timer went off, prioritize the event and
-        // ignore the timeout.
-        if received & event_mask != 0 {
+        // If the event arrived _and_ our timer went off, prioritize the event
+        // and ignore the timeout.
+        if received.check_notification_mask(event_mask) {
             // Attempt to clear our timer. Note that this does not protect against
             // the race where the kernel decided to wake us up, but the timer went
             // off before we got to this point.
             sys_set_timer(None, TIMER_NOTIFICATION);
             break I2cControlResult::Interrupted;
-        } else {
-            // The timer bit must have been set. Verify that our timer has
-            // actually expired:
-            if sys_get_timer().now >= dead {
-                break I2cControlResult::TimedOut;
-            }
-
-            // Otherwise, one of two things has happened:
-            // 1. Some joker has posted to our timer bit. Ha ha very funny.
-            // 2. The timer bit was _already set_ on entry to this routine, as a
-            //    hangover from a previous run.
-            //
-            // We don't need to re-set our timer here because we sampled
-            // sys_get_timer _after_ receiving notifications, meaning it either
-            // hasn't gone off, or it has gone off but that fact is still stored
-            // in our notification bits.
+        } else if received.has_timer_fired(TIMER_NOTIFICATION) {
+            // The timer has actually expired!
+            break I2cControlResult::TimedOut;
         }
+
+        // Otherwise, one of two things has happened:
+        // 1. Some joker has posted to our timer bit. Ha ha very funny.
+        // 2. The timer bit was _already set_ on entry to this routine, as a
+        //    hangover from a previous run.
+        //
+        // We don't need to re-set our timer here because we sampled
+        // sys_get_timer _after_ receiving notifications, meaning it either
+        // hasn't gone off, or it has gone off but that fact is still stored in
+        // our notification bits.
     }
 }

sys/userlib/src/lib.rs

@@ -330,16 +330,72 @@ pub fn sys_recv(
     }
 }
 
+/// Bitmask representing notifications from the kernel
+///
+/// The raw notification bits are available in [`get_raw_bits`], but
+/// higher-level functions make it harder to misuse notifications.
+#[derive(Copy, Clone)]
+#[repr(transparent)]
+pub struct NotificationBits(u32);
+
+impl NotificationBits {
+    /// Wraps a `u32`
+    #[inline]
+    pub fn new(bits: u32) -> Self {
+        Self(bits)
+    }
+
+    /// Returns the raw notification bitmask
+    #[inline]
+    pub fn get_raw_bits(&self) -> u32 {
+        self.0
+    }
+
+    /// Checks if a condition signaled by notification bits holds
+    ///
+    /// The `cond` function should verify the underlying condition, but as an
+    /// optimization, it will only be called if any bits set in mask are also
+    /// set in `self`. This double-check is important, because notification bits
+    /// can potentially be set even if the underlying condition is not true,
+    /// such as with extra pended interrupts or inter-task use of `sys_post`.
+    ///
+    /// If you find yourself passing `|| true` here, see
+    /// [`check_notification_mask`].
+    #[inline]
+    pub fn check_condition<F: Fn() -> bool>(&self, mask: u32, cond: F) -> bool {
+        self.check_notification_mask(mask) && cond()
+    }
+
+    /// Checks whether the given notification mask is active
+    ///
+    /// Notifications may occur spuriously; it is recommended to use
+    /// `check_condition` instead, if there's a way to verify that the event has
+    /// actually occurred.
+    #[inline]
+    pub fn check_notification_mask(&self, mask: u32) -> bool {
+        (self.0 & mask) != 0
+    }
+
+    /// Checks whether a timer appears to have gone off, because the timer_mask
+    /// bit is set and the kernel timer is no longer set.
+    ///
+    /// `mask` must the notification bitmask for the timer notification
+    #[inline]
+    pub fn has_timer_fired(&self, timer_mask: u32) -> bool {
+        self.check_condition(timer_mask, || sys_get_timer().deadline.is_none())
+    }
+}
+
 /// Convenience wrapper for `sys_recv` for the specific, but common, task of
 /// listening for notifications. In this specific use, it has the advantage of
 /// never panicking and not returning a `Result` that must be checked.
 #[inline(always)]
-pub fn sys_recv_notification(notification_mask: u32) -> u32 {
+pub fn sys_recv_notification(notification_mask: u32) -> NotificationBits {
     match sys_recv(&mut [], notification_mask, Some(TaskId::KERNEL)) {
         Ok(rm) => {
             // The notification bits come back from the kernel in the operation
             // code field.
-            rm.operation
+            NotificationBits(rm.operation)
         }
         Err(_) => {
             // Safety: Because we passed Some(TaskId::KERNEL), this is defined

task/nucleo-user-button/src/main.rs

@@ -104,7 +104,7 @@ pub fn main() -> ! {
 
         // Wait for the user button to be pressed.
         let notif = sys_recv_notification(notifications::BUTTON_MASK);
-        ringbuf_entry!(Trace::Notification(notif));
+        ringbuf_entry!(Trace::Notification(notif.get_raw_bits()));
     }
 }