From bac101c98869c7716700d1549f064e02a892cd78 Mon Sep 17 00:00:00 2001 From: RichardBarry <3073890+RichardBarry@users.noreply.github.com> Date: Fri, 1 May 2020 22:35:42 -0700 Subject: [PATCH] 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> --- portable/GCC/RISC-V/portASM.S | 12 +++++++----- portable/IAR/RISC-V/portASM.s | 11 ++++++----- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/portable/GCC/RISC-V/portASM.S b/portable/GCC/RISC-V/portASM.S index b9c56a693..f758674b8 100644 --- a/portable/GCC/RISC-V/portASM.S +++ b/portable/GCC/RISC-V/portASM.S @@ -313,11 +313,6 @@ xPortStartFirstTask: 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 @@ xPortStartFirstTask: 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 @@ xPortStartFirstTask: 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 a33009977..050e63bc6 100644 --- a/portable/IAR/RISC-V/portASM.s +++ b/portable/IAR/RISC-V/portASM.s @@ -320,11 +320,6 @@ xPortStartFirstTask: 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 @@ xPortStartFirstTask: 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 @@ xPortStartFirstTask: 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. */