Fix a couple of bugs in preemption priority update, add simple IRQ preemption test

hdl/hazard3_csr.v

@@ -405,7 +405,7 @@ always @ (posedge clk or negedge rst_n) begin
 	end else if (wen_m_mode && addr == MEINEXT && wdata_update[0]) begin
 		// Interrupt has been sampled, with the update request set, so update
 		// the context (including preemption level) appropriately.
-		meicontext_preempt <= preempt_level_next & IRQ_PRIORITY_MASK;
+		meicontext_preempt <= preempt_level_next & {1'b1, IRQ_PRIORITY_MASK};
 		meicontext_noirq <= meinext_noirq;
 		meicontext_irq <= meinext_irq;
 	end
@@ -480,7 +480,9 @@ always @ (*) begin: get_highest_eirq_priority
 	integer i;
 	eirq_highest_priority = 4'h0;
 	for (i = 0; i < NUM_IRQS; i = i + 1) begin
-		eirq_highest_priority = meipra[4 * i +: 4] & {4{highest_eirq_onehot[i]}};
+		eirq_highest_priority = eirq_highest_priority | (
+			meipra[4 * i +: 4] & {4{highest_eirq_onehot[i]}}
+		);
 	end
 end
 

test/sim/common/hazard3_csr.h

@@ -10,7 +10,7 @@
 #define hazard3_csr_meiea      0xbe0 // External interrupt pending array
 #define hazard3_csr_meipa      0xbe1 // External interrupt enable array
 #define hazard3_csr_meifa      0xbe2 // External interrupt force array
-#define hazard3_csr_meipr      0xbe3 // External interrupt priority array
+#define hazard3_csr_meipra     0xbe3 // External interrupt priority array
 #define hazard3_csr_meinext    0xbe4 // Next external interrupt
 #define hazard3_csr_meicontext 0xbe5 // External interrupt context register
 

test/sim/common/hazard3_irq.h

@@ -28,4 +28,20 @@ static inline bool h3irq_force_pending(unsigned int irq) {
 	h3irq_array_set(hazard3_csr_meifa, irq >> 4, 1u << (irq & 0xfu));
 }
 
+// -1 for no IRQ
+static inline int h3irq_get_current_irq() {
+	uint32_t meicontext = read_csr(hazard3_csr_meicontext);
+	return meicontext & 0x8000u ? -1 : (meicontext >> 4) & 0x1ffu;
+}
+
+static inline void h3irq_set_priority(unsigned int irq, uint32_t priority) {
+	// Don't want read-modify-write, but no instruction for atomically writing
+	// a bitfield. So, first drop priority to minimum, then set to the target
+	// value. It should be safe to drop an IRQ's priority below its current
+	// even from within that IRQ (but it is never safe to boost an IRQ when
+	// it may already be in an older stack frame)
+	h3irq_array_clear(hazard3_csr_meipra, irq >> 2, 0xfu << (4 * (irq & 0x3)));
+	h3irq_array_set(hazard3_csr_meipra, irq >> 2, (priority & 0xfu) << (4 * (irq & 0x3)));
+}
+
 #endif

test/sim/common/irq_dispatch.S

@@ -0,0 +1,119 @@
+#include "hazard3_csr.h"
+
+.global isr_external_irq
+isr_external_irq:
+	// Save caller saves and exception return state whilst IRQs are disabled.
+	// We can't be pre-empted during this time, but if a higher-priority IRQ
+	// arrives ("late arrival"), that will be the one displayed in meinext. 
+	addi sp, sp, -80
+	sw ra,  0(sp)
+	sw t0,  4(sp)
+	sw t1,  8(sp)
+	sw t2, 12(sp)
+	sw a0, 16(sp)
+	sw a1, 20(sp)
+	sw a2, 24(sp)
+	sw a3, 28(sp)
+	sw a4, 32(sp)
+	sw a5, 36(sp)
+	sw a6, 40(sp)
+	sw a7, 44(sp)
+	sw t3, 48(sp)
+	sw t4, 52(sp)
+	sw t5, 56(sp)
+	sw t6, 60(sp)
+
+	csrr a0, mepc
+	sw a0, 64(sp)
+	// Make sure to set meicontext.clearts to clear and save mie.msie/mtie
+	// when saving context.
+	csrrsi a0, hazard3_csr_meicontext, 0x2
+	sw a0, 68(sp)
+	csrr a0, mstatus
+	sw a0, 72(sp)
+
+	j get_next_irq
+
+dispatch_irq:
+	// Preemption priority was configured by meinext update, so enable preemption:
+	csrsi mstatus, 0x8
+	// meinext is pre-shifted by 2, so only an add is required to index table
+	la a1, _external_irq_table
+	add a1, a1, a0
+	lw a1, (a1)
+	jalr ra, a1
+
+	// Disable IRQs on returning so we can sample the next IRQ
+	csrci mstatus, 0x8
+
+get_next_irq:
+	// Sample the current highest-priority active IRQ (left-shifted by 2) from
+	// meinext, and write 1 to the LSB to tell hardware to tell hw to update
+	// meicontext with the preemption priority (and IRQ number) of this IRQ
+	csrrsi a0, hazard3_csr_meinext, 0x1
+	// MSB will be set if there is no active IRQ at the current priority level
+	bgez a0, dispatch_irq
+
+no_more_irqs:
+	// Restore saved context and return from IRQ
+	lw a0, 64(sp)
+	csrw mepc, a0
+	lw a0, 68(sp)
+	csrw hazard3_csr_meicontext, a0
+	lw a0, 72(sp)
+	csrw mstatus, a0
+
+	lw ra,  0(sp)
+	lw t0,  4(sp)
+	lw t1,  8(sp)
+	lw t2, 12(sp)
+	lw a0, 16(sp)
+	lw a1, 20(sp)
+	lw a2, 24(sp)
+	lw a3, 28(sp)
+	lw a4, 32(sp)
+	lw a5, 36(sp)
+	lw a6, 40(sp)
+	lw a7, 44(sp)
+	lw t3, 48(sp)
+	lw t4, 52(sp)
+	lw t5, 56(sp)
+	lw t6, 60(sp)
+	addi sp, sp, 80
+	mret
+
+// ------------------------------------------------------------
+// Handler table and default handler symbols
+
+// Provide weak symbol for all IRQs, pointing to a breakpoint instruction:
+
+.macro decl_eirq num
+.weak isr_irq\num
+isr_irq\num:
+.endm
+
+.macro ref_eirq num
+.word isr_irq\num
+.endm
+
+#define NUM_IRQS 64
+
+.equ i, 0
+.rept NUM_IRQS
+decl_eirq i
+.equ i, i + 1
+.endr
+	ebreak
+
+// Soft vector table is preloaded to RAM, and by default contains the weak ISR
+// symbols, but can also be patched at runtime:
+
+.section .data
+.global _external_irq_table
+_external_irq_table:
+
+.equ i, 0
+.rept NUM_IRQS
+ref_eirq i
+.equ i, i + 1
+.endr

test/sim/sw_testcases/Makefile

@@ -1,8 +1,9 @@
 APP        := hellow
-SRCS       := ../common/init.S $(APP).c
+SRCS        = ../common/init.S $(APP).c $(EXTRA_SRCS_$(APP))
 CCFLAGS    := -march=rv32imac_zicsr -Os
 MAX_CYCLES := 1000000
 INCDIR     := include ../common
 
+EXTRA_SRCS_irq_preempt_set_in_irq := ../common/irq_dispatch.S
 
 include ../common/src_only_app.mk

test/sim/sw_testcases/csr_readable.c

@@ -140,7 +140,7 @@ int main() {
 	(void)read_csr(hazard3_csr_meiea);
 	(void)read_csr(hazard3_csr_meipa);
 	(void)read_csr(hazard3_csr_meifa);
-	(void)read_csr(hazard3_csr_meipr);
+	(void)read_csr(hazard3_csr_meipra);
 	(void)read_csr(hazard3_csr_meinext);
 	(void)read_csr(hazard3_csr_meicontext);
 

test/sim/sw_testcases/csr_readable_umode.c

@@ -192,7 +192,7 @@ CSR was be0
 CSR was be1
 -> exception, mcause = 2, mpp = 0 // hazard3_csr_meifa
 CSR was be2
--> exception, mcause = 2, mpp = 0 // hazard3_csr_meipr
+-> exception, mcause = 2, mpp = 0 // hazard3_csr_meipra
 CSR was be3
 -> exception, mcause = 2, mpp = 0 // hazard3_csr_meinext
 CSR was be4
@@ -304,7 +304,7 @@ void read_all_csrs() {
 	(void)read_csr(hazard3_csr_meiea);
 	(void)read_csr(hazard3_csr_meipa);
 	(void)read_csr(hazard3_csr_meifa);
-	(void)read_csr(hazard3_csr_meipr);
+	(void)read_csr(hazard3_csr_meipra);
 	(void)read_csr(hazard3_csr_meinext);
 	(void)read_csr(hazard3_csr_meicontext);
 }

test/sim/sw_testcases/csr_writable.c

@@ -170,7 +170,7 @@ int main() {
 	write_csr(hazard3_csr_meiea,      read_csr(hazard3_csr_meiea     ));
 	write_csr(hazard3_csr_meipa,      read_csr(hazard3_csr_meipa     ));
 	write_csr(hazard3_csr_meifa,      read_csr(hazard3_csr_meifa     ));
-	write_csr(hazard3_csr_meipr,      read_csr(hazard3_csr_meipr     ));
+	write_csr(hazard3_csr_meipra,     read_csr(hazard3_csr_meipra    ));
 	write_csr(hazard3_csr_meinext,    read_csr(hazard3_csr_meinext   ));
 	write_csr(hazard3_csr_meicontext, read_csr(hazard3_csr_meicontext));
 

test/sim/sw_testcases/irq_preempt_set_in_irq.c

@@ -0,0 +1,42 @@
+#include "tb_cxxrtl_io.h"
+#include "hazard3_irq.h"
+
+#define MAX_PRIORITY 15
+#define NUM_IRQS 64
+
+extern uintptr_t _external_irq_table[NUM_IRQS];
+
+void handler(void);
+
+int main() {
+	// Enable external IRQs globally
+	set_csr(mstatus, 0x8);
+	set_csr(mie, 0x800);
+	// Enable one external IRQ at each priority level
+	for (int i = 0; i <= MAX_PRIORITY; ++i) {
+		h3irq_enable(i, true);
+		h3irq_set_priority(i, i);
+		_external_irq_table[i] = (uintptr_t)handler;
+	}
+	// Set off the lowest-priority IRQ. The IRQ handler will then set the
+	// next-lowest, which will preempt it. So on, up to the highest level,
+	// then return all the way back down through the nested frames.
+	tb_puts("Posting first IRQ\n");
+	tb_set_irq_masked(0x1);
+	asm ("wfi");
+
+	tb_puts("Done.\n");
+	return 0;
+}
+
+void handler(void) {
+	int irqnum = h3irq_get_current_irq();
+	tb_printf("Entered IRQ %d\n", irqnum);
+	if (irqnum < MAX_PRIORITY)
+		tb_set_irq_masked(1u << (irqnum + 1));
+	// !!! Get preempted here
+	tb_clr_irq_masked(1u << irqnum);
+	// Make sure context save/restore tracks as expected:
+	irqnum = h3irq_get_current_irq();
+	tb_printf("Exiting IRQ %d\n", irqnum);
+}