Add two new tests for IRQs-over-Zcmp, and fix a bug they found:

Interrupting the PC-setting step of a cm.popret (only) can sample the return target
as the exception return PC, which will cause the stack pointer adjust to be skipped
when returning from the IRQ. Fix this by making the PC-setting step uninterruptible

(note the PC-setting step is the instruction we execute first out of the group
of instructions specified in the Zc spec as being atomic wrt interrupts. This
does not itself imply that the PC-setting step is uninterruptible, it just
requires that when the PC-setting step retires, all following steps also retire.
However this is not sufficient given the special case logic that allows the jr
ra PC-setting step to execute before the final stack adjust as an optimisation.)

hdl/hazard3_instr_decompress.v

@@ -337,9 +337,11 @@ end else begin: instr_decompress
 			`RVOPC_CM_POPRET: if (~|EXTENSION_ZCMP || zcmp_rlist < 4'h4) begin
 				invalid = 1'b1;
 			end else if (uop_ctr == 4'he) begin
-				// Note we don't set the uop_atomic flag on the first uop in
-				// the uninterruptible sequence -- the rule is *if* one
-				// executes, they all execute. Having none execute is fine.
+				// Note although this is only the first instruction in the uninterruptible sequence,
+				// we mark this instruction as uninterruptible: there is some special case logic to
+				// allow this jump to execute without flushing the final stack adjust uop, which can
+				// cause the wrong exception PC to be sampled if this uop is interrupted.
+				uop_atomic = 1'b1;
 				in_uop_seq = 1'b1;
 				uop_ctr_nxt = uop_ctr + 4'h1;
 				instr_out = `RVOPC_NOZ_JALR | rfmt_rs1(5'h1);

test/sim/common/hazard3_irq.h

@@ -85,5 +85,14 @@ static inline void external_irq_enable(bool en) {
 	}
 }
 
+static inline void timer_irq_enable(bool en) {
+	// mie.mtie
+	if (en) {
+		set_csr(mie, 0x080);
+	}
+	else {
+		clear_csr(mie, 0x080);
+	}
+}
 
 #endif

test/sim/sw_testcases/zcmp_irq_kill_popret.c

@@ -0,0 +1,69 @@
+#include "tb_cxxrtl_io.h"
+#include "hazard3_irq.h"
+#include <stdlib.h>
+
+// Test intent: check that IRQs interrupting PC-setting phase of cm.popret does
+// not cause stack corruption. (This is not covered by zmp_irq_kill)
+
+uint32_t __attribute__((naked)) foreground_task(uint32_t iters) {
+	asm volatile (
+		// Save sp and ra
+		"addi sp, sp, -16\n"
+		"sw s0, 0(sp)\n"
+		"sw ra, 4(sp)\n"
+		"mv s0, sp\n"
+		"li a1, 0xf00b0000\n"
+		// Short push + popret sequence
+	"1:\n"
+		"la ra, 2f\n"
+		".hword 0xb852\n" // cm.push   {ra,s0},-16
+		".hword 0xbe52\n" // cm.popret {ra,s0}, 16
+	"2:\n"
+		// Check we haven't moved the stack pointer or lost our s0 contents
+		"bne s0, sp, 3f\n"
+		"addi a0, a0, -1\n"
+		"addi a1, a1, 1\n"
+		"bgtz a0, 1b\n"
+		// Normal return -- we've been counting up in a1 as we count up in a0, so
+		// return a1 to confirm we iterated the expected number of times
+		"mv a0, a1\n"
+		"lw s0, 0(sp)\n"
+		"lw ra, 4(sp)\n"
+		"addi sp, sp, 16\n"
+		"ret\n"
+	"3:\n"
+		// Uh oh, either sp or s0 changed. Panic!
+		"mv sp, s0\n"
+		"j panic_sp_changed\n"
+	);
+}
+
+void __attribute__((noreturn)) panic_sp_changed(void) {
+	tb_puts("Stack pointer changed, that wasn't supposed to happen!\n");
+	tb_exit(-1);
+	__builtin_unreachable();
+}
+
+void __attribute__((interrupt)) isr_machine_timer(void) {
+	uint dwell = rand() % 301;
+	mm_timer->mtimecmp = mm_timer->mtime + dwell;
+}
+
+int main() {
+	tb_puts("Starting\n");
+	timer_irq_enable(true);
+	global_irq_enable(true);
+	const uint32_t expected_iterations = 1000;
+	tb_printf("Running %u iterations\n", expected_iterations);
+	uint32_t result = foreground_task(expected_iterations);
+	tb_printf("Result: %08x\n", result);
+	tb_assert(result == 0xf00b0000u + expected_iterations, "Wrong number of iterations");
+
+	return 0;
+}
+
+/*EXPECTED-OUTPUT***************************************************************
+Starting
+Running 1000 iterations
+Result: f00b03e8
+*******************************************************************************/

