kern: let drivers disable double-interrupt behavior

Currently drivers tend to get a spurious interrupt notification just
after handling a real one. This is because the interrupt controller
notices the kernel generic ISR exit while the interrupt condition is
still asserted, because it doesn't expect us to be handling it in task
code. So, it sets the pending bit so the 'new' interrupt condition isn't
forgotten.

This causes an interrupt to occur immediately when we re-enable.

This adds an additional bit for IRQ_CONTROL that drivers can use to
request any pending interrupt status be cleared.

Fixes #536.

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());
     }
 }