Introduce and use sys_recv_notification.

This is an alternate API for accessing the common "receive only kernel
notifications" pattern. That specific use of the sys_recv syscall is
defined as not being able to fail. Different users have handled that in
different ways -- some panic, some ignore the result. The latter embeds
the assumption of that behavior in many different places in the
codebase, a pattern I'm trying to keep an eye on.

This change adds a new sys_recv_notification userlib function that
bundles up this common pattern. Because of its central placement in
userlib, it seems like a more appropriate place to encode the assumption
about the syscall not failing, and so I've done that, using
unreachable_unchecked to explain it to the compiler.

This removes a panic site from every notification-wait in programs that
were previously using unwrap or unwrap_lite.

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

@@ -153,10 +153,7 @@ fn main() -> ! {
                 // irrelevant. But, `rustc` doesn't realize that this should
                 // never return, we'll stick it in a `loop` anyway so the main
                 // function can return `!`
-                //
-                // We don't care if this returns an error, because we're just
-                // doing it to die as politely as possible.
-                let _ = sys_recv_closed(&mut [], 0, TaskId::KERNEL);
+                sys_recv_notification(0);
             }
         }
     }

drv/lpc55-sha256/src/lib.rs

@@ -55,7 +55,7 @@
 //!   (This restriction would be straightforward to lift if required.)
 
 use core::num::Wrapping;
-use userlib::{sys_irq_control, sys_recv_closed, TaskId};
+use userlib::{sys_irq_control, sys_recv_notification, TaskId};
 
 // These constants describe intrinsic properties of the SHA256 algorithm and
 // should not be changed.
