Refactoring of IRQ handling

README.md

@@ -83,24 +83,19 @@ transaction. In the default configuration the PicoRV32 core only expects the
 `mem_rdata` input to be valid in the cycle with `mem_valid && mem_ready` and
 latches the value internally.
 
-#### ENABLE_EXTERNAL_IRQ (default = 0)
+#### ENABLE_IRQ (default = 0)
 
-Set this to 1 to enable external IRQs.
+Set this to 1 to enable IRQs.
 
-#### ENABLE_ILLINSTR_IRQ (default = 0)
+#### MASKED_IRQ (default = 32'h 0000_0000)
 
-Set this to 1 to enable the illegal instruction IRQ. This can be used for
+A 1 bit in this bitmask corresponds to a permanently disabled IRQ.
-software implementations of instructions such as `MUL` and `DIV`.
 
-#### ENABLE_TIMER_IRQ (default = 0)
+#### PROGADDR_RESET (default = 32'h 0000_0000)
-
-Set this to 1 to enable the built-in timer and timer IRQ.
-
-#### PROGADDR_RESET (default = 0)
 
 The start address of the program.
 
-#### PROGADDR_IRQ (default = 16)
+#### PROGADDR_IRQ (default = 32'h 0000_0010)
 
 The start address of the interrupt handler.
 
@@ -141,11 +136,30 @@ Custom Instructions for IRQ Handling
 
 The following custom instructions are supported when IRQs are enabled.
 
+The PicoRV32 core has a built-in interrupt controller with 32 interrupts. An
+interrupt can be triggered by asserting the corresponding bit in the `irq`
+input of the core.
+
+When the interrupt handler is started, the `eoi` End Of Interrupt (EOI) signals
+for the handled interrupts goes high. The `eoi` signal goes low again when the
+interrupt handler returns.
+
+The IRQs 0-2 can be triggered internally and have the following meaning:
+
+| IRQ | Interrupt Source                   |
+| ---:| -----------------------------------|
+|   0 | Timer Interrupt                    |
+|   1 | SBREAK or Illegal Instruction      |
+|   2 | BUS Error (Unalign Memory Access)  |
+
 The core has 4 additional 32-bit registers `q0 .. q3` that are used for IRQ
 handling. When an IRQ triggers, the register `q0` contains the return address
-and `q1` contains the IRQ number. Registers `q2` and `q3` are uninitialized
+and `q1` contains a bitmask of all active IRQs. I.e. one call to the interrupt
-and can be used as temporary storage when saving/restoring register values
+handler might need to service one than more IRQ when more than one bit is set
-in the IRQ handler.
+in `q1`.
+
+Registers `q2` and `q3` are uninitialized and can be used as temporary storage
+when saving/restoring register values in the IRQ handler.
 
 #### getq rd, qs
 
@@ -182,7 +196,7 @@ Example assembler code using the `custom0` mnemonic:
 #### retirq
 
 Return from interrupt. This instruction copies the value from `q0`
-to the program counter and enables interrupts. The Instruction is
+to the program counter and re-enables interrupts. The Instruction is
 encoded under the `custom0` opcode:
 
     0000010 00000 00000 000 00000 0001011
@@ -196,36 +210,26 @@ Example assembler code using the `custom0` mnemonic:
 
 #### maskirq
 
+The "IRQ Mask" register contains a birtmask of masked (disabled) interrupts.
+This opcodes writes a new value to the irq mask register and reads the  old
+value.
+
 Enable/disable interrupt sources. The Instruction is encoded under the
 `custom0` opcode:
 
-    0000011 XXXXX 00000 000 00000 0001011
+    0000011 00000 XXXXX 000 XXXXX 0001011
     f7      f5    rs    f3  rd    opcode
 
-The following interrupt sources occupy the following bits
-in the `f5` field:
-
-| Bit   | Interrupt Source     |
-| ------| ---------------------|
-| f5[0] | External IRQ         |
-| f5[1] | Timer Interrupt      |
-| f5[2] | Illegal Instruction  |
-| f5[3] | Reserved             |
-| f5[4] | Reserved             |
-
-Set bits in the IRQ mask correspond to enabled interrupt sources.
-
 Example assembler code using the `custom0` mnemonic:
 
 | Instruction       | Assember Code       |
 | ------------------| --------------------|
