From f627086366b6d106a58556606f22294bd336f9f7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 27 Apr 2026 13:45:46 +0200 Subject: [PATCH 1/3] rewrite NVIC to use derive-mmio --- cortex-m/Cargo.toml | 1 + cortex-m/src/peripheral/mod.rs | 44 +++++--- cortex-m/src/peripheral/nvic.rs | 171 ++++++++++++++++++++------------ cortex-m/src/peripheral/test.rs | 20 ++-- 4 files changed, 144 insertions(+), 92 deletions(-) diff --git a/cortex-m/Cargo.toml b/cortex-m/Cargo.toml index 358ba721..7b617f11 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.8" 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 d69ea0aa..d18a010d 100644 --- a/cortex-m/src/peripheral/mod.rs +++ b/cortex-m/src/peripheral/mod.rs @@ -131,7 +131,7 @@ pub struct Peripherals { pub MPU: MPU, /// Nested Vector Interrupt Controller - pub NVIC: NVIC, + pub NVIC: nvic::OwnedRegisterBlock<{ nvic::BASE_ADDRESS }>, /// Security Attribution Unit pub SAU: SAU, @@ -213,9 +213,7 @@ impl Peripherals { MPU: MPU { _marker: PhantomData, }, - NVIC: NVIC { - _marker: PhantomData, - }, + NVIC: nvic::OwnedRegisterBlock::new(), SAU: SAU { _marker: PhantomData, }, @@ -547,30 +545,44 @@ 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 _; + /// Create a new instance of the NVIC driver. + /// + /// This API consumes the peripheral singleton instance. + #[inline] + pub const fn new(_resource: nvic::OwnedRegisterBlock<{ nvic::BASE_ADDRESS }>) -> Self { + unsafe { Self::steal() } + } - /// 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 const 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 21066c46..1d783407 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)) } } /// Route `interrupt` to the Non-Secure world (ARMv8-M only). @@ -275,7 +311,9 @@ impl NVIC { let nr = interrupt.number(); let group_idx = usize::from(nr / 32); let bit_mask = 1 << (nr % 32); - unsafe { self.itns[group_idx].modify(|v| v | bit_mask) } + + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { self.modify_itns_unchecked(group_idx, |val| val | bit_mask) }; } /// Route `interrupt` back to the Secure world (ARMv8-M only). @@ -294,7 +332,8 @@ impl NVIC { let nr = interrupt.number(); let group_idx = usize::from(nr / 32); let bit_mask = 1 << (nr % 32); - unsafe { self.itns[group_idx].modify(|v| v & !bit_mask) } + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { self.modify_itns_unchecked(group_idx, |val| val & !bit_mask) }; } /// Returns `true` if `interrupt` is routed to the Non-Secure world (ARMv8-M only). @@ -308,7 +347,9 @@ impl NVIC { let group_idx = usize::from(nr / 32); let bit_mask = 1 << (nr % 32); // NOTE(unsafe) atomic read with no side effects - unsafe { ((*Self::PTR).itns[group_idx].read() & bit_mask) == bit_mask } + let nvic = unsafe { Self::steal() }; + // SAFETY: InterruptNumber is an unsafe trait, we can assume correct implementation. + unsafe { nvic.read_itns_unchecked(group_idx) & bit_mask == bit_mask } } #[cfg(armv6m)] diff --git a/cortex-m/src/peripheral/test.rs b/cortex-m/src/peripheral/test.rs index f22ee4d5..4aed8769 100644 --- a/cortex-m/src/peripheral/test.rs +++ b/cortex-m/src/peripheral/test.rs @@ -109,20 +109,18 @@ fn mpu() { #[test] fn nvic() { - let nvic = unsafe { &*crate::peripheral::NVIC::PTR }; + let nvic = unsafe { crate::peripheral::NVIC::steal() }; - 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.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); #[cfg(armv8m)] - assert_eq!(address(&nvic.itns), 0xE000_E380); - #[cfg(armv8m)] - assert_eq!(address(&nvic.itns[1]), 0xE000_E384); - assert_eq!(address(&nvic.ipr), 0xE000E400); + assert_eq!(address(nvic.pointer_to_itns_start()), 0xE000_E380); + 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] From 2a16f581510dd893ee6fc73e7cad395d243d29c9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 29 Jul 2026 11:27:19 +0200 Subject: [PATCH 2/3] move NVIC to nvic module --- cortex-m/src/peripheral/mod.rs | 44 ++------------------------------- cortex-m/src/peripheral/nvic.rs | 43 +++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/cortex-m/src/peripheral/mod.rs b/cortex-m/src/peripheral/mod.rs index d18a010d..e99add61 100644 --- a/cortex-m/src/peripheral/mod.rs +++ b/cortex-m/src/peripheral/mod.rs @@ -87,6 +87,8 @@ pub mod tpiu; #[cfg(test)] mod test; +pub use nvic::NVIC; + // NOTE the `PhantomData` used in the peripherals proxy is to make them `Send` but *not* `Sync` /// Core peripherals @@ -544,48 +546,6 @@ impl ops::Deref for MPU { } } -/// Nested Vector Interrupt Controller -pub struct NVIC(nvic::MmioRegisterBlock<'static>); - -unsafe impl Send for NVIC {} - -impl NVIC { - /// Create a new instance of the NVIC driver. - /// - /// This API consumes the peripheral singleton instance. - #[inline] - pub const fn new(_resource: nvic::OwnedRegisterBlock<{ nvic::BASE_ADDRESS }>) -> Self { - unsafe { Self::steal() } - } - - /// 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 const unsafe fn steal() -> Self { - NVIC(unsafe { nvic::RegisterBlock::new_mmio_fixed() }) - } -} - -impl ops::Deref for NVIC { - type Target = nvic::MmioRegisterBlock<'static>; - - #[inline(always)] - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl ops::DerefMut for NVIC { - #[inline(always)] - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } -} - /// Security Attribution Unit pub struct SAU { _marker: PhantomData<*const ()>, diff --git a/cortex-m/src/peripheral/nvic.rs b/cortex-m/src/peripheral/nvic.rs index 1d783407..11eeab7d 100644 --- a/cortex-m/src/peripheral/nvic.rs +++ b/cortex-m/src/peripheral/nvic.rs @@ -1,10 +1,51 @@ //! Nested Vector Interrupt Controller use crate::interrupt::InterruptNumber; -use crate::peripheral::NVIC; /// NVIC base address. pub const BASE_ADDRESS: usize = 0xE000_E100; +/// Nested Vector Interrupt Controller +pub struct NVIC(MmioRegisterBlock<'static>); + +unsafe impl Send for NVIC {} + +impl NVIC { + /// Create a new instance of the NVIC driver. + /// + /// This API consumes the peripheral singleton instance. + #[inline] + pub const fn new(_resource: OwnedRegisterBlock<{ BASE_ADDRESS }>) -> Self { + unsafe { Self::steal() } + } + + /// 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 const unsafe fn steal() -> Self { + NVIC(unsafe { RegisterBlock::new_mmio_fixed() }) + } +} + +impl core::ops::Deref for NVIC { + type Target = MmioRegisterBlock<'static>; + + #[inline(always)] + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl core::ops::DerefMut for NVIC { + #[inline(always)] + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + /// NVIC register block. #[derive(derive_mmio::Mmio)] #[repr(C)] From b12192425b8f78d8fe1032dfca74dd9d5a7ed655 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 29 Jul 2026 11:33:02 +0200 Subject: [PATCH 3/3] docs correction --- cortex-m/src/peripheral/nvic.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cortex-m/src/peripheral/nvic.rs b/cortex-m/src/peripheral/nvic.rs index 11eeab7d..e77a621d 100644 --- a/cortex-m/src/peripheral/nvic.rs +++ b/cortex-m/src/peripheral/nvic.rs @@ -12,9 +12,10 @@ unsafe impl Send for NVIC {} impl NVIC { /// Create a new instance of the NVIC driver. /// - /// This API consumes the peripheral singleton instance. + /// This API consumes the NVIC field of the [super::Peripherals] instance. #[inline] pub const fn new(_resource: OwnedRegisterBlock<{ BASE_ADDRESS }>) -> Self { + // Safety: NVIC field of singleton is consumed here to verify ownership transfer. unsafe { Self::steal() } }