From 624d62fd7aa2c5735601adc8c13a8617019aa297 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 27 Apr 2026 13:45:46 +0200 Subject: [PATCH] rewrite NVIC to use derive-mmio --- cortex-m/Cargo.toml | 1 + cortex-m/src/peripheral/mod.rs | 36 +++---- cortex-m/src/peripheral/nvic.rs | 160 +++++++++++++++++++------------- cortex-m/src/peripheral/test.rs | 18 ++-- testsuite/Cargo.toml | 11 ++- testsuite/src/main.rs | 63 ++++++++++++- 6 files changed, 196 insertions(+), 93 deletions(-) diff --git a/cortex-m/Cargo.toml b/cortex-m/Cargo.toml index 74ce9f35f..49beae0db 100644 --- a/cortex-m/Cargo.toml +++ b/cortex-m/Cargo.toml @@ -15,6 +15,7 @@ rust-version = "1.85" [dependencies] bare-metal = { version = "0.2.4", features = ["const-fn"] } critical-section = "1.0.0" +derive-mmio = "0.6" volatile-register = "0.2.2" bitfield = "0.13.2" eh0 = { package = "embedded-hal", version = "0.2.4" } diff --git a/cortex-m/src/peripheral/mod.rs b/cortex-m/src/peripheral/mod.rs index d65707c14..e7cabc616 100644 --- a/cortex-m/src/peripheral/mod.rs +++ b/cortex-m/src/peripheral/mod.rs @@ -206,9 +206,7 @@ impl Peripherals { MPU: MPU { _marker: PhantomData, }, - NVIC: NVIC { - _marker: PhantomData, - }, + NVIC: NVIC::steal(), SAU: SAU { _marker: PhantomData, }, @@ -536,30 +534,36 @@ impl ops::Deref for MPU { } /// Nested Vector Interrupt Controller -pub struct NVIC { - _marker: PhantomData<*const ()>, -} +pub struct NVIC(nvic::MmioRegisterBlock<'static>); unsafe impl Send for NVIC {} impl NVIC { - /// Pointer to the register block - pub const PTR: *const nvic::RegisterBlock = 0xE000_E100 as *const _; - - /// Returns a pointer to the register block - #[inline(always)] - #[deprecated(since = "0.7.5", note = "Use the associated constant `PTR` instead")] - pub const fn ptr() -> *const nvic::RegisterBlock { - Self::PTR + /// Unsafely steal an instance of the NVIC. + /// + /// # Safety + /// + /// This potentially allows to create multiple instances of the NVIC register block, which + /// might only be valid in certain multi-core environments. + #[inline] + pub unsafe fn steal() -> Self { + NVIC(unsafe { nvic::RegisterBlock::new_mmio_fixed() }) } } impl ops::Deref for NVIC { - type Target = self::nvic::RegisterBlock; + type Target = nvic::MmioRegisterBlock<'static>; #[inline(always)] fn deref(&self) -> &Self::Target { - unsafe { &*Self::PTR } + &self.0 + } +} + +impl ops::DerefMut for NVIC { + #[inline(always)] + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 } } diff --git a/cortex-m/src/peripheral/nvic.rs b/cortex-m/src/peripheral/nvic.rs index 60411f0e7..b2c82d8ce 100644 --- a/cortex-m/src/peripheral/nvic.rs +++ b/cortex-m/src/peripheral/nvic.rs @@ -1,38 +1,38 @@ //! Nested Vector Interrupt Controller - -use volatile_register::RW; -#[cfg(not(armv6m))] -use volatile_register::{RO, WO}; - use crate::interrupt::InterruptNumber; use crate::peripheral::NVIC; -/// Register block +/// NVIC base address. +pub const BASE_ADDRESS: usize = 0xE000_E100; + +/// NVIC register block. +#[derive(derive_mmio::Mmio)] #[repr(C)] pub struct RegisterBlock { /// Interrupt Set-Enable - pub iser: [RW; 16], + iser: [u32; 16], _reserved0: [u32; 16], /// Interrupt Clear-Enable - pub icer: [RW; 16], + icer: [u32; 16], _reserved1: [u32; 16], /// Interrupt Set-Pending - pub ispr: [RW; 16], + ispr: [u32; 16], _reserved2: [u32; 16], /// Interrupt Clear-Pending - pub icpr: [RW; 16], + icpr: [u32; 16], _reserved3: [u32; 16], /// Interrupt Active Bit (not present on Cortex-M0 variants) #[cfg(not(armv6m))] - pub iabr: [RO; 16], + #[mmio(PureRead)] + iabr: [u32; 16], #[cfg(armv6m)] _reserved4: [u32; 16], @@ -40,7 +40,7 @@ pub struct RegisterBlock { #[cfg(armv8m)] /// Interrupt Target Non-secure (only present on Arm v8-M) - pub itns: [RW; 16], + itns: [u32; 16], #[cfg(not(armv8m))] _reserved6: [u32; 16], @@ -50,36 +50,46 @@ pub struct RegisterBlock { /// /// On ARMv7-M, 124 word-sized registers are available. Each of those /// contains of 4 interrupt priorities of 8 byte each.The architecture - /// specifically allows accessing those along byte boundaries, so they are - /// represented as 496 byte-sized registers, for convenience, and to allow - /// atomic priority updates. + /// specifically allows accessing those along byte boundaries. /// /// On ARMv6-M, the registers must only be accessed along word boundaries, /// so convenient byte-sized representation wouldn't work on that /// architecture. #[cfg(not(armv6m))] - pub ipr: [RW; 496], + ipr: [u32; 124], /// Interrupt Priority /// /// On ARMv7-M, 124 word-sized registers are available. Each of those /// contains of 4 interrupt priorities of 8 byte each.The architecture - /// specifically allows accessing those along byte boundaries, so they are - /// represented as 496 byte-sized registers, for convenience, and to allow - /// atomic priority updates. + /// specifically allows accessing those along byte boundaries. /// /// On ARMv6-M, the registers must only be accessed along word boundaries, /// so convenient byte-sized representation wouldn't work on that /// architecture. #[cfg(armv6m)] - pub ipr: [RW; 8], + ipr: [u32; 8], #[cfg(not(armv6m))] _reserved8: [u32; 580], /// Software Trigger Interrupt #[cfg(not(armv6m))] - pub stir: WO, + #[mmio(Write)] + stir: u32, +} + +impl RegisterBlock { + /// Creates a new instance of the NVIC register block. + /// + /// # Safety + /// + /// This potentially allows to create multiple instances of the NVIC register block, which + /// might only be valid in certain multi-core environments. + #[inline] + pub const unsafe fn new_mmio_fixed() -> MmioRegisterBlock<'static> { + unsafe { RegisterBlock::new_mmio_at(BASE_ADDRESS) } + } } impl NVIC { @@ -100,9 +110,7 @@ impl NVIC { { let nr = interrupt.number(); - unsafe { - self.stir.write(u32::from(nr)); - } + self.write_stir(u32::from(nr)); } /// Disables `interrupt` @@ -112,8 +120,10 @@ impl NVIC { I: InterruptNumber, { let nr = interrupt.number(); - // NOTE(unsafe) this is a write to a stateless register - unsafe { (*Self::PTR).icer[usize::from(nr / 32)].write(1 << (nr % 32)) } + // SAFETY: this is a write to a stateless register. + let mut nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { nvic.write_icer_unchecked(usize::from(nr / 32), 1 << (nr % 32)) } } /// Enables `interrupt` @@ -124,18 +134,20 @@ impl NVIC { where I: InterruptNumber, { - unsafe { - let nr = interrupt.number(); - // NOTE(ptr) this is a write to a stateless register - (*Self::PTR).iser[usize::from(nr / 32)].write(1 << (nr % 32)) - } + let nr = interrupt.number(); + // SAFETY: this is a write to a stateless register, + let mut nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { nvic.write_iser_unchecked(usize::from(nr / 32), 1 << (nr % 32)) } } /// Returns the NVIC priority of `interrupt` /// - /// *NOTE* NVIC encodes priority in the highest bits of a byte so values like `1` and `2` map - /// to the same priority. Also for NVIC priorities, a lower value (e.g. `16`) has higher - /// priority (urgency) than a larger value (e.g. `32`). + /// *NOTE* The NVIC encodes priorities in the *most-significant* bits of the 8-bit block for + /// each interrupt. This means that the priority value retrieved by this function MUST be + /// right-shifted by (8 - NUMBER_OF_PRIORITY_BITS), where NUMBER_OF_PRIORITY_BITS can be + /// different between cores. Also for NVIC priorities, a lower value (e.g. `0b0000_0000`) has + /// higher priority (urgency) than a larger value (e.g. `0b0100_0000`). #[inline] pub fn get_priority(interrupt: I) -> u8 where @@ -144,16 +156,22 @@ impl NVIC { #[cfg(not(armv6m))] { let nr = interrupt.number(); - // NOTE(unsafe) atomic read with no side effects - unsafe { (*Self::PTR).ipr[usize::from(nr)].read() } + // SAFETY: atomic read with no side effects + let nvic = unsafe { Self::steal() }; + let ipr_ptr = nvic.pointer_to_ipr_start() as *const u8; + // SAFETY: + // - atomic read with no side effects + // - InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { core::ptr::read_volatile(ipr_ptr.offset(nr as isize)) } } #[cfg(armv6m)] { - // NOTE(unsafe) atomic read with no side effects - let ipr_n = unsafe { (*Self::PTR).ipr[Self::ipr_index(interrupt)].read() }; - let prio = (ipr_n >> Self::ipr_shift(interrupt)) & 0x0000_00ff; - prio as u8 + // SAFETY: atomic read with no side effects + let nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + let ipr_n = unsafe { nvic.read_ipr_unchecked(Self::ipr_index(interrupt)) }; + ((ipr_n >> Self::ipr_shift(interrupt)) & 0x0000_00ff) as u8 } } @@ -167,8 +185,10 @@ impl NVIC { let nr = interrupt.number(); let mask = 1 << (nr % 32); - // NOTE(unsafe) atomic read with no side effects - unsafe { ((*Self::PTR).iabr[usize::from(nr / 32)].read() & mask) == mask } + // SAFETY: atomic read with no side effects + let nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { nvic.read_iabr_unchecked(usize::from(nr / 32)) & mask == mask } } /// Checks if `interrupt` is enabled @@ -180,8 +200,10 @@ impl NVIC { let nr = interrupt.number(); let mask = 1 << (nr % 32); - // NOTE(unsafe) atomic read with no side effects - unsafe { ((*Self::PTR).iser[usize::from(nr / 32)].read() & mask) == mask } + // SAFETY: atomic read with no side effects + let nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { nvic.read_iser_unchecked(usize::from(nr / 32)) & mask == mask } } /// Checks if `interrupt` is pending @@ -193,8 +215,10 @@ impl NVIC { let nr = interrupt.number(); let mask = 1 << (nr % 32); - // NOTE(unsafe) atomic read with no side effects - unsafe { ((*Self::PTR).ispr[usize::from(nr / 32)].read() & mask) == mask } + // SAFETY: atomic read with no side effects + let nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { nvic.read_ispr_unchecked(usize::from(nr / 32)) & mask == mask } } /// Forces `interrupt` into pending state @@ -205,14 +229,19 @@ impl NVIC { { let nr = interrupt.number(); - // NOTE(unsafe) atomic stateless write; ICPR doesn't store any state - unsafe { (*Self::PTR).ispr[usize::from(nr / 32)].write(1 << (nr % 32)) } + // SAFETY: atomic stateless write; Register doesn't store any state + let mut nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { nvic.write_ispr_unchecked(usize::from(nr / 32), 1 << (nr % 32)) } } /// Sets the "priority" of `interrupt` to `prio` /// - /// *NOTE* See [`get_priority`](struct.NVIC.html#method.get_priority) method for an explanation - /// of how NVIC priorities work. + /// *NOTE* The NVIC encodes priorities in the *most-significant* bits of the 8-bit block for + /// each interrupt. This means that the priority value passed to this function MUST be shifted + /// by (8 - NUMBER_OF_PRIORITY_BITS), where NUMBER_OF_PRIORITY_BITS can be different between + /// cores. Also for NVIC priorities, a lower value (e.g. `0b0000_0000`) has higher + /// priority (urgency) than a larger value (e.g. `0b0010_0000`). /// /// On ARMv6-M, updating an interrupt priority requires a read-modify-write operation. On /// ARMv7-M, the operation is performed in a single atomic write operation. @@ -226,21 +255,26 @@ impl NVIC { where I: InterruptNumber, { - unsafe { - #[cfg(not(armv6m))] - { - let nr = interrupt.number(); - self.ipr[usize::from(nr)].write(prio) - } + #[cfg(not(armv6m))] + { + let nr = interrupt.number(); + let ipr_ptr = self.pointer_to_ipr_start() as *mut u8; + // SAFETY: + // - atomic stateless write; IPR doesn't store any state + // - InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { core::ptr::write_volatile(ipr_ptr.offset(nr as isize), prio) } + } - #[cfg(armv6m)] - { - self.ipr[Self::ipr_index(interrupt)].modify(|value| { + #[cfg(armv6m)] + { + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { + self.modify_ipr_unchecked(Self::ipr_index(interrupt), |value| { let mask = 0x0000_00ff << Self::ipr_shift(interrupt); let prio = u32::from(prio) << Self::ipr_shift(interrupt); (value & !mask) | prio - }) + }); } } } @@ -253,8 +287,10 @@ impl NVIC { { let nr = interrupt.number(); - // NOTE(unsafe) atomic stateless write; ICPR doesn't store any state - unsafe { (*Self::PTR).icpr[usize::from(nr / 32)].write(1 << (nr % 32)) } + // SAFETY: atomic stateless write; ICPR doesn't store any state + let mut nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { nvic.write_icpr_unchecked(usize::from(nr / 32), 1 << (nr % 32)) } } #[cfg(armv6m)] diff --git a/cortex-m/src/peripheral/test.rs b/cortex-m/src/peripheral/test.rs index cab064aad..f978f7441 100644 --- a/cortex-m/src/peripheral/test.rs +++ b/cortex-m/src/peripheral/test.rs @@ -109,16 +109,16 @@ fn mpu() { #[test] fn nvic() { - let nvic = unsafe { &*crate::peripheral::NVIC::PTR }; - - assert_eq!(address(&nvic.iser), 0xE000E100); - assert_eq!(address(&nvic.icer), 0xE000E180); - assert_eq!(address(&nvic.ispr), 0xE000E200); - assert_eq!(address(&nvic.icpr), 0xE000E280); - assert_eq!(address(&nvic.iabr), 0xE000E300); - assert_eq!(address(&nvic.ipr), 0xE000E400); + let nvic = unsafe { crate::peripheral::NVIC::steal() }; + + assert_eq!(address(nvic.pointer_to_iser_start()), 0xE000E100); + assert_eq!(address(nvic.pointer_to_icer_start()), 0xE000E180); + assert_eq!(address(nvic.pointer_to_ispr_start()), 0xE000E200); + assert_eq!(address(nvic.pointer_to_icpr_start()), 0xE000E280); + assert_eq!(address(nvic.pointer_to_iabr_start()), 0xE000E300); + assert_eq!(address(nvic.pointer_to_ipr_start()), 0xE000E400); #[cfg(not(armv6m))] - assert_eq!(address(&nvic.stir), 0xE000EF00); + assert_eq!(address(nvic.pointer_to_stir()), 0xE000EF00); } #[test] diff --git a/testsuite/Cargo.toml b/testsuite/Cargo.toml index 84661f0ea..a6f8c2bdf 100644 --- a/testsuite/Cargo.toml +++ b/testsuite/Cargo.toml @@ -9,9 +9,10 @@ rust-version = "1.85" rtt = ["rtt-target", "minitest/rtt"] [dependencies] -cortex-m-rt.path = "../cortex-m-rt" +cortex-m-rt = { path = "../cortex-m-rt"} cortex-m = { path = "../cortex-m", features = ["critical-section-single-core"] } -minitest.path = "minitest" -critical-section = "1.0.0" -cortex-m-semihosting.path = "../cortex-m-semihosting" -rtt-target = { version = "0.5.0", optional = true } +minitest = { path = "minitest" } +critical-section = "1" +num_enum = { version = "0.7", default-features = false } +cortex-m-semihosting = { path = "../cortex-m-semihosting" } +rtt-target = { version = "0.6", optional = true } diff --git a/testsuite/src/main.rs b/testsuite/src/main.rs index 259c2f3af..34ca4a91a 100644 --- a/testsuite/src/main.rs +++ b/testsuite/src/main.rs @@ -20,6 +20,49 @@ const STACK_SIZE_WORDS: usize = 1024; static STACK: cortex_m::psp::Stack = cortex_m::psp::Stack::new(); +#[derive(Debug, Clone, Copy, num_enum::TryFromPrimitive)] +#[repr(u16)] +pub enum DummyInterrupts { + Dummy0 = 0, + Dummy1 = 1, + Dummy2 = 2, + Dummy3 = 3, + Dummy4 = 4, + Dummy5 = 5, + Dummy6 = 6, + Dummy7 = 7, + Dummy8 = 8, + Dummy9 = 9, + Dummy10 = 10, + Dummy11 = 11, + Dummy12 = 12, + Dummy13 = 13, + Dummy14 = 14, + Dummy15 = 15, + Dummy16 = 16, + Dummy17 = 17, + Dummy18 = 18, + Dummy19 = 19, + Dummy20 = 20, + Dummy21 = 21, + Dummy22 = 22, + Dummy23 = 23, + Dummy24 = 24, + Dummy25 = 25, + Dummy26 = 26, + Dummy27 = 27, + Dummy28 = 28, + Dummy29 = 29, + Dummy30 = 30, + Dummy31 = 31, +} + +unsafe impl cortex_m::interrupt::InterruptNumber for DummyInterrupts { + fn number(self) -> u16 { + self as u16 + } +} + #[cortex_m_rt::exception] fn PendSV() { minitest::log!("Hit PendSV!"); @@ -45,7 +88,8 @@ unsafe fn HardFault(frame: &cortex_m_rt::ExceptionFrame) -> ! { #[minitest::tests] mod tests { - use crate::{Ordering, PENDSV_FLAG}; + use crate::{DummyInterrupts, Ordering, PENDSV_FLAG}; + use cortex_m::peripheral::NVIC; use minitest::log; #[init] @@ -59,6 +103,23 @@ mod tests { assert!(cortex_m::Peripherals::take().is_none()); } + #[test] + fn test_priority_bits() { + let mut nvic = unsafe { NVIC::steal() }; + for i in 0..32 { + let int = DummyInterrupts::try_from(i).unwrap(); + // Priorities are encoded in the most-significant bits. 2 should always be implemented. + unsafe { nvic.set_priority(int, 0b11 << 6) }; + assert_eq!(NVIC::get_priority(int), 0b11 << 6); + // Set a different value. + unsafe { nvic.set_priority(int, 0b01 << 6) }; + assert_eq!(NVIC::get_priority(int), 0b01 << 6); + // Set zero again. + unsafe { nvic.set_priority(int, 0) }; + assert_eq!(NVIC::get_priority(int), 0); + } + } + #[test] #[cfg(feature = "rtt")] // QEMU does not model the cycle counter fn cycle_count(p: &mut cortex_m::Peripherals) {