Fix/clear MIE bit in initial RISC-V mstatus register. (#57)
* fix: CLEAR MIE BIT IN INITIAL RISC-V MSTATUS VALUE
The MIE bit in the RISC-V MSTATUS register is used to globally enable
or disable interrupts. It is copied into the MPIE bit and cleared
on entry to an interrupt, and then copied back from the MPIE bit on
exit from an interrupt.
When a task is created it is given an initial MSTATUS value that is
derived from the current MSTATUS value with the MPIE bit force to 1,
but the MIE bit is not forced into any state. This change forces
the MIE bit to 0 (interrupts disabled).
Why:
If a task is created before the scheduler is started the MIE bit
will happen to be 0 (interrupts disabled), which is fine. If a
task is created after the scheduler has been started the MIE bit
is set (interrupts enabled), causing interrupts to unintentionally
become enabled inside the interrupt in which the task is first
moved to the running state - effectively breaking a critical
section which in turn could cause a crash if enabling interrupts
causes interrupts to nest. It is only an issue when starting a
newly created task that was created after the scheduler was started.
Related Issues:
https://forums.freertos.org/t/risc-v-port-pxportinitialisestack-issue-about-mstatus-value-onto-the-stack/9622
Co-authored-by: Cobus van Eeden <35851496+cobusve@users.noreply.github.com>
diff --git a/portable/GCC/RISC-V/portASM.S b/portable/GCC/RISC-V/portASM.S
index b9c56a6..f758674 100644
--- a/portable/GCC/RISC-V/portASM.S
+++ b/portable/GCC/RISC-V/portASM.S
@@ -313,11 +313,6 @@
portasmRESTORE_ADDITIONAL_REGISTERS /* Defined in freertos_risc_v_chip_specific_extensions.h to restore any registers unique to the RISC-V implementation. */
- load_x t0, 29 * portWORD_SIZE( sp ) /* mstatus */
- addi t0, t0, 0x08 /* Set MIE bit so the first task starts with interrupts enabled - required as returns with ret not eret. */
- csrrw x0, mstatus, t0 /* Interrupts enabled from here! */
-
- load_x x5, 2 * portWORD_SIZE( sp ) /* t0 */
load_x x6, 3 * portWORD_SIZE( sp ) /* t1 */
load_x x7, 4 * portWORD_SIZE( sp ) /* t2 */
load_x x8, 5 * portWORD_SIZE( sp ) /* s0/fp */
@@ -344,6 +339,12 @@
load_x x29, 26 * portWORD_SIZE( sp ) /* t4 */
load_x x30, 27 * portWORD_SIZE( sp ) /* t5 */
load_x x31, 28 * portWORD_SIZE( sp ) /* t6 */
+
+ load_x x5, 29 * portWORD_SIZE( sp ) /* Initial mstatus into x5 (t0) */
+ addi x5, x5, 0x08 /* Set MIE bit so the first task starts with interrupts enabled - required as returns with ret not eret. */
+ csrrw x0, mstatus, x5 /* Interrupts enabled from here! */
+ load_x x5, 2 * portWORD_SIZE( sp ) /* Initial x5 (t0) value. */
+
addi sp, sp, portCONTEXT_SIZE
ret
.endfunc
@@ -416,6 +417,7 @@
pxPortInitialiseStack:
csrr t0, mstatus /* Obtain current mstatus value. */
+ andi t0, t0, ~0x8 /* Ensure interrupts are disabled when the stack is restored within an ISR. Required when a task is created after the schedulre has been started, otherwise interrupts would be disabled anyway. */
addi t1, x0, 0x188 /* Generate the value 0x1880, which are the MPIE and MPP bits to set in mstatus. */
slli t1, t1, 4
or t0, t0, t1 /* Set MPIE and MPP bits in mstatus value. */
diff --git a/portable/IAR/RISC-V/portASM.s b/portable/IAR/RISC-V/portASM.s
index a330099..050e63b 100644
--- a/portable/IAR/RISC-V/portASM.s
+++ b/portable/IAR/RISC-V/portASM.s
@@ -320,11 +320,6 @@
portasmRESTORE_ADDITIONAL_REGISTERS /* Defined in freertos_risc_v_chip_specific_extensions.h to restore any registers unique to the RISC-V implementation. */
- load_x t0, 29 * portWORD_SIZE( sp ) /* mstatus */
- addi t0, t0, 0x08 /* Set MIE bit so the first task starts with interrupts enabled - required as returns with ret not eret. */
- csrrw x0, CSR_MSTATUS, t0 /* Interrupts enabled from here! */
-
- load_x x5, 2 * portWORD_SIZE( sp ) /* t0 */
load_x x6, 3 * portWORD_SIZE( sp ) /* t1 */
load_x x7, 4 * portWORD_SIZE( sp ) /* t2 */
load_x x8, 5 * portWORD_SIZE( sp ) /* s0/fp */
@@ -351,6 +346,11 @@
load_x x29, 26 * portWORD_SIZE( sp ) /* t4 */
load_x x30, 27 * portWORD_SIZE( sp ) /* t5 */
load_x x31, 28 * portWORD_SIZE( sp ) /* t6 */
+
+ load_x x5, 29 * portWORD_SIZE( sp ) /* Initial mstatus into x5 (t0) */
+ addi x5, x5, 0x08 /* Set MIE bit so the first task starts with interrupts enabled - required as returns with ret not eret. */
+ csrrw x0, CSR_MSTATUS, x5 /* Interrupts enabled from here! */
+ load_x x5, 2 * portWORD_SIZE( sp ) /* Initial x5 (t0) value. */
addi sp, sp, portCONTEXT_SIZE
ret
@@ -421,6 +421,7 @@
pxPortInitialiseStack:
csrr t0, CSR_MSTATUS /* Obtain current mstatus value. */
+ andi t0, t0, ~0x8 /* Ensure interrupts are disabled when the stack is restored within an ISR. Required when a task is created after the schedulre has been started, otherwise interrupts would be disabled anyway. */
addi t1, x0, 0x188 /* Generate the value 0x1880, which are the MPIE and MPP bits to set in mstatus. */
slli t1, t1, 4
or t0, t0, t1 /* Set MPIE and MPP bits in mstatus value. */