test/sim/sw_testcases/zcmp_irq_kill_popretz.c

@@ -0,0 +1,71 @@
+#include "tb_cxxrtl_io.h"
+#include "hazard3_irq.h"
+#include <stdlib.h>
+
+// Test intent: check that IRQs interrupting PC-setting phase of cm.popretz does
+// not cause stack corruption. (This is not covered by zmp_irq_kill)
+
+uint32_t __attribute__((naked)) foreground_task(uint32_t iters) {
+	asm volatile (
+		// Save sp and ra
+		"addi sp, sp, -16\n"
+		"sw s0, 0(sp)\n"
+		"sw ra, 4(sp)\n"
+		"mv s0, sp\n"
+		// a0 is going to be trashed by the popretz, so use a different arg reg
+		"mv a2, a0\n"
+		"li a1, 0xf00b0000\n"
+		// Short push + popret sequence
+	"1:\n"
+		"la ra, 2f\n"
+		".hword 0xb852\n" // cm.push    {ra,s0},-16
+		".hword 0xbc52\n" // cm.popretz {ra,s0}, 16
+	"2:\n"
+		// Check we haven't moved the stack pointer or lost our s0 contents
+		"bne s0, sp, 3f\n"
+		"addi a2, a2, -1\n"
+		"addi a1, a1, 1\n"
+		"bgtz a2, 1b\n"
+		// Normal return -- we've been counting up in a1 as we count up in a2, so
+		// return a1 to confirm we iterated the expected number of times
+		"mv a0, a1\n"
+		"lw s0, 0(sp)\n"
+		"lw ra, 4(sp)\n"
+		"addi sp, sp, 16\n"
+		"ret\n"
+	"3:\n"
+		// Uh oh, either sp or s0 changed. Panic!
+		"mv sp, s0\n"
+		"j panic_sp_changed\n"
+	);
+}
+
+void __attribute__((noreturn)) panic_sp_changed(void) {
+	tb_puts("Stack pointer changed, that wasn't supposed to happen!\n");
+	tb_exit(-1);
+	__builtin_unreachable();
+}
+
+void __attribute__((interrupt)) isr_machine_timer(void) {
+	uint dwell = rand() % 301;
+	mm_timer->mtimecmp = mm_timer->mtime + dwell;
+}
+
+int main() {
+	tb_puts("Starting\n");
+	timer_irq_enable(true);
+	global_irq_enable(true);
+	const uint32_t expected_iterations = 1000;
+	tb_printf("Running %u iterations\n", expected_iterations);
+	uint32_t result = foreground_task(expected_iterations);
+	tb_printf("Result: %08x\n", result);
+	tb_assert(result == 0xf00b0000u + expected_iterations, "Wrong number of iterations");
+
+	return 0;
+}
+
+/*EXPECTED-OUTPUT***************************************************************
+Starting
+Running 1000 iterations
+Result: f00b03e8
+*******************************************************************************/