reworked external interrupts controller (XIRQ) handshake

CHANGELOG.md

@@ -26,6 +26,7 @@ defined by the `hw_version_c` constant in the main VHDL package file [`rtl/core/
 
 | Date (*dd.mm.yyyy*) | Version | Comment |
 |:----------:|:-------:|:--------|
+| 22.09.2021 | 1.6.0.10 | reworked CPU/software handshake of external interrupt controller `XIRQ` to avoid "external IRQ -> CPU IRQ" race conditions |
 | 22.09.2021 | 1.6.0.9 | if `CPU_CNT_WIDTH` generic (actual width of `[m]cycle` and `[m]instret` counters) is less than 64 the remaining bits are now just hardwired to zero ignoring any write access instead of causing an exception; minor CPU hardware optimizations |
 | 22.09.2021 | 1.6.0.8 | :bug: fixed bug introduced in previous version: misaligned instruction address - PC and all instruction address-related registers need to have bit 0 hardwired to zero, misaligned instructions can only appear if NOT using `C` ISA extension |
 | 21.09.2021 | 1.6.0.7 | :warning: **reworked CPU trap/exception system** (in order to comply with RISC-V specs.): removed non-maskable interrupt (`NMI`, top signal `nm_irq_i`); reworked CPU trap prioritization (sync before async); RISC-V interrupts (`MTI`, `MSI`, `MEI`) are now high-level-triggered and require to stay asserted until they are explicitly acknowledged; fixed minor bug in misaligned instruction check logic (PC(0) = '1' will always cause a misalignment exception); updated trap/interrupt-related documentation |

rtl/core/neorv32_package.vhd

@@ -64,7 +64,7 @@ package neorv32_package is
   -- Architecture Constants (do not modify!) ------------------------------------------------
   -- -------------------------------------------------------------------------------------------
   constant data_width_c : natural := 32; -- native data path width - do not change!
-  constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01060009"; -- no touchy!
+  constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01060010"; -- no touchy!
   constant archid_c     : natural := 19; -- official NEORV32 architecture ID - hands off!
 
   -- External Interface Types ---------------------------------------------------------------

rtl/core/neorv32_xirq.vhd

@@ -80,7 +80,8 @@ architecture neorv32_xirq_rtl of neorv32_xirq is
 
   -- control registers --
   signal irq_enable  : std_ulogic_vector(XIRQ_NUM_CH-1 downto 0); -- r/w: interrupt enable
-  signal clr_pending : std_ulogic_vector(XIRQ_NUM_CH-1 downto 0); -- (r)/w: clear/ack pending IRQs
+  signal clr_pending : std_ulogic_vector(XIRQ_NUM_CH-1 downto 0); -- r/w: clear pending IRQs
+  signal irq_src     : std_ulogic_vector(4 downto 0); -- r/w: source IRQ, ACK on any write
 
   -- interrupt trigger --
   signal irq_sync  : std_ulogic_vector(XIRQ_NUM_CH-1 downto 0);
@@ -92,12 +93,11 @@ architecture neorv32_xirq_rtl of neorv32_xirq is
   signal irq_fire : std_ulogic;
 
   -- interrupt source --
-  signal irq_src, irq_src_nxt : std_ulogic_vector(04 downto 0);
+  signal irq_src_nxt : std_ulogic_vector(4 downto 0);
 
   -- arbiter --
-  signal irq_run    : std_ulogic;
-  signal irq_run_ff : std_ulogic;
-  signal host_ack   : std_ulogic;
+  signal irq_run  : std_ulogic;
+  signal host_ack : std_ulogic;
 
 begin
 
@@ -122,16 +122,19 @@ begin
 
       -- write access --
       host_ack    <= '0';
-      clr_pending <= (others => '0');
+      clr_pending <= (others => '1');
       if ((acc_en and wren_i) = '1') then
         -- channel-enable --
         if (addr = xirq_enable_addr_c) then
           irq_enable <= data_i(XIRQ_NUM_CH-1 downto 0);
         end if;
-        -- clear/ACK pending IRQ --
+        -- clear pending IRQs --
         if (addr = xirq_pending_addr_c) then
-          host_ack    <= '1';
-          clr_pending <= data_i(XIRQ_NUM_CH-1 downto 0);
+          clr_pending <= data_i(XIRQ_NUM_CH-1 downto 0); -- set zero to clear pending IRQ
+        end if;
+        -- acknowledge IRQ --
+        if (addr = xirq_source_addr_c) then -- write _any_ value to ACK
+          host_ack <= '1';
         end if;
       end if;
 
@@ -180,7 +183,7 @@ begin
   irq_buffer: process(clk_i)
   begin
     if rising_edge(clk_i) then
-      irq_buf <= (irq_buf or (irq_trig and irq_enable)) and (not clr_pending);
+      irq_buf <= (irq_buf or (irq_trig and irq_enable)) and clr_pending;
     end if;
   end process irq_buffer;
 
@@ -193,12 +196,14 @@ begin
   irq_priority: process(irq_buf)
   begin
     irq_src_nxt <= (others => '0');
-    for i in 0 to XIRQ_NUM_CH-1 loop
-      if (irq_buf(i) = '1') then
-        irq_src_nxt <= std_ulogic_vector(to_unsigned(i, 5));
-        exit;
-      end if;
-    end loop;
+    if (XIRQ_NUM_CH > 1) then
+      for i in 0 to XIRQ_NUM_CH-1 loop
+        if (irq_buf(i) = '1') then
+          irq_src_nxt(index_size_f(XIRQ_NUM_CH)-1 downto 0) <= std_ulogic_vector(to_unsigned(i, index_size_f(XIRQ_NUM_CH)));
+          exit;
+        end if;
+      end loop;
+    end if;
   end process irq_priority;
 
 
@@ -207,13 +212,14 @@ begin
   irq_arbiter: process(clk_i)
   begin
     if rising_edge(clk_i) then
-      irq_run_ff <= irq_run;
+      cpu_irq_o <= '0';
       if (irq_run = '0') then -- no active IRQ
         if (irq_fire = '1') then
-          irq_run <= '1';
-          irq_src <= irq_src_nxt;
+          cpu_irq_o <= '1';
+          irq_run   <= '1';
+          irq_src   <= irq_src_nxt;
         end if;
-      else -- active IRQ, wait for CPU acknowledge
+      else -- active IRQ, wait for CPU to acknowledge
         if (host_ack = '1') then
           irq_run <= '0';
         end if;
@@ -221,8 +227,5 @@ begin
     end if;
   end process irq_arbiter;
 
-  -- rising-edge detector --
-  cpu_irq_o <= irq_run and (not irq_run_ff);
-
 
 end neorv32_xirq_rtl;

sw/example/demo_xirq/main.c

@@ -99,7 +99,8 @@ int main() {
 
 
   // install handler functions for XIRQ channel 0,1,2,3. note that these functions are "normal" functions!
-  // (details: these are "third-level" interrupt handler)
+  // (details: these are "third-level" interrupt handlers)
+  // neorv32_xirq_install() also enables the specified XIRQ channel and clears any pending interrupts
   err_cnt = 0;
   err_cnt += neorv32_xirq_install(0, xirq_handler_ch0); // handler function for channel 0
   err_cnt += neorv32_xirq_install(1, xirq_handler_ch1); // handler function for channel 1
@@ -121,13 +122,12 @@ int main() {
   neorv32_cpu_eint();
 
 
-  // now we are ready to got!
   // the code below assumes the XIRQ inputs are connected to the processor's GPIO output port
   // so we can trigger the IRQs from software; if you have connected the XIRQs to buttons you
   // can remove the code below (note the trigger configuration using the XIRQ generics!)
   {
     // trigger XIRQs 3:0 at once
-    // assumes xirq_i <= gpio.output(31:0)
+    // assumes xirq_i(31:0) <= gpio.output(31:0)
 
     // due to the prioritization this will execute
     // -> xirq_handler_ch0
@@ -142,9 +142,9 @@ int main() {
   // --- wait for interrupts ---
   // All incoming XIRQ interrupt requests are "prioritized" in this example. The XIRQ FIRQ handler
   // reads the ID of the interrupt with the highest priority from the XIRQ controller ("source" register) and calls the according
-  // handler function.
+  // handler function (installed via neorv32_xirq_install();).
   // Non-prioritized handling of interrupts (or custom prioritization) can be implemented by manually reading the
-  // XIRQ controller's "pending" register. It is up to the software to define which pending IRQ should be served.
+  // XIRQ controller's "pending" register. Then it is up to the software to define which pending IRQ should be served first.
   while(1);
 
 
@@ -155,6 +155,17 @@ int main() {
   neorv32_xirq_uninstall(2); // disable XIRQ channel 2 and remove associated handler
   neorv32_xirq_uninstall(3); // disable XIRQ channel 3 and remove associated handler
 
+  // you can also manually clear pending interrupts
+  neorv32_xirq_clear_pending(0); // clear pending interrupt of channel 0
+
+  // manually enable and disable XIRQ channels
+  neorv32_xirq_channel_enable(0); // enable channel 0
+  neorv32_xirq_channel_disable(0); // disable channel 0
+
+  // globally enable/disable XIRQ CPU interrupt
+  // this will not affect the XIR configuration / pending interrupts
+  neorv32_xirq_global_enable();
+  neorv32_xirq_global_disable();
 
   return 0;
 }

sw/lib/include/neorv32_xirq.h

@@ -62,6 +62,9 @@ int neorv32_xirq_setup(void);
 void neorv32_xirq_global_enable(void);
 void neorv32_xirq_global_disable(void);
 int neorv32_xirq_get_num(void);
+void neorv32_xirq_clear_pending(uint8_t ch);
+void neorv32_xirq_channel_enable(uint8_t ch);
+void neorv32_xirq_channel_disable(uint8_t ch);
 
 int neorv32_xirq_install(uint8_t ch, void (*handler)(void));
 int neorv32_xirq_uninstall(uint8_t ch);

sw/lib/source/neorv32_xirq.c

@@ -49,8 +49,8 @@
 static uint32_t __neorv32_xirq_vector_lut[32] __attribute__((unused)); // trap handler vector table
 
 // private functions
-static void __attribute__((aligned(16))) __attribute__((unused)) __neorv32_xirq_core(void);
-static void __attribute__((unused)) __neorv32_xirq_dummy_handler(void);
+static void __attribute__((aligned(16))) __neorv32_xirq_core(void);
+static void __neorv32_xirq_dummy_handler(void);
 
 
 /**********************************************************************//**
@@ -79,14 +79,14 @@ int neorv32_xirq_available(void) {
 int neorv32_xirq_setup(void) {
 
   NEORV32_XIRQ.IER = 0; // disable all input channels
-  NEORV32_XIRQ.IPR = 0xffffffff; // clear/ack all pending IRQs
+  NEORV32_XIRQ.IPR = 0; // clear all pending IRQs
 
   int i;
   for (i=0; i<32; i++) {
     __neorv32_xirq_vector_lut[i] = (uint32_t)(&__neorv32_xirq_dummy_handler);
   }
 
-  // register XIRQ handler in RTE
+  // register XIRQ handler in NEORV32 RTE
   return neorv32_rte_exception_install(XIRQ_RTE_ID, __neorv32_xirq_core);
 }
 
@@ -143,6 +143,45 @@ int neorv32_xirq_get_num(void) {
 }
 
 
+/**********************************************************************//**
+ * Clear pending interrupt.
+ *
+ * @param[in] ch XIRQ interrupt channel (0..31).
+ **************************************************************************/
+void neorv32_xirq_clear_pending(uint8_t ch) {
+
+  if (ch < 32) { // channel valid?
+    NEORV32_XIRQ.IPR = ~(1 << ch);
+  }
+}
+
+
+/**********************************************************************//**
+ * Enable IRQ channel.
+ *
+ * @param[in] ch XIRQ interrupt channel (0..31).
+ **************************************************************************/
+void neorv32_xirq_channel_enable(uint8_t ch) {
+
+  if (ch < 32) { // channel valid?
+    NEORV32_XIRQ.IER |= 1 << ch;
+  }
+}
+
+
+/**********************************************************************//**
+ * Disable IRQ channel.
+ *
+ * @param[in] ch XIRQ interrupt channel (0..31).
+ **************************************************************************/
+void neorv32_xirq_channel_disable(uint8_t ch) {
+
+  if (ch < 32) { // channel valid?
+    NEORV32_XIRQ.IER &= ~(1 << ch);
+  }
+}
+
+
 /**********************************************************************//**
  * Install exception handler function for XIRQ channel.
  *
@@ -158,7 +197,7 @@ int neorv32_xirq_install(uint8_t ch, void (*handler)(void)) {
   if (ch < 32) {
     __neorv32_xirq_vector_lut[ch] = (uint32_t)handler; // install handler
     uint32_t mask = 1 << ch;
-    NEORV32_XIRQ.IPR = mask; // clear if pending
+    NEORV32_XIRQ.IPR = ~mask; // clear if pending
     NEORV32_XIRQ.IER |= mask; // enable channel
     return 0;
   }
@@ -181,7 +220,7 @@ int neorv32_xirq_uninstall(uint8_t ch) {
     __neorv32_xirq_vector_lut[ch] = (uint32_t)(&__neorv32_xirq_dummy_handler); // override using dummy handler
     uint32_t mask = 1 << ch;
     NEORV32_XIRQ.IER &= ~mask; // disable channel
-    NEORV32_XIRQ.IPR = mask; // clear if pending
+    NEORV32_XIRQ.IPR = ~mask; // clear if pending
     return 0;
   }
   return 1; 
@@ -189,17 +228,16 @@ int neorv32_xirq_uninstall(uint8_t ch) {
 
 
 /**********************************************************************//**
- * This is the actual second-level IRQ handler for the XIRQ. It will call the previously installed handler
- * if an XIRQ fires.
- *
- * @note This function must no be used by the user.
+ * This is the actual second-level (F)IRQ handler for the XIRQ. It will
+ * call the previously installed handler if an XIRQ fires.
  **************************************************************************/
-static void __attribute__((aligned(16))) __attribute__((unused)) __neorv32_xirq_core(void) {
+static void __attribute__((aligned(16))) __neorv32_xirq_core(void) {
 
   register uint32_t src = NEORV32_XIRQ.SCR; // get IRQ source (with highest priority)
-  src &= 0x1f;
 
-  NEORV32_XIRQ.IPR = (uint32_t)(1 << src); // acknowledge pending interrupt
+  uint32_t mask = 1 << src;
+  NEORV32_XIRQ.IPR = ~mask; // clear current pending interrupt
+  NEORV32_XIRQ.SCR = 0; // acknowledge current interrupt (CPU FIRQ)
 
   // execute handler
   register uint32_t xirq_handler = __neorv32_xirq_vector_lut[src];
@@ -212,7 +250,7 @@ static void __attribute__((aligned(16))) __attribute__((unused)) __neorv32_xirq_
 /**********************************************************************//**
  * XIRQ dummy handler.
  **************************************************************************/
-static void __attribute__((unused)) __neorv32_xirq_dummy_handler(void) {
+static void __neorv32_xirq_dummy_handler(void) {
 
   asm volatile ("nop");
 }