-| maskirq 0         | custom0 0, 0, 0, 3  |
+| maskirq x1, x2    | custom0 1, 2, 0, 3  |
-| maskirq 1         | custom0 0, 0, 1, 3  |
 
 The processor starts with all interrupts disabled.
 
-An illegal instruction while the illegal instruction interrupt is disabled will
+An illegal instruction or bus error while the illegal instruction or bus error
-cause the processor to halt.
+interrupt is disabled will cause the processor to halt.
 
 #### waitirq (unimplemented)
 
@@ -243,7 +247,7 @@ Example assembler code using the `custom0` mnemonic:
 
 #### timer
 
-Reset the timer counter to a new value. The counter counts down cycles and
+Reset the timer counter to a new value. The counter counts down clock cycles and
 triggers the timer interrupt when transitioning from 1 to 0. Setting the
 counter to zero disables the timer.
 
@@ -261,6 +265,7 @@ Todos:
 ------
 
 - Optional FENCE support
+- Optional Co-Processor Interface
 - Optional write-through cache
 - Optional support for compressed ISA
 - Improved documentation and examples

firmware/sieve.c

@@ -33,7 +33,7 @@ static void print_dec(int val)
 {
 	char buffer[10];
 	char *p = buffer;
-	while (val) {
+	while (val || p == buffer) {
 		*(p++) = val % 10;
 		val = val / 10;
 	}
@@ -88,24 +88,28 @@ void sieve()
 	}
 }
 
