Use saved mstatus for FPU/VPU state determination (#1330)

According to the RISC-V Privileged Architecture Specification (20211203),
writing Initial or Clean to the FS field of mstatus may result in the FS
value getting set to Dirty in some implementations. This means we cannot
rely on reading back the same FS value after writing to mstatus.

Previously, the context restore code would:
1. Write an FS value to mstatus
2. Read mstatus again at a later point
3. Use the read FS value to determine FPU status

This change updates the context restore code to use the mstatus value
from the saved context instead of re-reading mstatus after writing to
it. This required chaning the location of the mstatus slot in the
context.

Fixes: #1327

Signed-off-by: Gaurav Aggarwal <[email protected]>

Author: aggarg

portable/GCC/RISC-V/chip_specific_extensions/Pulpino_Vega_RV32M1RM/freertos_risc_v_chip_specific_extensions.h

@@ -80,22 +80,22 @@ csrr t2, lpcount0
 csrr t3, lpstart1
 csrr t4, lpend1
 csrr t5, lpcount1
-sw t0, 2 * portWORD_SIZE( sp )
-sw t1, 3 * portWORD_SIZE( sp )
-sw t2, 4 * portWORD_SIZE( sp )
-sw t3, 5 * portWORD_SIZE( sp )
-sw t4, 6 * portWORD_SIZE( sp )
-sw t5, 7 * portWORD_SIZE( sp )
+sw t0, 1 * portWORD_SIZE( sp )
+sw t1, 2 * portWORD_SIZE( sp )
+sw t2, 3 * portWORD_SIZE( sp )
+sw t3, 4 * portWORD_SIZE( sp )
+sw t4, 5 * portWORD_SIZE( sp )
+sw t5, 6 * portWORD_SIZE( sp )
    .endm
 
 /* Restore the additional registers found on the Pulpino. */
    .macro portasmRESTORE_ADDITIONAL_REGISTERS
-lw t0, 2 * portWORD_SIZE( sp ) /* Load additional registers into accessible temporary registers. */
-lw t1, 3 * portWORD_SIZE( sp )
-lw t2, 4 * portWORD_SIZE( sp )
-lw t3, 5 * portWORD_SIZE( sp )
-lw t4, 6 * portWORD_SIZE( sp )
-lw t5, 7 * portWORD_SIZE( sp )
+lw t0, 1 * portWORD_SIZE( sp ) /* Load additional registers into accessible temporary registers. */
+lw t1, 2 * portWORD_SIZE( sp )
+lw t2, 3 * portWORD_SIZE( sp )
+lw t3, 4 * portWORD_SIZE( sp )
+lw t4, 5 * portWORD_SIZE( sp )
+lw t5, 6 * portWORD_SIZE( sp )
 csrw lpstart0, t0
 csrw lpend0, t1
 csrw lpcount0, t2

portable/GCC/RISC-V/portASM.S

@@ -193,8 +193,8 @@ definitions. */
  * portTASK_RETURN_ADDRESS
  * [FPU registers (when enabled/available) go here]
  * [VPU registers (when enabled/available) go here]
- * [chip specific registers go here]
  * mstatus
+ * [chip specific registers go here]
  * pxCode
  */
 pxPortInitialiseStack:
@@ -212,14 +212,6 @@ pxPortInitialiseStack:
     load_x t0, xTaskReturnAddress
     store_x t0, 0(a0)                   /* Return address onto the stack. */
 
-    addi t0, x0, portasmADDITIONAL_CONTEXT_SIZE /* The number of chip specific additional registers. */
-chip_specific_stack_frame:              /* First add any chip specific registers to the stack frame being created. */
-    beq t0, x0, 1f                      /* No more chip specific registers to save. */
-    addi a0, a0, -portWORD_SIZE         /* Make space for chip specific register. */
-    store_x x0, 0(a0)                   /* Give the chip specific register an initial value of zero. */
-    addi t0, t0, -1                     /* Decrement the count of chip specific registers remaining. */
-    j chip_specific_stack_frame         /* Until no more chip specific registers. */
-1:
     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 scheduler has been started, otherwise interrupts would be disabled anyway. */
     addi t1, x0, 0x188                  /* Generate the value 0x1880, which are the MPIE=1 and MPP=M_Mode in mstatus. */
@@ -245,6 +237,15 @@ chip_specific_stack_frame:              /* First add any chip specific registers
     addi a0, a0, -portWORD_SIZE
     store_x t0, 0(a0)                   /* mstatus onto the stack. */
 
+    addi t0, x0, portasmADDITIONAL_CONTEXT_SIZE /* The number of chip specific additional registers. */
+chip_specific_stack_frame:              /* First add any chip specific registers to the stack frame being created. */
+    beq t0, x0, 1f                      /* No more chip specific registers to save. */
+    addi a0, a0, -portWORD_SIZE         /* Make space for chip specific register. */
+    store_x x0, 0(a0)                   /* Give the chip specific register an initial value of zero. */
+    addi t0, t0, -1                     /* Decrement the count of chip specific registers remaining. */
+    j chip_specific_stack_frame         /* Until no more chip specific registers. */
+1:
+
     addi a0, a0, -portWORD_SIZE
     store_x a1, 0(a0)                   /* mret value (pxCode parameter) onto the stack. */
     ret
@@ -256,12 +257,12 @@ xPortStartFirstTask:
 
     load_x  x1, 0( sp ) /* Note for starting the scheduler the exception return address is used as the function return address. */
 
+    portasmRESTORE_ADDITIONAL_REGISTERS /* Defined in freertos_risc_v_chip_specific_extensions.h to restore any registers unique to the RISC-V implementation. */
+
     load_x  x5, 1 * 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. */
     csrw    mstatus, x5                 /* Interrupts enabled from here! */
 
-    portasmRESTORE_ADDITIONAL_REGISTERS /* Defined in freertos_risc_v_chip_specific_extensions.h to restore any registers unique to the RISC-V implementation. */
-
     load_x  x7,  5  * portWORD_SIZE( sp )   /* t2 */
     load_x  x8,  6  * portWORD_SIZE( sp )   /* s0/fp */
     load_x  x9,  7  * portWORD_SIZE( sp )   /* s1 */

portable/GCC/RISC-V/portContext.h

@@ -322,17 +322,17 @@ store_x t0, portCRITICAL_NESTING_OFFSET * portWORD_SIZE( sp ) /* Store the criti
     srl t1, t0, MSTATUS_VS_OFFSET
     andi t1, t1, 3
     addi t2, x0, 3
-    bne t1, t2, 2f /* If VPU status is not dirty, do not save FPU registers. */
+    bne t1, t2, 2f /* If VPU status is not dirty, do not save VPU registers. */
 
     portcontexSAVE_VPU_CONTEXT
 2:
 #endif
 
-portasmSAVE_ADDITIONAL_REGISTERS /* Defined in freertos_risc_v_chip_specific_extensions.h to save any registers unique to the RISC-V implementation. */
-
 csrr t0, mstatus
 store_x t0, 1 * portWORD_SIZE( sp )
 
+portasmSAVE_ADDITIONAL_REGISTERS /* Defined in freertos_risc_v_chip_specific_extensions.h to save any registers unique to the RISC-V implementation. */
+
 #if( configENABLE_FPU == 1 )
     /* Mark the FPU as clean, if it was dirty and we saved FPU registers. */
     srl t1, t0, MSTATUS_FS_OFFSET
@@ -396,16 +396,17 @@ load_x sp, 0 ( t1 )     /* Read sp from first TCB member. */
 load_x t0, 0 ( sp )
 csrw mepc, t0
 
-/* Restore mstatus register. */
-load_x t0, 1 * portWORD_SIZE( sp )
-csrw mstatus, t0
-
 /* Defined in freertos_risc_v_chip_specific_extensions.h to restore any registers unique to the RISC-V implementation. */
 portasmRESTORE_ADDITIONAL_REGISTERS
 
+/* Restore mstatus register. It is important to use t3 (and not t0) here as t3
+ * is not clobbered by portcontextRESTORE_VPU_CONTEXT and
+ * portcontextRESTORE_FPU_CONTEXT. */
+load_x t3, 1 * portWORD_SIZE( sp )
+csrw mstatus, t3
+
 #if( configENABLE_VPU == 1 )
-    csrr t0, mstatus
-    srl t1, t0, MSTATUS_VS_OFFSET
+    srl t1, t3, MSTATUS_VS_OFFSET
     andi t1, t1, 3
     addi t2, x0, 3
     bne t1, t2, 5f /* If VPU status is not dirty, do not restore VPU registers. */
@@ -415,8 +416,7 @@ portasmRESTORE_ADDITIONAL_REGISTERS
 #endif /* ifdef portasmSTORE_VPU_CONTEXT */
 
 #if( configENABLE_FPU == 1 )
-    csrr t0, mstatus
-    srl t1, t0, MSTATUS_FS_OFFSET
+    srl t1, t3, MSTATUS_FS_OFFSET
     andi t1, t1, 3
     addi t2, x0, 3
     bne t1, t2, 6f /* If FPU status is not dirty, do not restore FPU registers. */