Merge pull request 'improve GIC modules' (#31) from improve-gic-modules into main

Reviewed-on: #31

zynq/zynq7000-hal/CHANGELOG.md

@@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Increased UART type safety by providing dedicated MIO constructors for UART 0 and UART 1
   respectively.
+- Several bugfixes and improvements for GIC module. Some of the registers previously were
+  completely overwritten instead of only modifying their own bit portions. Also allow targeting
+  interrupts without clearing other CPU target.
 
 # [v0.1.1] 2025-10-10
 

zynq/zynq7000-hal/Cargo.toml

@@ -20,6 +20,7 @@ bitbybit = "1.4"
 arbitrary-int = "2"
 thiserror = { version = "2", default-features = false }
 num_enum = { version = "0.7", default-features = false }
+bitflags = "2"
 ringbuf = { version = "0.4.8", default-features = false }
 embedded-hal-nb = "1"
 embedded-io = "0.7"
@@ -40,6 +41,7 @@ smoltcp = { version = "0.12", default-features = false, features = ["proto-ipv4"
 vcell = "0.1"
 raw-slicee = "0.1"
 embedded-io-async = "0.7"
+serde = { version = "1", optional = true, features = ["derive"] }
 
 [features]
 std = ["thiserror/std", "alloc"]

zynq/zynq7000-hal/src/gic.rs

@@ -6,17 +6,22 @@
 //! # Examples
 //!
 //! - [GTC ticks](https://egit.irs.uni-stuttgart.de/rust/zynq7000-rs/src/branch/main/examples/simple/src/bin/gtc-ticks.rs)
+#![deny(missing_docs)]
 use arbitrary_int::prelude::*;
 
 use aarch32_cpu::interrupt;
 use zynq7000::gic::{
     CpuInterfaceRegisters, DistributorControlRegister, DistributorRegisters, InterfaceControl,
-    InterruptSignalRegister, MmioCpuInterfaceRegisters, MmioDistributorRegisters, PriorityRegister,
+    InterruptProcessorTargetRegister, InterruptSignalRegister, MmioCpuInterfaceRegisters,
+    MmioDistributorRegisters, PriorityRegister,
 };
 
-const SPURIOUS_INTERRUPT_ID: u32 = 1023;
+/// Spurious interrupt ID.
+pub const SPURIOUS_INTERRUPT_ID: u32 = 1023;
 
+/// Highest interrupt priority (smallest number).
 pub const HIGHEST_PRIORITY: u8 = 0;
+/// Lowest interrupt priority (largest number).
 pub const LOWEST_PRIORITY: u8 = 31;
 
 /// These fixed values must be programmed according to the Zynq7000 TRM p.236.
@@ -36,119 +41,218 @@ pub const ICFR_4_FIXED_VALUE: u32 = 0b01110101010101010101010101010101;
 pub const ICFR_5_FIXED_VALUE: u32 = 0b00000011010101010101010101010101;
 
 /// Helper value to target all interrupts which can be targetted to CPU 0
-pub const TARGETS_ALL_CPU_0_IPTR_VAL: u32 = 0x01010101;
+pub const TARGETS_ALL_CPU_0_IPTR_VAL: InterruptProcessorTargetRegister =
+    InterruptProcessorTargetRegister::new_with_raw_value(0x01010101);
 /// Helper value to target all interrupts which can be targetted to CPU 1
-pub const TARGETS_ALL_CPU_1_IPTR_VAL: u32 = 0x02020202;
+pub const TARGETS_ALL_CPU_1_IPTR_VAL: InterruptProcessorTargetRegister =
+    InterruptProcessorTargetRegister::new_with_raw_value(0x02020202);
 
+/// Mask for activating all softare generated interrupts.
 pub const ACTIVATE_ALL_SGIS_MASK_ISER: u32 = 0x0000_FFFF;
+/// Mask for activating all private peripheral interrupts.
 pub const ACTIVATE_ALL_PPIS_MASK_ISER: u32 = 0xF800_0000;
 
+/// Shared peripheral interrupt sensitivity.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum SpiSensitivity {
+    /// Level triggered interrupt.
     Level = 0b01,
+    /// Edge triggered interrupt.
     Edge = 0b11,
 }
 
-pub enum TargetCpu {
+/// CPU enumeration.
-    None = 0b00,
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-    Cpu0 = 0b01,
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-    Cpu1 = 0b10,
+pub enum Cpu {
-    Both = 0b11,
+    /// CPU 0.
+    Cpu0,
+    /// CPU 1.
+    Cpu1,
+}
+
+bitflags::bitflags! {
+    /// Target CPU bitflags.
+    #[derive(Debug, Eq, PartialEq, Clone, Copy)]
+    #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+    pub struct TargetCpus: u8 {
+        /// No CPU.
+        const NONE = 0b00;
+        /// CPU 0.
+        const CPU_0 = 0b01;
+        /// CPU 1.
+        const CPU_1 = 0b10;
+        /// Both CPUs.
+        const BOTH_CPUS = 0b11;
+    }
 }
 
 /// Private Peripheral Interrupt (PPI) which are private to the CPU.
 #[derive(Debug, Eq, PartialEq, Clone, Copy, num_enum::TryFromPrimitive)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[repr(u8)]
 pub enum PpiInterrupt {
+    /// Global timer.
     GlobalTimer = 27,
-    // Interrupt signal from the PL. CPU0: `IRQF2P[18]` and CPU1: `IRQF2P[19]`
+    /// Interrupt signal from the PL. CPU0: `IRQF2P[18]` and CPU1: `IRQF2P[19]`
     NFiq = 28,
+    /// CPU private timer.
     CpuPrivateTimer = 29,
     /// AWDT0 and AWDT1 for each CPU.
     Awdt = 30,
-    // Interrupt signal from the PL. CPU0: `IRQF2P[16]` and CPU1: `IRQF2P[17]`
+    /// Interrupt signal from the PL. CPU0: `IRQF2P[16]` and CPU1: `IRQF2P[17]`
     NIrq = 31,
 }
 
 /// Shared Peripheral Interrupt IDs.
 #[derive(Debug, Eq, PartialEq, Clone, Copy, num_enum::TryFromPrimitive)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[repr(u8)]
 pub enum SpiInterrupt {
+    /// CPU 0.
     Cpu0 = 32,
+    /// CPU 1.
     Cpu1 = 33,
+    /// L2 cache.
     L2Cache = 34,
+    /// On-chip memory.
     Ocm = 35,
+    /// Reserved.
     _Reserved0 = 36,
+    /// Performance monitor unit 0.
     Pmu0 = 37,
+    /// Performance monitor unit 1.
     Pmu1 = 38,
+    /// XADC.
     Xadc = 39,
+    /// Device configuration.
     DevC = 40,
+    /// System watchdog timer.
     Swdt = 41,
+    /// Triple timer counter 00.
     Ttc00 = 42,
+    /// Triple timer counter 01.
     Ttc01 = 43,
+    /// Triple timer counter 02.
     Ttc02 = 44,
+    /// DMAC abort.
     DmacAbort = 45,
+    /// DMAC 0.
     Dmac0 = 46,
+    /// DMAC 1.
     Dmac1 = 47,
+    /// DMAC 2.
     Dmac2 = 48,
+    /// DMAC 3.
     Dmac3 = 49,
+    /// Shared memory controller.
     Smc = 50,
+    /// Quad SPI.
     Qspi = 51,
+    /// GPIO.
     Gpio = 52,
+    /// USB 0.
     Usb0 = 53,
+    /// Ethernet 0.
     Eth0 = 54,
+    /// Ethernet 0 wakeup.
     Eth0Wakeup = 55,
+    /// SDIO 0.
     Sdio0 = 56,
+    /// I2C 0.
     I2c0 = 57,
+    /// SPI 0.
     Spi0 = 58,
+    /// UART 0.
     Uart0 = 59,
+    /// CAN 0.
     Can0 = 60,
+    /// Programmable Logic 0.
     Pl0 = 61,
+    /// Programmable Logic 1.
     Pl1 = 62,
+    /// Programmable Logic 2.
     Pl2 = 63,
+    /// Programmable Logic 3.
     Pl3 = 64,
+    /// Programmable Logic 4.
     Pl4 = 65,
+    /// Programmable Logic 5.
     Pl5 = 66,
+    /// Programmable Logic 6.
     Pl6 = 67,
+    /// Programmable Logic 7.
     Pl7 = 68,
+    /// Triple timer counter 10.
     Ttc10 = 69,
+    /// Triple timer counter 11.
     Ttc11 = 70,
+    /// Triple timer counter 12.
     Ttc12 = 71,
+    /// DMAC 4.
     Dmac4 = 72,
+    /// DMAC 5.
     Dmac5 = 73,
+    /// DMAC 6.
     Dmac6 = 74,
+    /// DMAC 7.
     Dmac7 = 75,
+    /// USB 1.
     Usb1 = 76,
+    /// Ethernet 1.
     Eth1 = 77,
+    /// Ethernet 1 wakeup.
     Eth1Wakeup = 78,
+    /// SDIO 1.
     Sdio1 = 79,
+    /// I2C 1.
     I2c1 = 80,
+    /// SPI 1.
     Spi1 = 81,
+    /// UART 1.
     Uart1 = 82,
+    /// CAN 1.
     Can1 = 83,
+    /// Programmable Logic 8.
     Pl8 = 84,
+    /// Programmable Logic 9.
     Pl9 = 85,
+    /// Programmable Logic 10.
     Pl10 = 86,
+    /// Programmable Logic 11.
     Pl11 = 87,
+    /// Programmable Logic 12.
     Pl12 = 88,
+    /// Programmable Logic 13.
     Pl13 = 89,
+    /// Programmable Logic 14.
     Pl14 = 90,
+    /// Programmable Logic 15.
     Pl15 = 91,
+    /// Snoop control unit parity.
     ScuParity = 92,
 }
 
 /// Interrupt ID wrapper.
 #[derive(Debug, Clone, Copy)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum Interrupt {
+    /// Software-generated interrupt (SGI).
     Sgi(usize),
+    /// Private peripheral interrupt (PPI).
     Ppi(PpiInterrupt),
+    /// Shared peripheral interrupt (SPI).
     Spi(SpiInterrupt),
     /// Detects an invalid interrupt ID.
     Invalid(usize),
-    /// Spurious interrupt (ID# 1023).
+    /// Spurious interrupt (ID# 1023, [SPURIOUS_INTERRUPT_ID]).
     Spurious,
 }
 
+/// Interrupt information structure.
 #[derive(Debug)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct InterruptInfo {
     raw_reg: InterruptSignalRegister,
     interrupt: Interrupt,
@@ -156,34 +260,46 @@ pub struct InterruptInfo {
 }
 
 impl InterruptInfo {
-    pub fn raw_reg(&self) -> InterruptSignalRegister {
+    /// Raw interrupt signal register value.
+    #[inline]
+    pub const fn raw_reg(&self) -> InterruptSignalRegister {
         self.raw_reg
     }
 
-    pub fn cpu_id(&self) -> u8 {
+    /// CPU ID.
+    #[inline]
+    pub const fn cpu_id(&self) -> u8 {
         self.cpu_id
     }
 
-    pub fn interrupt(&self) -> Interrupt {
+    /// Interrupt ID.
+    #[inline]
+    pub const fn interrupt(&self) -> Interrupt {
         self.interrupt
     }
 }
 
+/// Invalid priority value error.
 #[derive(Debug, thiserror::Error)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[error("Invalid priority value {0}, range is [0, 31]")]
 pub struct InvalidPriorityValue(pub u8);
 
+/// Invalid programmable logic interrupt ID.
 #[derive(Debug, thiserror::Error)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[error("Invalid PL interrupt ID {0}")]
 pub struct InvalidPlInterruptId(pub usize);
 
 /// Invalid Shared Peripheral Interrupt (SPI) ID.
 #[derive(Debug, thiserror::Error)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[error("Invalid SPI interrupt ID {0}")]
 pub struct InvalidSpiInterruptId(pub usize);
 
 /// Invalid Software Generated Interrupt (SGI) ID.
 #[derive(Debug, thiserror::Error)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[error("Invalid SGI interrupt ID {0}")]
 pub struct InvalidSgiInterruptId(pub usize);
 
@@ -202,7 +318,8 @@ pub struct InvalidSgiInterruptId(pub usize);
 ///    the [SpiSensitivity] enum. You can use the following (helper) API to configure the
 ///    interrupts:
 ///
-///     - [Self::set_spi_interrupt_cpu_target]
+///     - [Self::set_spi_interrupt_target_for_cpu]
+///     - [Self::set_spi_interrupt_cpu_target_flags]
 ///     - [Self::set_all_spi_interrupt_targets_cpu0]
 ///     - [Self::set_pl_interrupt_sensitivity]
 ///
@@ -219,12 +336,14 @@ pub struct InvalidSgiInterruptId(pub usize);
 ///    You might also chose to enable these interrupts at run-time after the GIC was started.
 /// 4. Start the GIC by calling [Self::update_ctrl_regs] with the required settings or
 ///    with [Self::enable] which assumes a certain configuration.
-/// 5. Enable interrupts for the Cortex-AR core by calling [Self::enable_interrupts].
+/// 5. Enable interrupts for the Cortex-A core by calling [Self::enable_interrupts].
 ///
 /// For the handling of the interrupts, you can use the [GicInterruptHelper] which assumes a
 /// properly configured GIC.
 pub struct GicConfigurator {
+    /// GIC CPU interface registers.
     pub gicc: MmioCpuInterfaceRegisters<'static>,
+    /// GIC Distributor interface registers.
     pub gicd: MmioDistributorRegisters<'static>,
 }
 
@@ -325,45 +444,90 @@ impl GicConfigurator {
         Ok(())
     }
 
-    /// Set the CPU target for a SPI interrupt.
+    /// Set the CPU target(s) for a SPI interrupt.
     ///
     /// See [Self::set_all_spi_interrupt_targets_cpu0] for a utility method to handle all
     /// interrupts with one core.
     #[inline]
-    pub fn set_spi_interrupt_cpu_target(&mut self, spi_int: SpiInterrupt, target: TargetCpu) {
+    pub fn set_spi_interrupt_cpu_target_flags(
+        &mut self,
+        spi_int: SpiInterrupt,
+        target: TargetCpus,
+    ) {
         let spi_int_raw = spi_int as u32;
         let spi_offset_to_0 = spi_int_raw as usize - 32;
         // Unwrap okay, calculated index is always valid.
         self.gicd
-            .write_iptr_spi(
+            .modify_iptr_spi(spi_offset_to_0 / 4, |mut v| {
-                spi_offset_to_0 / 4,
+                // Every register contains 4 target flags, the modulo extracts the index because
-                (target as u32) << ((spi_offset_to_0 % 4) * 8),
+                // counting starts at 32 (32 -> index 0, 33 -> index 1 etc.).
-            )
+                v.set_targets(spi_offset_to_0 % 4, u2::new(target.bits()));
+                v
+            })
+            .unwrap();
+    }
+
+    /// Enable SPI interrupt target for a specific CPU without clearing the other CPU bit if it is
+    /// set.
+    #[inline]
+    pub fn set_spi_interrupt_target_for_cpu(&mut self, spi_int: SpiInterrupt, cpu: Cpu) {
+        let bitflag = match cpu {
+            Cpu::Cpu0 => TargetCpus::CPU_0,
+            Cpu::Cpu1 => TargetCpus::CPU_1,
+        };
+        let spi_int_raw = spi_int as u32;
+        let spi_offset_to_0 = spi_int_raw as usize - 32;
+        // Unwrap okay, calculated index is always valid.
+        self.gicd
+            .modify_iptr_spi(spi_offset_to_0 / 4, |mut v| {
+                v.set_targets(
+                    spi_offset_to_0 % 4,
+                    // Extract the target bits, bitwise OR them with [TargetCpus::CPU_0], and set
+                    // them back.
+                    u2::new(
+                        (TargetCpus::from_bits(v.targets(spi_offset_to_0 % 4).as_u8()).unwrap()
+                            | bitflag)
+                            .bits(),
+                    ),
+                );
+                v
+            })
             .unwrap();
     }
 
     /// Utility function to set all SGI interrupt targets to CPU0.
     ///
+    /// This does not clear interrupt target bits for CPU1, it only activates the interrupts for
+    /// CPU 0 as well.
     /// This is useful if only CPU0 is active in a system, or if CPU0 handles most interrupts in
     /// the system.
     #[inline]
     pub fn set_all_spi_interrupt_targets_cpu0(&mut self) {
         for i in 0..0x10 {
             self.gicd
-                .write_iptr_spi(i, TARGETS_ALL_CPU_0_IPTR_VAL)
+                .modify_iptr_spi(i, |v| {
+                    InterruptProcessorTargetRegister::new_with_raw_value(
+                        v.raw_value() | TARGETS_ALL_CPU_0_IPTR_VAL.raw_value(),
+                    )
+                })
                 .unwrap();
         }
     }
 
+    /// Enable a specific SGI interrupt.
     #[inline]
     pub fn enable_sgi_interrupt(&mut self, int_id: usize) -> Result<(), InvalidSpiInterruptId> {
         if int_id >= 16 {
             return Err(InvalidSpiInterruptId(int_id));
         }
-        unsafe { self.gicd.write_iser_unchecked(0, 1 << int_id) };
+        unsafe {
+            self.gicd
+                .modify_iser_unchecked(0, |val| val | (1 << int_id))
+        };
         Ok(())
     }
 
+    /// Enable all SGI interrupts.
     #[inline]
     pub fn enable_all_sgi_interrupts(&mut self) {
         // Unwrap okay, index is valid.
@@ -375,17 +539,16 @@ impl GicConfigurator {
             .unwrap();
     }
 
+    /// Enable specific PPI interrupt.
     #[inline]
     pub fn enable_ppi_interrupt(&mut self, ppi_int: PpiInterrupt) {
         // Unwrap okay, index is valid.
         self.gicd
-            .modify_iser(0, |mut v| {
+            .modify_iser(0, |v| v | 1 << (ppi_int as u32))
-                v |= 1 << (ppi_int as u32);
-                v
-            })
             .unwrap();
     }
 
+    /// Enable all PPI interrupts.
     #[inline]
     pub fn enable_all_ppi_interrupts(&mut self) {
         unsafe {
@@ -396,6 +559,7 @@ impl GicConfigurator {
         };
     }
 
+    /// Enable specific SPI interrupt.
     #[inline]
     pub fn enable_spi_interrupt(&mut self, spi_int: SpiInterrupt) {
         let spi_int_raw = spi_int as u32;
@@ -403,17 +567,18 @@ impl GicConfigurator {
             32..=63 => {
                 let bit_pos = spi_int_raw - 32;
                 // Unwrap okay, valid index.
-                self.gicd.write_iser(1, 1 << bit_pos).unwrap();
+                self.gicd.modify_iser(1, |v| v | (1 << bit_pos)).unwrap();
             }
             64..=92 => {
                 let bit_pos = spi_int_raw - 64;
                 // Unwrap okay, valid index.
-                self.gicd.write_iser(2, 1 << bit_pos).unwrap();
+                self.gicd.modify_iser(2, |v| v | (1 << bit_pos)).unwrap();
             }
             _ => unreachable!(),
         }
     }
 
+    /// Enable all SPI interrupts.
     #[inline]
     pub fn enable_all_spi_interrupts(&mut self) {
         self.gicd.write_iser(1, 0xFFFF_FFFF).unwrap();

zynq/zynq7000/CHANGELOG.md

@@ -8,8 +8,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 # [unreleased]
 
-- Renamed all register blocks to `Registers` to subblocks to `<Subblock>Registers`
+- Renamed all register blocks to `Registers` to subblocks to `<Subblock>Registers`.
-- Added SDIO registers
+- Updated IPTR registers in the GIC module to use a custom register type instead of a raw u32.
+- Added SDIO registers.
 
 # [v0.1.1] 2025-10-09
 

zynq/zynq7000/src/gic.rs

@@ -1,6 +1,6 @@
 //! # GIC (Generic Interrupt Controller) register module.
 pub use crate::mpcore::{GICC_BASE_ADDR, GICD_BASE_ADDR};
-use arbitrary_int::{u3, u5, u10};
+use arbitrary_int::{u2, u3, u5, u10};
 use static_assertions::const_assert_eq;
 
 /// Distributor Control Register
@@ -40,6 +40,16 @@ impl TypeRegister {
 
 pub type Typer = TypeRegister;
 
+// TODO: Use bitbybit debug derive if new release was released.
+/// Interrupt processor target register (IPTR).
+#[bitbybit::bitfield(u32)]
+#[derive(Debug, PartialEq, Eq)]
+pub struct InterruptProcessorTargetRegister {
+    /// Target array. Every register holds the information for 4 interrupts.
+    #[bits(0..=1, rw, stride = 8)]
+    targets: [u2; 4],
+}
+
 #[deprecated(note = "Use DistributorRegisters instead")]
 pub type GicDistributorTyper = DistributorRegisters;
 
@@ -78,10 +88,11 @@ pub struct DistributorRegisters {
     pub ipr: [u32; 0x18],
     _reserved_11: [u32; 0xE8],
     /// Interrupt Processor Targes Registers
-    pub iptr_sgi: [u32; 0x4],
+    pub iptr_sgi: [InterruptProcessorTargetRegister; 0x4],
-    // TODO: Mark those read-only as soon as that works for arrays.
+    /// These are read-only because they always target their private CPU.
-    pub iptr_ppi: [u32; 0x4],
+    #[mmio(PureRead)]
-    pub iptr_spi: [u32; 0x10],
+    pub iptr_ppi: [InterruptProcessorTargetRegister; 0x4],
+    pub iptr_spi: [InterruptProcessorTargetRegister; 0x10],
     // Those are split in the ARM documentation for some reason..
     _reserved_12: [u32; 0xE8],
     /// Interrupt Configuration Registers