Simplify ISR registration for cortex-m

arch/arm/cortex_m/link.x

@@ -159,6 +159,12 @@ SECTIONS
     KEEP(*(.bk_app_array))
     PROVIDE_HIDDEN (__bk_app_array_end = .);
 
+    . = ALIGN(4);
+    PROVIDE_HIDDEN(__isr_array_start = .);
+    KEEP(*(SORT_BY_INIT_PRIORITY(.isr.reg.*)))
+    KEEP(*(.isr.reg))
+    PROVIDE_HIDDEN(__isr_array_end = .);
+
     . = ALIGN(4);
     __start___llvm_prf_cnts = .;
     KEEP(*(__llvm_prf_cnts))

driver/BUILD.gn

@@ -21,10 +21,12 @@ build_rust("blueos_driver") {
            "//external/vendor/bitflags-2.10.0:bitflags",
            "//external/vendor/cfg-if-1.0.4:cfg_if",
            "//external/vendor/safe-mmio-0.2.5:safe_mmio",
+           "//external/vendor/spin-0.9.8:spin",
            "//external/vendor/tock-registers-0.9.0:tock_registers",
            "//external/vendor/zerocopy-0.8.27:zerocopy",
            "//kernel/hal:blueos_hal",
          ] + board_deps
+  proc_macro_deps = [ "//kernel/macro:blueos_macro" ]
 
   features = []
   rustflags = [

driver/src/systimer/esp32_sys_timer.rs

@@ -192,7 +192,9 @@ impl<const BASE_ADDR: usize, const HZ: u64> Clock for Esp32SysTimer<BASE_ADDR, H
     // FIXME: A stress test should be added to verify whether this API is stalled in multi-task.
     fn estimate_current_cycles() -> u64 {
         Self::registers().unit0_op.modify(UNIT_OP::UPDATE::SET);
-        while !Self::registers().unit0_op.is_set(UNIT_OP::VALUE_VALID) {}
+        while !Self::registers().unit0_op.is_set(UNIT_OP::VALUE_VALID) {
+            Self::registers().unit0_op.modify(UNIT_OP::UPDATE::SET);
+        }
 
         let mut lo_prev = Self::registers()
             .unit0_value_lo
@@ -220,16 +222,16 @@ impl<const BASE_ADDR: usize, const HZ: u64> Clock for Esp32SysTimer<BASE_ADDR, H
         // Fixme: Alarm cannot be triggered on esp32c3 when the target is too old on the qemu.
         // So we need to add a compensation here to make sure the target is always in the future.
         // See https://github.com/espressif/qemu/issues/69
-        let moment = {
-            let now = Self::estimate_current_cycles();
-            let compensation = core::cmp::max(1, HZ / 2_00); // ~5ms
-            let moment = if moment < now + compensation {
-                now.saturating_add(compensation)
-            } else {
-                moment
-            };
-            moment
-        };
+        // let moment = {
+        //     let now = Self::estimate_current_cycles();
+        //     let compensation = core::cmp::max(1, HZ / 2_00); // ~5ms
+        //     let moment = if moment < now + compensation {
+        //         now.saturating_add(compensation)
+        //     } else {
+        //         moment
+        //     };
+        //     moment
+        // };
         Self::set_comparator_enable(false);
         Self::clear_interrupt();
         Self::registers()

kernel/src/boards/raspberry_pico2_cortexm/handler.rs → hal/src/isr.rs

@@ -4,34 +4,20 @@
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
-//      http://www.apache.org/licenses/LICENSE-2.0
+//       http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::{
-    arch::{
-        self,
-        irq::{InterruptTable, Vector, INTERRUPT_TABLE_LEN},
-    },
-    boot::_start,
-    time,
-};
-
-unsafe extern "C" {
-    fn uart0_handler();
+pub trait IsrDesc: Sync + 'static {
+    fn service_isr(&self);
 }
 
-#[doc(hidden)]
-#[link_section = ".interrupt.handlers"]
-#[no_mangle]
-static __INTERRUPT_HANDLERS__: InterruptTable = {
-    let mut tbl = [Vector { reserved: 0 }; INTERRUPT_TABLE_LEN];
-    tbl[33] = Vector {
-        handler: uart0_handler,
-    };
-    tbl
-};
+#[repr(C)]
+pub struct IsrReg {
+    pub no: usize,
+    pub desc: &'static dyn IsrDesc,
+}

hal/src/lib.rs

@@ -23,6 +23,7 @@ use err::Result;
 pub mod clock;
 pub mod clock_control;
 pub mod i2c;
+pub mod isr;
 pub mod pinctrl;
 pub mod reset;
 pub mod uart;

kernel/src/arch/arm/irq.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use blueos_hal::isr::{IsrDesc, IsrReg};
 use cortex_m::{interrupt::InterruptNumber, peripheral::scb::SystemHandler, Peripherals};
 
 #[cfg(irq_priority_bits_2)]
@@ -114,50 +115,88 @@ pub union Vector {
     pub reserved: usize,
 }
 
+pub const INTERRUPT_TABLE_LEN: usize = blueos_kconfig::CONFIG_NUM_IRQS as usize;
+
 /// Interrupt vector table configuration for ARM Cortex-M processors.
 ///
-/// Users must define their own `__INTERRUPTS` based on their specific device requirements.
-/// The interrupt vector table should be placed in the `.vector_table.interrupts` section.
-///
-/// # Example
-///
-/// ```rust
-///
-/// #[used]
-/// #[link_section = ".interrupt.vectors"]
-/// #[no_mangle]
-/// pub static __INTERRUPT_HANDLERS__: InterruptTable = {
-///     let mut tbl = [Vector { reserved: 0 }; INTERRUPT_TABLE_LEN];
-///     tbl[0] = Vector {
-///         handler: uart0rx_handler,
-///     };
-///     tbl[1] = Vector {
-///         handler: uart0tx_handler,
-///     };
-///     tbl[2] = Vector {
-///         handler: uart1rx_handler,
-///     };
-///     tbl
-/// };
-///
-/// // Declare external interrupt handlers
-/// extern "C" {
-///     fn uart0rx_handler();
-///     fn uart0tx_handler();
-///     fn uart1rx_handler();
-/// }
-/// ```
-///
-/// # Architecture-specific Details
-///
-/// Maximum number of device-specific interrupts for different ARM Cortex-M architectures:
-/// - ARMv6-M: 32 interrupts
-/// - ARMv7-M/ARMv7E-M: 240 interrupts
-/// - ARMv8-M: 496 interrupts
+/// There are two types of ISRs, one is the RAW type, which is no different
+/// from general interrupt service handling. The other is the more flexible
+/// SWI type. SWI type interrupt service handling implements the trait object
+/// IsrDesc. For some complex processing scenarios, consider using IsrDesc
+/// to encapsulate relevant data, such as Shared ISR, Async ISR, Nested ISR,
+/// and so on.
 ///
 /// # Safety
 ///
 /// The interrupt vector table must be properly aligned and contain valid function pointers
 /// for all used interrupt vectors. Incorrect configuration may lead to undefined behavior.
-pub const INTERRUPT_TABLE_LEN: usize = blueos_kconfig::CONFIG_NUM_IRQS as usize;
-pub type InterruptTable = [Vector; blueos_kconfig::CONFIG_NUM_IRQS as usize];
+#[used]
+#[link_section = ".interrupt.handlers"]
+static mut __INTERRUPT_HANDLERS__: [Vector; blueos_kconfig::CONFIG_NUM_IRQS as usize] = [Vector {
+    handler: _generic_isr_handler,
+};
+    INTERRUPT_TABLE_LEN];
+
+extern "C" fn _generic_isr_handler() {
+    use cortex_m::peripheral::NVIC;
+    // Get the current ISR index from the IPSR register
+    let ipsr: u32;
+    unsafe {
+        core::arch::asm!("mrs {}, ipsr", out(reg) ipsr, options(nomem, nostack, preserves_flags));
+    }
+    let isr_index = (ipsr & 0x1FF)
+        .checked_sub(16)
+        .expect("Invalid ISR index, IPSR value: {ipsr:#X}");
+
+    if let Some(isr_desc) = unsafe { ISR_DESC[isr_index as usize].as_ref() } {
+        isr_desc.service_isr();
+    } else {
+        // FIXME: If the ISR is not explicitly registered, what should be done?
+    }
+
+    #[cfg(round_robin)]
+    {
+        // If the scheduler is preemptive, trigger PendSV to perform
+        // a context switch after handling the current interrupt.
+        cortex_m::peripheral::SCB::set_pendsv();
+    }
+}
+
+static mut ISR_DESC: [Option<&dyn IsrDesc>; INTERRUPT_TABLE_LEN] = [None; INTERRUPT_TABLE_LEN];
+
+/// Safety: ISR_DESC only be read in the interrupt handler,
+/// and only be written in the boot process early, so it's
+/// safe to use unsafe to write it.
+pub fn init_interrupt_registry() {
+    extern "C" {
+        static __isr_array_start: usize;
+        static __isr_array_end: usize;
+    }
+
+    unsafe {
+        let mut p = core::ptr::addr_of!(__isr_array_start);
+        while p < core::ptr::addr_of!(__isr_array_end) {
+            let r = &*(p as *const IsrReg);
+            assert!(
+                r.no < INTERRUPT_TABLE_LEN,
+                "ISR number {} exceeds the maximum limit {}",
+                r.no,
+                INTERRUPT_TABLE_LEN
+            );
+            ISR_DESC[r.no] = Some(r.desc);
+            p = p.add(1);
+        }
+    }
+}
+
+/// This function is used to register the raw interrupt handler for the given irq number.
+/// The handler should be defined in the assembly file, and the caller should ensure that
+/// the handler is properly defined and linked. This function is unsafe because it allows
+/// registering a raw interrupt handler, which may lead to undefined behavior if not used
+/// correctly.
+/// Safety: race condition may occur if this function is called while the corresponding
+/// interrupt is enabled and can be triggered, so the caller should ensure that the interrupt
+/// is disabled before calling this function, and enable it after the handler is registered.
+pub unsafe fn register_raw_isr(irq: IrqNumber, handler: unsafe extern "C" fn()) {
+    __INTERRUPT_HANDLERS__[irq.0 as usize] = Vector { handler };
+}

kernel/src/boards/gd32e507_eval/mod.rs

@@ -13,7 +13,6 @@
 // limitations under the License.
 
 mod config;
-mod handler;
 
 use crate::{
     arch, arch::irq::IrqNumber, boot, boot::INIT_BSS_DONE, devices::clock::systick, irq::IrqTrace,
@@ -77,13 +76,19 @@ const HZ: usize = config::PLL_SYS_FREQ as usize;
 pub type ClockImpl = systick::SysTickClock<TICKS_PS, HZ>;
 
 pub(crate) fn init() {
-    unsafe { copy_data() };
+    unsafe {
+        copy_data();
+        arch::irq::init_interrupt_registry();
+    };
     boot::init_runtime();
     use blueos_hal::clock_control::ClockControl;
     blueos_driver::clock_control::gd32_clock_control::Gd32ClockControl::init();
 
     unsafe { boot::init_heap() };
     arch::irq::init();
+    unsafe {
+        arch::irq::register_raw_isr(config::USBFS_IRQn, uart0_handler);
+    }
     arch::irq::enable_irq_with_priority(config::USBFS_IRQn, arch::irq::Priority::Normal);
     ClockImpl::init();
 }
@@ -123,8 +128,7 @@ crate::define_pin_states! {
     )
 }
 
-#[no_mangle]
-pub unsafe extern "C" fn uart0_handler() {
+unsafe extern "C" fn uart0_handler() {
     let _trace = IrqTrace::new(config::USBFS_IRQn);
     use blueos_hal::HasInterruptReg;
     let uart = get_device!(console_uart);