-void irq(uint32_t *regs, uint32_t irqnum)
+uint32_t *irq(uint32_t *regs, uint32_t irqs)
 {
-	static int ext_irq_count = 0;
+	static int ext_irq_4_count = 0;
+	static int ext_irq_5_count = 0;
 	static int timer_irq_count = 0;
 
-	if (irqnum == 0) {
+	if ((irqs & (1<<4)) != 0) {
-		ext_irq_count++;
+		ext_irq_4_count++;
-		// print_str("[EXT-IRQ]");
+		// print_str("[EXT-IRQ-4]");
-		return;
 	}
 
-	if (irqnum == 1) {
+	if ((irqs & (1<<5)) != 0) {
+		ext_irq_5_count++;
+		// print_str("[EXT-IRQ-5]");
+	}
+
+	if ((irqs & 1) != 0) {
 		timer_irq_count++;
 		// print_str("[TIMER-IRQ]");
-		return;
 	}
 
-	if (irqnum == 2)
+	if ((irqs & 6) != 0)
 	{
 		int i, k;
 		uint32_t pc = regs[0] - 4;
@@ -114,12 +118,22 @@ void irq(uint32_t *regs, uint32_t irqnum)
 		print_str("\n");
 		print_str("------------------------------------------------------------\n");
 
-		if (instr == 0x00100073) {
+		if ((irqs & 2) != 0) {
-			print_str("SBREAK instruction at 0x");
+			if (instr == 0x00100073) {
-			print_hex(pc);
+				print_str("SBREAK instruction at 0x");
-			print_str("\n");
+				print_hex(pc);
-		} else {
+				print_str("\n");
-			print_str("Illegal Instruction at 0x");
+			} else {
+				print_str("Illegal Instruction at 0x");
+				print_hex(pc);
+				print_str(": 0x");
+				print_hex(instr);
+				print_str("\n");
+			}
+		}
+
+		if ((irqs & 4) != 0) {
+			print_str("Bus error in Instruction at 0x");
 			print_hex(pc);
 			print_str(": 0x");
 			print_hex(instr);
@@ -164,8 +178,12 @@ void irq(uint32_t *regs, uint32_t irqnum)
 
 		print_str("------------------------------------------------------------\n");
 
-		print_str("Number of external IRQs counted: ");
+		print_str("Number of fast external IRQs counted: ");
-		print_dec(ext_irq_count);
+		print_dec(ext_irq_4_count);
+		print_str("\n");
+
+		print_str("Number of slow external IRQs counted: ");
+		print_dec(ext_irq_5_count);
 		print_str("\n");
 
 		print_str("Number of timer IRQs counted: ");
@@ -173,7 +191,8 @@ void irq(uint32_t *regs, uint32_t irqnum)
 		print_str("\n");
 
 		__asm__("sbreak");
-		return;
 	}
+
+	return regs;
 }
 

firmware/start.S

@@ -10,7 +10,7 @@
 	jal zero,n; n ## _ret:
 
 reset_vec:
-	custom0 0,0,7,3 // maskirq 7
+	custom0 0,0,0,3 // maskirq zero, zero
 	j start
 	nop
 	nop

picorv32.v

@@ -26,17 +26,16 @@
  ***************************************************************/
 
 module picorv32 #(
-	parameter ENABLE_COUNTERS = 1,
+	parameter [ 0:0] ENABLE_COUNTERS = 1,
-	parameter ENABLE_REGS_16_31 = 1,
+	parameter [ 0:0] ENABLE_REGS_16_31 = 1,
-	parameter ENABLE_REGS_DUALPORT = 1,
+	parameter [ 0:0] ENABLE_REGS_DUALPORT = 1,
-	parameter LATCHED_MEM_RDATA = 0,
+	parameter [ 0:0] LATCHED_MEM_RDATA = 0,
-	parameter ENABLE_EXTERNAL_IRQ = 0,
+	parameter [ 0:0] ENABLE_IRQ = 0,
-	parameter ENABLE_ILLINSTR_IRQ = 0,
+	parameter [31:0] MASKED_IRQ = 32'h 0000_0000,
-	parameter ENABLE_TIMER_IRQ = 0,
+	parameter [31:0] PROGADDR_RESET = 32'h 0000_0000,
-	parameter PROGADDR_RESET = 0,
+	parameter [31:0] PROGADDR_IRQ = 32'h 0000_0010
-	parameter PROGADDR_IRQ = 16
 ) (
-	input clk, resetn, irq,
+	input clk, resetn,
 	output reg trap,
 
 	output reg        mem_valid,
@@ -53,9 +52,15 @@ module picorv32 #(
 	output            mem_la_write,
 	output     [31:0] mem_la_addr,
 	output reg [31:0] mem_la_wdata,
-	output reg [ 3:0] mem_la_wstrb
+	output reg [ 3:0] mem_la_wstrb,
+
+	// IRQ interface
+	input      [31:0] irq,
+	output reg [31:0] eoi
 );
-	localparam ENABLE_IRQ = ENABLE_EXTERNAL_IRQ || ENABLE_ILLINSTR_IRQ || ENABLE_TIMER_IRQ;
+	localparam integer irq_timer = 0;
+	localparam integer irq_sbreak = 1;
+	localparam integer irq_buserror = 2;
 
 	localparam integer irqregs_offset = ENABLE_REGS_16_31 ? 32 : 16;
 	localparam integer regfile_size = (ENABLE_REGS_16_31 ? 32 : 16) + 4*ENABLE_IRQ;
@@ -69,8 +74,8 @@ module picorv32 #(
 	wire [31:0] next_pc;
 
 	reg irq_active;
-	reg [4:0] irq_mask;
+	reg [31:0] irq_mask;
-	reg [4:0] irq_pending;
+	reg [31:0] irq_pending;
 	reg [31:0] timer;
 
 	// Memory Interface
@@ -185,6 +190,7 @@ module picorv32 #(
 	reg [regindex_bits-1:0] decoded_rd, decoded_rs1, decoded_rs2;
 	reg [31:0] decoded_imm, decoded_imm_uj;
 	reg decoder_trigger;
+	reg decoder_trigger_q;
 	reg decoder_pseudo_trigger;
 
 	reg is_lui_auipc_jal;
@@ -272,7 +278,7 @@ module picorv32 #(
 		if (instr_waitirq)  new_instruction = "waitirq";
 		if (instr_timer)    new_instruction = "timer";
 
-		if (new_instruction)
+		if (decoder_trigger_q)
 			instruction = new_instruction;
 	end
 
@@ -362,7 +368,7 @@ module picorv32 #(
 			instr_setq    <= mem_rdata_q[6:0] == 7'b0001011 && mem_rdata_q[31:25] == 7'b0000001 && ENABLE_IRQ;
 			instr_maskirq <= mem_rdata_q[6:0] == 7'b0001011 && mem_rdata_q[31:25] == 7'b0000011 && ENABLE_IRQ;
 			instr_waitirq <= mem_rdata_q[6:0] == 7'b0001011 && mem_rdata_q[31:25] == 7'b0000100 && ENABLE_IRQ;
-			instr_timer   <= mem_rdata_q[6:0] == 7'b0001011 && mem_rdata_q[31:25] == 7'b0000101 && ENABLE_TIMER_IRQ;
+			instr_timer   <= mem_rdata_q[6:0] == 7'b0001011 && mem_rdata_q[31:25] == 7'b0000101 && ENABLE_IRQ;
 
 			is_slli_srli_srai <= is_alu_reg_imm && |{
 				mem_rdata_q[14:12] == 3'b001 && mem_rdata_q[31:25] == 7'b0000000,
@@ -432,6 +438,8 @@ module picorv32 #(
 	reg [31:0] current_pc;
 	assign next_pc = latched_store && latched_branch ? reg_out : reg_next_pc;
 
+	reg [31:0] next_irq_pending;
+
 	reg [31:0] alu_out;
 	reg alu_out_0;
 
@@ -484,16 +492,19 @@ module picorv32 #(
 		if (ENABLE_COUNTERS)
 			count_cycle <= resetn ? count_cycle + 1 : 0;
 
-		if (ENABLE_TIMER_IRQ && timer) begin
+		next_irq_pending = irq_pending;
+
+		if (ENABLE_IRQ && timer) begin
 			if (timer - 1 == 0)
-				irq_pending[1] <= 1;
+				next_irq_pending[irq_timer] = 1;
 			timer <= timer - 1;
 		end
 
-		if (ENABLE_EXTERNAL_IRQ && irq) begin
+		if (ENABLE_IRQ) begin
-			irq_pending[0] <= 1;
+			next_irq_pending = next_irq_pending | irq;
 		end
 
+		decoder_trigger_q <= decoder_trigger;
 		decoder_trigger <= mem_do_rinst && mem_done;
 		decoder_pseudo_trigger <= 0;
 
@@ -509,9 +520,10 @@ module picorv32 #(
 			latched_is_lh <= 0;
 			latched_is_lb <= 0;
 			irq_active <= 0;
-			irq_mask <= 0;
+			irq_mask <= ~0;
-			irq_pending <= 0;
+			next_irq_pending = 0;
 			irq_state <= 0;
+			eoi <= 0;
 			timer <= 0;
 			cpu_state <= cpu_state_fetch;
 		end else
@@ -546,16 +558,9 @@ module picorv32 #(
 					mem_do_rinst <= 1;
 				end else
 				if (ENABLE_IRQ && irq_state[1]) begin
-					cpuregs[latched_rd] <=
+					eoi <= irq_pending & ~irq_mask;
-						irq_pending[0] && irq_mask[0] ? 0 :
+					cpuregs[latched_rd] <= irq_pending & ~irq_mask;
-						irq_pending[1] && irq_mask[1] ? 1 :
+					next_irq_pending = next_irq_pending & irq_mask;
-						irq_pending[2] && irq_mask[2] ? 2 :
-						irq_pending[3] && irq_mask[3] ? 3 : 4;
-					irq_pending <=
-						irq_pending[0] && irq_mask[0] ? irq_pending & 5'b11110 :
-						irq_pending[1] && irq_mask[1] ? irq_pending & 5'b11101 :
-						irq_pending[2] && irq_mask[2] ? irq_pending & 5'b11011 :
-						irq_pending[3] && irq_mask[3] ? irq_pending & 5'b10111 : irq_pending & 5'b01111;
 				end
 
 				reg_pc <= current_pc;
@@ -569,7 +574,7 @@ module picorv32 #(
 				latched_is_lb <= 0;
 				latched_rd <= decoded_rd;
 
-				if (ENABLE_IRQ && ((decoder_trigger && !irq_active && |(irq_pending & irq_mask)) || irq_state)) begin
+				if (ENABLE_IRQ && ((decoder_trigger && !irq_active && |(irq_pending & ~irq_mask)) || irq_state)) begin
 					irq_state <=
 						irq_state == 2'b00 ? 2'b01 :
 						irq_state == 2'b01 ? 2'b10 : 2'b00;
@@ -600,14 +605,14 @@ module picorv32 #(
 				reg_op1 <= 'bx;
 				reg_op2 <= 'bx;
 `ifdef DEBUG
-				$display("DECODE: 0x%08x %-0s", reg_pc, instruction);
+				$display("DECODE: 0x%08x %-0s", reg_pc, instruction ? instruction : "UNKNOWN");
 `endif
 				if (instr_trap) begin
 `ifdef DEBUG
 					$display("SBREAK OR UNSUPPORTED INSN AT 0x%08x", reg_pc);
 `endif
-					if (ENABLE_ILLINSTR_IRQ && irq_mask[2] && !irq_active) begin
+					if (ENABLE_IRQ && !irq_mask[irq_sbreak] && !irq_active) begin
-						irq_pending[2] <= 1;
+						next_irq_pending[irq_sbreak] = 1;
 						cpu_state <= cpu_state_fetch;
 					end else
 						cpu_state <= cpu_state_trap;
@@ -645,6 +650,7 @@ module picorv32 #(
 					cpu_state <= cpu_state_fetch;
 				end else
 				if (ENABLE_IRQ && instr_retirq) begin
+					eoi <= 0;
 					irq_active <= 0;
 					latched_branch <= 1;
 					latched_store <= 1;
@@ -652,10 +658,12 @@ module picorv32 #(
 					cpu_state <= cpu_state_fetch;
 				end else
 				if (ENABLE_IRQ && instr_maskirq) begin
-					irq_mask = decoded_rs2;
+					latched_store <= 1;
+					reg_out <= irq_mask;
+					irq_mask <= (decoded_rs1 ? cpuregs[decoded_rs1] : 0) | MASKED_IRQ;
 					cpu_state <= cpu_state_fetch;
 				end else
-				if (ENABLE_TIMER_IRQ && instr_timer) begin
+				if (ENABLE_IRQ && instr_timer) begin
 					timer <= cpuregs[decoded_rs1];
 					cpu_state <= cpu_state_fetch;
 				end else begin
@@ -806,20 +814,29 @@ module picorv32 #(
 `ifdef DEBUG
 				$display("MISALIGNED WORD: 0x%08x", reg_op1);
 `endif
-				cpu_state <= cpu_state_trap;
+				if (ENABLE_IRQ && !irq_mask[irq_buserror] && !irq_active) begin
+					next_irq_pending[irq_buserror] = 1;
+				end else
+					cpu_state <= cpu_state_trap;
 			end
 			if (mem_wordsize == 1 && reg_op1[0] != 0) begin
 `ifdef DEBUG
 				$display("MISALIGNED HALFWORD: 0x%08x", reg_op1);
 `endif
-				cpu_state <= cpu_state_trap;
+				if (ENABLE_IRQ && !irq_mask[irq_buserror] && !irq_active) begin
+					next_irq_pending[irq_buserror] = 1;
+				end else
+					cpu_state <= cpu_state_trap;
 			end
 		end
 		if (resetn && mem_do_rinst && reg_pc[1:0] != 0) begin
 `ifdef DEBUG
 			$display("MISALIGNED INSTRUCTION: 0x%08x", reg_pc);
 `endif
-			cpu_state <= cpu_state_trap;
+			if (ENABLE_IRQ && !irq_mask[irq_buserror] && !irq_active) begin
+				next_irq_pending[irq_buserror] = 1;
+			end else
+				cpu_state <= cpu_state_trap;
 		end
 
 		if (!resetn || mem_done) begin
@@ -836,6 +853,8 @@ module picorv32 #(
 		if (set_mem_do_wdata)
 			mem_do_wdata <= 1;
 
+		irq_pending <= next_irq_pending & ~MASKED_IRQ;
+
 		reg_pc[1:0] <= 0;
 		reg_next_pc[1:0] <= 0;
 		current_pc = 'bx;
@@ -848,14 +867,15 @@ endmodule
  ***************************************************************/
 
 module picorv32_axi #(
-	parameter ENABLE_COUNTERS = 1,
+	parameter [ 0:0] ENABLE_COUNTERS = 1,
-	parameter ENABLE_REGS_16_31 = 1,
+	parameter [ 0:0] ENABLE_REGS_16_31 = 1,
-	parameter ENABLE_REGS_DUALPORT = 1,
+	parameter [ 0:0] ENABLE_REGS_DUALPORT = 1,
-	parameter ENABLE_EXTERNAL_IRQ = 0,
+	parameter [ 0:0] ENABLE_IRQ = 0,
-	parameter ENABLE_ILLINSTR_IRQ = 0,
+	parameter [31:0] MASKED_IRQ = 32'h 0000_0000,
-	parameter ENABLE_TIMER_IRQ = 0
+	parameter [31:0] PROGADDR_RESET = 32'h 0000_0000,
+	parameter [31:0] PROGADDR_IRQ = 32'h 0000_0010
 ) (
-	input clk, resetn, irq,
+	input clk, resetn,
 	output trap,
 
 	// AXI4-lite master memory interface
@@ -880,7 +900,11 @@ module picorv32_axi #(
 
 	input         mem_axi_rvalid,
 	output        mem_axi_rready,
-	input  [31:0] mem_axi_rdata
+	input  [31:0] mem_axi_rdata,
+
+	// IRQ interface
+	input  [31:0] irq,
+	output [31:0] eoi
 );
 	wire        mem_valid;
 	wire [31:0] mem_addr;
@@ -923,13 +947,13 @@ module picorv32_axi #(
 		.ENABLE_COUNTERS     (ENABLE_COUNTERS     ),
 		.ENABLE_REGS_16_31   (ENABLE_REGS_16_31   ),
 		.ENABLE_REGS_DUALPORT(ENABLE_REGS_DUALPORT),
-		.ENABLE_EXTERNAL_IRQ (ENABLE_EXTERNAL_IRQ ),
+		.ENABLE_IRQ          (ENABLE_IRQ          ),
-		.ENABLE_ILLINSTR_IRQ (ENABLE_ILLINSTR_IRQ ),
+		.MASKED_IRQ          (MASKED_IRQ          ),
-		.ENABLE_TIMER_IRQ    (ENABLE_TIMER_IRQ    )
+		.PROGADDR_RESET      (PROGADDR_RESET      ),
+		.PROGADDR_IRQ        (PROGADDR_IRQ        )
 	) picorv32_core (
 		.clk      (clk      ),
 		.resetn   (resetn   ),
-		.irq      (irq      ),
 		.trap     (trap     ),
 		.mem_valid(mem_valid),
 		.mem_addr (mem_addr ),
@@ -937,7 +961,9 @@ module picorv32_axi #(
 		.mem_wstrb(mem_wstrb),
 		.mem_instr(mem_instr),
 		.mem_ready(mem_ready),
-		.mem_rdata(mem_rdata)
+		.mem_rdata(mem_rdata),
+		.irq      (irq      ),
+		.eoi      (eoi      )
 	);
 endmodule
 

testbench.v

@@ -6,9 +6,15 @@ module testbench;
 
 	reg clk = 1;
 	reg resetn = 0;
-	wire irq = &uut.picorv32_core.count_cycle[12:0];
+	reg [31:0] irq;
 	wire trap;
 
+	always @* begin
+		irq = 0;
+		irq[4] = &uut.picorv32_core.count_cycle[12:0];
+		irq[5] = &uut.picorv32_core.count_cycle[15:0];
+	end
+
 	always #5 clk = ~clk;
 
 	initial begin
@@ -39,13 +45,10 @@ module testbench;
 	reg  [31:0] mem_axi_rdata;
 
 	picorv32_axi #(
-		.ENABLE_EXTERNAL_IRQ (1),
+		.ENABLE_IRQ(1)
-		.ENABLE_ILLINSTR_IRQ (1),
-		.ENABLE_TIMER_IRQ    (1)
 	) uut (
 		.clk            (clk            ),
 		.resetn         (resetn         ),
-		.irq            (irq            ),
 		.trap           (trap           ),
 		.mem_axi_awvalid(mem_axi_awvalid),
 		.mem_axi_awready(mem_axi_awready),
@@ -63,7 +66,8 @@ module testbench;
 		.mem_axi_arprot (mem_axi_arprot ),
 		.mem_axi_rvalid (mem_axi_rvalid ),
 		.mem_axi_rready (mem_axi_rready ),
-		.mem_axi_rdata  (mem_axi_rdata  )
+		.mem_axi_rdata  (mem_axi_rdata  ),
+		.irq            (irq            )
 	);
 
 	reg [31:0] memory [0:64*1024/4-1];