@@ -201,11 +201,7 @@ impl<'a> Hasher<'a> {
 
                 // Wait for it!
                 sys_irq_control(self.notification_mask, true);
-                let _ = sys_recv_closed(
-                    &mut [],
-                    self.notification_mask,
-                    TaskId::KERNEL,
-                );
+                sys_recv_notification(self.notification_mask);
 
                 // Turn it back off lest it spam us in the future.
                 self.engine.intenclr.write(|w| w.digest().set_bit());
@@ -235,11 +231,7 @@ impl<'a> Hasher<'a> {
 
                     // Wait for it!
                     sys_irq_control(self.notification_mask, true);
-                    let _ = sys_recv_closed(
-                        &mut [],
-                        self.notification_mask,
-                        TaskId::KERNEL,
-                    );
+                    sys_recv_notification(self.notification_mask);
 
                     // Turn it back off lest it spam us in the future.
                     self.engine.intenclr.write(|w| w.waiting().set_bit());

drv/lpc55-spi-server/src/main.rs

@@ -83,11 +83,7 @@ fn main() -> ! {
     let mut rx_done = false;
 
     loop {
-        if sys_recv_closed(&mut [], notifications::SPI_IRQ_MASK, TaskId::KERNEL)
-            .is_err()
-        {
-            panic!()
-        }
+        sys_recv_notification(notifications::SPI_IRQ_MASK);
 
         ringbuf_entry!(Trace::Irq);
 

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

@@ -57,7 +57,7 @@ use drv_sprot_api::{
 use lpc55_pac as device;
 use ringbuf::{ringbuf, ringbuf_entry};
 use userlib::{
-    sys_irq_control, sys_recv_closed, task_slot, TaskId, UnwrapLite,
+    sys_irq_control, sys_recv_notification, task_slot, TaskId, UnwrapLite,
 };
 
 mod handler;
@@ -216,12 +216,7 @@ impl Io {
         loop {
             sys_irq_control(notifications::SPI_IRQ_MASK, true);
 
-            sys_recv_closed(
-                &mut [],
-                notifications::SPI_IRQ_MASK,
-                TaskId::KERNEL,
-            )
-            .unwrap_lite();
+            sys_recv_notification(notifications::SPI_IRQ_MASK);
 
             // Is CSn asserted by the SP?
             let intstat = self.spi.intstat();

drv/lpc55-update-server/src/main.rs

@@ -639,12 +639,7 @@ fn indirect_flash_read_words(
 
             flash.enable_interrupt_sources();
             sys_irq_control(notifications::FLASH_IRQ_MASK, true);
-            // RECV from the kernel cannot produce an error, so ignore it.
-            let _ = sys_recv_closed(
-                &mut [],
-                notifications::FLASH_IRQ_MASK,
-                TaskId::KERNEL,
-            );
+            sys_recv_notification(notifications::FLASH_IRQ_MASK);
             flash.disable_interrupt_sources();
         }
     }
@@ -773,9 +768,7 @@ fn do_block_write(
 
 fn wait_for_flash_interrupt() {
     sys_irq_control(notifications::FLASH_IRQ_MASK, true);
-    // RECV from the kernel cannot produce an error, so ignore it.
-    let _ =
-        sys_recv_closed(&mut [], notifications::FLASH_IRQ_MASK, TaskId::KERNEL);
+    sys_recv_notification(notifications::FLASH_IRQ_MASK);
 }
 
 fn same_image(which: UpdateTarget) -> bool {

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

@@ -45,6 +45,6 @@ fn main() -> ! {
     // We have nothing else to do, so sleep forever via waiting for a message
     // from the kernel that won't arrive.
     loop {
-        _ = sys_recv_closed(&mut [], 0, TaskId::KERNEL);
+        sys_recv_notification(0);
     }
 }

drv/stm32h7-eth/src/lib.rs

@@ -435,11 +435,7 @@ impl Ethernet {
         // has disabled itself before proceeding.
         loop {
             userlib::sys_irq_control(self.mdio_timer_irq_mask, true);
-            let _ = userlib::sys_recv_closed(
-                &mut [],
-                self.mdio_timer_irq_mask,
-                userlib::TaskId::KERNEL,
-            );
+            userlib::sys_recv_notification(self.mdio_timer_irq_mask);
             if !self.mdio_timer.cr1.read().cen().bit() {
                 break;
             }

drv/stm32h7-hash/src/lib.rs

@@ -311,8 +311,7 @@ impl Hash {
                     hl::sleep_for(1);
                 }
             }
-            let _rm = sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
-                .unwrap();
+            sys_recv_notification(self.interrupt);
             if self.reg.sr.read().dcis().bit() {
                 break;
             }

drv/stm32h7-qspi/src/lib.rs

@@ -15,7 +15,7 @@ use stm32h7::stm32h743 as device;
 use stm32h7::stm32h753 as device;
 
 use drv_qspi_api::Command;
-use userlib::{sys_irq_control, sys_recv_closed, TaskId};
+use userlib::{sys_irq_control, sys_recv_notification};
 use zerocopy::AsBytes;
 
 const FIFO_SIZE: usize = 32;
@@ -198,9 +198,7 @@ impl Qspi {
                 // Unmask our interrupt.
                 sys_irq_control(self.interrupt, true);
                 // And wait for it to arrive.
-                let _rm =
-                    sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
-                        .unwrap();
+                sys_recv_notification(self.interrupt);
                 if self.reg.sr.read().ftf().bit() {
                     break;
                 }
@@ -218,8 +216,7 @@ impl Qspi {
             // Unmask our interrupt.
             sys_irq_control(self.interrupt, true);
             // And wait for it to arrive.
-            let _rm = sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
-                .unwrap();
+            sys_recv_notification(self.interrupt);
         }
         self.reg.cr.modify(|_, w| w.tcie().clear_bit());
     }
@@ -284,9 +281,7 @@ impl Qspi {
                 // Unmask our interrupt.
                 sys_irq_control(self.interrupt, true);
                 // And wait for it to arrive.
-                let _rm =
-                    sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
-                        .unwrap();
+                sys_recv_notification(self.interrupt);
 
                 // Try the check again. We may retry the check on spurious
                 // wakeups, but, spurious wakeups are expected to be pretty
@@ -321,8 +316,7 @@ impl Qspi {
             // Unmask our interrupt.
             sys_irq_control(self.interrupt, true);
             // And wait for it to arrive.
-            let _rm = sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
-                .unwrap();
+            sys_recv_notification(self.interrupt);
         }
 
         // Clean up by disabling our interrupt sources.

drv/stm32h7-spi-server-core/src/lib.rs

@@ -537,10 +537,8 @@ impl SpiServerCore {
                 // Allow the controller interrupt to post to our
                 // notification set.
                 sys_irq_control(self.irq_mask, true);
-                // Wait for our notification set to get, well, set. We ignore
-                // the result of this because an error would mean the kernel
-                // violated the ABI, which we can't usefully respond to.
-                let _ = sys_recv_closed(&mut [], self.irq_mask, TaskId::KERNEL);
+                // Wait for our notification set to get, well, set.
+                sys_recv_notification(self.irq_mask);
             }
         }
 

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

@@ -264,12 +264,7 @@ impl<'a> ServerImpl<'a> {
 
         // Wait for EOP notification via interrupt.
         loop {
-            sys_recv_closed(
-                &mut [],
-                notifications::FLASH_IRQ_MASK,
-                TaskId::KERNEL,
-            )
-            .unwrap_lite();
+            sys_recv_notification(notifications::FLASH_IRQ_MASK);
             if self.flash.bank2().sr.read().eop().bit() {
                 break;
             } else {

drv/stm32xx-i2c-server/src/main.rs

@@ -343,17 +343,17 @@ fn main() -> ! {
         wfi: |notification, timeout| {
             const TIMER_NOTIFICATION: u32 = 1 << 31;
 
-            let dead = sys_get_timer().now.checked_add(timeout.0).unwrap_lite();
+            // If the driver passes in a timeout that is large enough that it
+            // would overflow the kernel's 64-bit timestamp space... well, we'll
+            // do the best we can without compiling in an unlikely panic.
+            let dead = sys_get_timer().now.saturating_add(timeout.0);
+
             sys_set_timer(Some(dead), TIMER_NOTIFICATION);
 
-            let m = sys_recv_closed(
-                &mut [],
-                notification | TIMER_NOTIFICATION,
-                TaskId::KERNEL,
-            )
-            .unwrap_lite();
+            let notification =
+                sys_recv_notification(notification | TIMER_NOTIFICATION);
 
-            if m.operation == TIMER_NOTIFICATION {
+            if notification == TIMER_NOTIFICATION {
                 I2cControlResult::TimedOut
             } else {
                 sys_set_timer(None, TIMER_NOTIFICATION);

sys/userlib/src/hl.rs

@@ -7,7 +7,7 @@
 //! This is intended to provide a more ergonomic interface than the raw
 //! syscalls.
 
-use abi::TaskId;
+use abi::{Generation, TaskId};
 use core::marker::PhantomData;
 use unwrap_lite::UnwrapLite;
 use zerocopy::{AsBytes, FromBytes, LayoutVerified};
@@ -147,8 +147,8 @@ where
     O: FromPrimitive,
     E: Into<u32>,
 {
-    let rm =
-        sys_recv(buffer, mask, source).map_err(|_| ClosedRecvError::Dead)?;
+    let rm = sys_recv(buffer, mask, source)
+        .map_err(|code| ClosedRecvError::Dead(Generation::from(code as u8)))?;
     let sender = rm.sender;
     if rm.sender == TaskId::KERNEL {
         notify(state, rm.operation);

sys/userlib/src/lib.rs

@@ -237,12 +237,16 @@ unsafe extern "C" fn sys_send_stub(_args: &mut SendArgs<'_>) -> RcLen {
 /// let it, but it always receives _something_.
 #[inline(always)]
 pub fn sys_recv_open(buffer: &mut [u8], notification_mask: u32) -> RecvMessage {
-    // The open-receive version of the syscall is defined as being unable to
-    // fail, and so we should always get a success here. (This is not using
-    // `unwrap` because that generates handling code with formatting.)
     match sys_recv(buffer, notification_mask, None) {
         Ok(rm) => rm,
-        Err(_) => panic!(),
+        Err(_) => {
+            // Safety: the open-receive version of the syscall is defined as
+            // being unable to fail in the kernel ABI, so this path can't happen
+            // modulo a kernel bug.
+            unsafe {
+                core::hint::unreachable_unchecked()
+            }
+        },
     }
 }
 
@@ -259,20 +263,32 @@ pub fn sys_recv_open(buffer: &mut [u8], notification_mask: u32) -> RecvMessage {
 ///
 /// If `sender` is stale (i.e. refers to a deceased generation of the task) when
 /// you call this, or if `sender` is rebooted while you're blocked in this
-/// operation, this will fail with `ClosedRecvError::Dead`.
+/// operation, this will fail with `ClosedRecvError::Dead`, indicating the
+/// `sender`'s new generation (not that a server generally cares).
 #[inline(always)]
 pub fn sys_recv_closed(
     buffer: &mut [u8],
     notification_mask: u32,
     sender: TaskId,
 ) -> Result<RecvMessage, ClosedRecvError> {
-    sys_recv(buffer, notification_mask, Some(sender))
-        .map_err(|_| ClosedRecvError::Dead)
+    sys_recv(buffer, notification_mask, Some(sender)).map_err(|code| {
+        // We're not using the extract_new_generation function here because
+        // that has a failure code path for cases where the code is not a
+        // dead code. In this case, sys_recv is defined as being _only_
+        // capable of returning a dead code -- otherwise we have a serious
+        // kernel bug. So to avoid the introduction of a panic that can't
+        // trigger, we will do this manually:
+        ClosedRecvError::Dead(Generation::from(code as u8))
+    })
 }
 
+/// Things that can go wrong (without faulting) during a closed receive
+/// operation.
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum ClosedRecvError {
-    Dead,
+    /// The task you requested to receive from has restarted and the message may
+    /// never come.
+    Dead(Generation),
 }
 
 /// General version of RECV that lets you pick closed vs. open receive at
@@ -287,8 +303,9 @@ pub fn sys_recv(
 ) -> Result<RecvMessage, u32> {
     use core::mem::MaybeUninit;
 
-    // Flatten option into a packed u32.
-    let specific_sender = specific_sender
+    // Flatten option into a packed u32; in the C-compatible ABI we provide the
+    // task ID in the LSBs, and the "some" flag in the MSB.
+    let specific_sender_bits = specific_sender
         .map(|tid| (1u32 << 31) | u32::from(tid.0))
         .unwrap_or(0);
     let mut out = MaybeUninit::<RawRecvMessage>::uninit();
@@ -297,7 +314,7 @@ pub fn sys_recv(
             buffer.as_mut_ptr(),
             buffer.len(),
             notification_mask,
-            specific_sender,
+            specific_sender_bits,
             out.as_mut_ptr(),
         )
     };
@@ -319,6 +336,25 @@ pub fn sys_recv(
     }
 }
 
+/// 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 {
+    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
+        }
+        Err(_) => {
+            // Because we passed Some(TaskId::KERNEL), this is defined as not
+            // being able to happen.
+            unsafe { core::hint::unreachable_unchecked() }
+        }
+    }
+}
+
 pub struct RecvMessage {
     pub sender: TaskId,
     pub operation: u32,

task/gimlet-inspector/src/main.rs

@@ -99,24 +99,14 @@ fn main() -> ! {
                         // packets off our recv queue at the top of our main
                         // loop.
                         Err(SendError::QueueFull) => {
-                            sys_recv_closed(
-                                &mut [],
-                                notifications::SOCKET_MASK,
-                                TaskId::KERNEL,
-                            )
-                            .unwrap_lite();
+                            sys_recv_notification(notifications::SOCKET_MASK);
                         }
                     }
                 }
             }
             Err(RecvError::QueueEmpty) => {
                 // Our incoming queue is empty. Wait for more packets.
-                sys_recv_closed(
-                    &mut [],
-                    notifications::SOCKET_MASK,
-                    TaskId::KERNEL,
-                )
-                .unwrap_lite();
+                sys_recv_notification(notifications::SOCKET_MASK);
             }
             Err(RecvError::ServerRestarted) => {
                 // `net` restarted; just retry.