diff --git a/doc/syscalls.adoc b/doc/syscalls.adoc index a353b7674..b721f0dde 100644 --- a/doc/syscalls.adoc +++ b/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. diff --git a/sys/abi/src/lib.rs b/sys/abi/src/lib.rs index dd00782e4..3ef37e36f 100644 --- a/sys/abi/src/lib.rs +++ b/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; + } +} diff --git a/sys/kern/src/arch/arm_m.rs b/sys/kern/src/arch/arm_m.rs index c8d70a0dd..cb2928d71 100644 --- a/sys/kern/src/arch/arm_m.rs +++ b/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,7 +1148,7 @@ 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; @@ -1156,13 +1156,24 @@ pub fn disable_irq(n: u32) { 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); } diff --git a/sys/kern/src/syscalls.rs b/sys/kern/src/syscalls.rs index 52517ef63..d97c08f1e 100644 --- a/sys/kern/src/syscalls.rs +++ b/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 { 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) } diff --git a/sys/userlib/src/lib.rs b/sys/userlib/src/lib.rs index 99632049f..3a78b4e11 100644 --- a/sys/userlib/src/lib.rs +++ b/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()); } }