kern: let drivers disable double-interrupt behavior

doc/syscalls.adoc

@@ -469,7 +469,9 @@ Collects information about one entry in a sender's lease table.
 ==== Arguments
 
 - 0: notification bitmask corresponding to the interrupt
-- 1: desired state (0 = disabled, 1 = enabled)
+- 1: desired state
+** bit 0: 0 = disabled, 1 = enabled
+** bit 1: 0 = leave pending, 1 = clear pending
 
 ==== Return values
 
@@ -498,6 +500,13 @@ their notification bits. However, this is quite deliberate, for two reasons:
 2. It makes it impossible for a task to mess with other tasks' interrupts,
    since it can only refer to its _own_ mapped interrupts, by construction.
 
+The concept of a "pending" interrupt is inherently specific to a particular
+architecture and interrupt controller. On an architecture without a concept of
+pending interrupts, bit 1 has no effect. However, on architectures with
+level-triggered interrupts from peripherals and a concept of "pending"
+interrupts, clearing the pending status when re-enabling may be important for
+avoiding a duplicate notification.
+
 === `PANIC` (8)
 
 Delivers a `Panic` fault to the calling task, recording an optional message.

sys/abi/src/lib.rs

@@ -549,3 +549,15 @@ bitflags::bitflags! {
         const POSTED = 1 << 2;
     }
 }
+
+bitflags::bitflags! {
+    /// Bitflags that can be passed into the `IRQ_CONTROL` syscall.
+    #[derive(Copy, Clone, Debug, PartialEq, Eq)]
+    pub struct IrqControlArg: u32 {
+        /// Enables the interrupt if present, disables if not present.
+        const ENABLED = 1 << 0;
+        /// If present, requests that any pending instance of this interrupt be
+        // cleared.
+        const CLEAR_PENDING = 1 << 1;
+    }
+}

sys/kern/src/arch/arm_m.rs

@@ -1131,7 +1131,7 @@ pub unsafe extern "C" fn DefaultHandler() {
                 .unwrap_or_else(|| panic!("unhandled IRQ {irq_num}"));
 
             let switch = with_task_table(|tasks| {
-                disable_irq(irq_num);
+                disable_irq(irq_num, false);
 
                 // Now, post the notification and return the
                 // scheduling hint.
@@ -1148,21 +1148,32 @@ pub unsafe extern "C" fn DefaultHandler() {
     crate::profiling::event_isr_exit();
 }
 
-pub fn disable_irq(n: u32) {
+pub fn disable_irq(n: u32, also_clear_pending: bool) {
     // Disable the interrupt by poking the Interrupt Clear Enable Register.
     let nvic = unsafe { &*cortex_m::peripheral::NVIC::PTR };
     let reg_num = (n / 32) as usize;
     let bit_mask = 1 << (n % 32);
     unsafe {
         nvic.icer[reg_num].write(bit_mask);
     }
+    if also_clear_pending {
+        unsafe {
+            nvic.icpr[reg_num].write(bit_mask);
+        }
+    }
 }
 
-pub fn enable_irq(n: u32) {
+pub fn enable_irq(n: u32, also_clear_pending: bool) {
     // Enable the interrupt by poking the Interrupt Set Enable Register.
     let nvic = unsafe { &*cortex_m::peripheral::NVIC::PTR };
     let reg_num = (n / 32) as usize;
     let bit_mask = 1 << (n % 32);
+    if also_clear_pending {
+        // Do this _before_ enabling.
+        unsafe {
+            nvic.icpr[reg_num].write(bit_mask);
+        }
+    }
     unsafe {
         nvic.iser[reg_num].write(bit_mask);
     }

sys/kern/src/syscalls.rs

@@ -31,8 +31,8 @@
 use core::sync::atomic::{AtomicBool, Ordering};
 
 use abi::{
-    FaultInfo, IrqStatus, LeaseAttributes, SchedState, Sysnum, TaskId,
-    TaskState, ULease, UsageError,
+    FaultInfo, IrqControlArg, IrqStatus, LeaseAttributes, SchedState, Sysnum,
+    TaskId, TaskState, ULease, UsageError,
 };
 use unwrap_lite::UnwrapLite;
 
@@ -753,15 +753,19 @@ fn irq_control(
 ) -> Result<NextTask, UserError> {
     let args = tasks[caller].save().as_irq_args();
 
-    let operation = match args.control {
-        0 => crate::arch::disable_irq,
-        1 => crate::arch::enable_irq,
-        _ => {
-            return Err(UserError::Unrecoverable(FaultInfo::SyscallUsage(
-                UsageError::NoIrq,
-            )))
-        }
+    // TODO: our use of NoIrq here is conventional but is getting increasingly
+    // weird. Arguably it's always been wrong, since it's validating the control
+    // argument.
+    let control = IrqControlArg::from_bits(args.control).ok_or(
+        UserError::Unrecoverable(FaultInfo::SyscallUsage(UsageError::NoIrq)),
+    )?;
+
+    let operation = if control.contains(IrqControlArg::ENABLED) {
+        crate::arch::enable_irq
+    } else {
+        crate::arch::disable_irq
     };
+    let also_clear_pending = control.contains(IrqControlArg::CLEAR_PENDING);
 
     let caller = caller as u32;
 
@@ -774,7 +778,7 @@ fn irq_control(
             UsageError::NoIrq,
         )))?;
     for i in irqs.iter() {
-        operation(i.0);
+        operation(i.0, also_clear_pending);
     }
     Ok(NextTask::Same)
 }

sys/userlib/src/lib.rs

@@ -964,8 +964,30 @@ unsafe extern "C" fn sys_borrow_info_stub(
 
 #[inline(always)]
 pub fn sys_irq_control(mask: u32, enable: bool) {
+    let mut arg = IrqControlArg::empty();
+    if enable {
+        arg |= IrqControlArg::ENABLED;
+    }
+
+    unsafe {
+        sys_irq_control_stub(mask, arg.bits());
+    }
+}
+
+/// Variation on [`sys_irq_control`] that also clears any pending interrupt.
+///
+/// This sets the interrupt enable status based on `enable`, and also cancels a
+/// pending instance of this interrupt in the interrupt controller, if the
+/// interrupt controller supports such a concept (ARM M-profile NVIC does, for
+/// instance).
+#[inline(always)]
+pub fn sys_irq_control_clear_pending(mask: u32, enable: bool) {
+    let mut arg = IrqControlArg::CLEAR_PENDING;
+    if enable {
+        arg |= IrqControlArg::ENABLED;
+    }
     unsafe {
-        sys_irq_control_stub(mask, enable as u32);
+        sys_irq_control_stub(mask, arg.bits());
     }
 }