RISC-V port pxPortInitialiseStack() issue about "mstatus" value onto the stack

It seems there is a bug in RISC-V port pxPortInitialiseStack().

In RISC-V port pxPortInitialiseStack() implementation, the mstatus value onto the stack is the current mstatus value with MPIE and MPP bits(0x1880) set. The pxPortInitialiseStack() might be called with mstatus.MIE enabled if a task is created by another task by calling xTaskCreate(). So I consider the MIE bit of the mstatus value onto the stack should be also cleared.

pxPortInitialiseStack:

    csrr t0, mstatus                                        /* Obtain current mstatus value. */
 +  andi t0, t0, ~0x8                                       /* Clear MIE bit to avoid it is set when task is created by another task. */
    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. */

    addi a0, a0, -portWORD_SIZE
    store_x t0, 0(a0)                                       /* mstatus onto the stack. */

If the MIE bit of the stored mstatus is NOT cleared, it will cause the interrupt enabled while restoring mstatus from stack in processed_source() of portASM.S file and break the critical section.

processed_source:

    ...
    ...
    /* Load mret with the address of the next instruction in the task to run next. */
    load_x t0, 0( sp )
    csrw mepc, t0

    portasmRESTORE_ADDITIONAL_REGISTERS     /* Defined in freertos_risc_v_chip_specific_extensions.h to restore any registers unique to the RISC-V implementation. */

    /* Load mstatus with the interrupt enable bits used by the task. */
    load_x  t0, 29 * portWORD_SIZE( sp )
    csrw mstatus, t0                                                /* Required for MPIE bit. */
    ...

This is one of the fun support requests :thinking:

I need to dive a little deeper into this but I think there would then be an issue with starting the first task, as that uses a ret instruction, not an mret, in which case the MIE bit would need to be set. The update might not be as simple as clearing the bit when creating the initial stack context, but also require a different method of starting the first task.

Hi Richard,

It would NOT be an issue in starting the first task.
Because the MIE bit will be set in xPortStartFirstTask(), so the first task starts with interrupts enabled. After the first task starts, other tasks are performed by processed_source() and the mstatus will be updated to the stored value (MIE is cleared) in stack which prepared in pxPortInitialiseStack() before mret.

I need to reassess by I don’t think this:

is the case, as interrupts are deliberately left disabled once you start using the FreeRTOS API until the scheduler is started. That is to avoid a once common cause of system crashes whereby an interrupt tried to use the scheduler before it was started.

However, after writing my last post I had the thought that the way the scheduler is started need not be changed much - all that would be needed is for interrupts to be manually enabled before the ret, because at that point the first task is effectively running already (its stack frame is set up, the kernel data structures are in a consistent state, etc.), it just hasn’t reached its entry point yet…I think that should work. So two changes, one as per your initial post to clear the MIE bit, and then setting the MIE bit manually immediately before the ret instruction that mimics returning to the task’s entry point.

Will give this a go.

The issue impacted tasks that had been created after the scheduler had been started only.

The MIE bit was already being set before the ret instruction in the function used to start the first task. I updated that function so the MIE bit was set as one of the last actions which reduced the potential for additional stack use should an interrupt occur between the MIE bit being set and the stack pointer being reset while the first task was being started.