arch: riscv32: fix IRQ handling bugs when benchmarking

The way that CONFIG_EXECUTION_BENCHMARKING=y is handled on this
architecture is incorrect. The goals are:

- call read_timer_start_of_isr() as close as possible to the
  beginning of the ISR
- call read_timer_end_of_isr() after all preparations have
  been made to call the driver-level IRQ handler, but it hasn't
  been called yet

The current implementation could cause kernel crashes, though.

The read_timer_start_of_isr() call is made before saving MEPC or any
SoC-specific context. The MEPC issue is not that big of a deal, but
doing it before saving SoC context could clobber state that hasn't
been saved yet and corrupt the kernel.

One example is a pulpino style RISC-V SoC. Some Pulpino cores have
extra registers that are used for ISA extensions used to generate code
for C loops. There's no guarantee read_timer_start_of_isr() will never
have a loop inside: in fact, the RISC-V User-Level ISA v2.2 explicitly
recommends using a loop to get the 64-bit value of the "cycle" CSR. A
Pulpino-like SoC with a cycle CSR could thus naturally have a
read_timer_start_of_isr() implementation that involves loops. Saving
the loop state before reading the timer would then be needed.

Fix this issue by moving the call to read_timer_start_of_isr to after
all context saving is done. (This is a fairer comparison to Arm
Cortex-M SoCs anyway, since register stacking is performed in hardware
on Cortex M and is done before the first ISR instruction executes.)

The call to read_timer_end_of_isr() has an issue as well: it's called
after the ISR's argument has been stored in a0 and the ISR address is
in t1, but before actually calling the ISR itself.

In the standard RV32I calling convention, both t1 and a0 are caller
saved, so read_timer_end_of_isr() is within its rights to set them to
any garbage, which we'll happily treat as a function and its argument
and execute.

Avoid that possibility by saving the register values to the stack in
this configuration.

Signed-off-by: Marti Bolivar <[email protected]>

arch/riscv32/core/isr.S

@@ -89,9 +89,6 @@ SECTION_FUNC(exception.entry, __irq_wrapper)
 	sw a6, __NANO_ESF_a6_OFFSET(sp)
 	sw a7, __NANO_ESF_a7_OFFSET(sp)
 
-#ifdef CONFIG_EXECUTION_BENCHMARKING
-	call read_timer_start_of_isr
-#endif
 	/* Save MEPC register */
 	csrr t0, mepc
 	sw t0, __NANO_ESF_mepc_OFFSET(sp)
@@ -105,6 +102,10 @@ SECTION_FUNC(exception.entry, __irq_wrapper)
 	jal ra, __soc_save_context
 #endif /* CONFIG_RISCV_SOC_CONTEXT_SAVE */
 
+#ifdef CONFIG_EXECUTION_BENCHMARKING
+	call read_timer_start_of_isr
+#endif
+
 	/*
 	 * Check if exception is the result of an interrupt or not.
 	 * (SOC dependent). Following the RISC-V architecture spec, the MSB
@@ -250,7 +251,13 @@ call_irq:
 	lw t1, 0x04(t0)
 
 #ifdef CONFIG_EXECUTION_BENCHMARKING
+	addi sp, sp, -16
+	sw a0, 0x00(sp)
+	sw t1, 0x04(sp)
 	call read_timer_end_of_isr
+	lw t1, 0x04(sp)
+	lw a0, 0x00(sp)
+	addi sp, sp, 16
 #endif
 	/* Call ISR function */
 	jalr ra, t1