[BUG] Incorrect FPU restore for RISC-V

[3reality-wangzhijie]

Describe the bug
A concise description of what the bug is.

When restoring the FPU context from the stack, we first restore the mstatus register, during which the FS field of mstatus is modified. However, according to the RISC-V Privileged Architecture Specification (20211203), in some implementations, writing either Initial or Clean to this field may result in its actual value becoming Dirty.

Specifically, after a task is created, the FS field value of the mstatus register saved in the stack is Clean;

FreeRTOS-Kernel/portable/GCC/RISC-V/portASM.S

Lines 229 to 234 in e5987bb

	#if( configENABLE_FPU == 1 )
	/* Mark the FPU as clean in the mstatus value. */
	li t1, ~MSTATUS_FS_MASK
	and t0, t0, t1
	li t1, MSTATUS_FS_CLEAN
	or t0, t0, t1

but when mstatus is restored during the first run, the FS field may turn to Dirty due to chip-specific implementations.

FreeRTOS-Kernel/portable/GCC/RISC-V/portContext.h

Lines 400 to 422 in e5987bb

	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
	#if( configENABLE_VPU == 1 )
	csrr t0, mstatus
	srl t1, t0, 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. */
	portcontextRESTORE_VPU_CONTEXT
	5:
	#endif /* ifdef portasmSTORE_VPU_CONTEXT */
	#if( configENABLE_FPU == 1 )
	csrr t0, mstatus
	srl t1, t0, 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. */

This causes the subsequent process to erroneously restore the FPU register set from the stack—yet in reality, we did not save the FPU register set to the stack when creating the task. Which in turn leads to an incorrect runtime environment (including incorrect input parameters)

Target

• bl702 (risc-v)

Host

• Host OS: Arch Linux

To Reproduce

• Use project ... and configure with ...
• Run on ... and could observe ...

Expected behavior
A concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.
e.g. code snippet to reproduce the issue.
e.g. stack trace, memory dump, debugger log, and many etc.

[3reality-wangzhijie]

main...3reality-wangzhijie:FreeRTOS-Kernel:main

This modification can resolve the issue I'm currently facing, but it's unclear whether it applies to implementations on other platforms.

[aggarg]

Thank you for reporting the issue. The solution you mention will not work on all the platforms as portasmRESTORE_ADDITIONAL_REGISTERS may modify t0 on some platforms -

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

Line 93 in e5987bb

lw t0, 2 * portWORD_SIZE( sp ) /* Load additional registers into accessible temporary registers. */

How about setting the FPU state to OFF at the time of task creation?

FreeRTOS-Kernel/portable/GCC/RISC-V/portASM.S

Line 233 in e5987bb

li t1, MSTATUS_FS_CLEAN

[3reality-wangzhijie]

Thank you for reporting the issue. The solution you mention will not work on all the platforms as portasmRESTORE_ADDITIONAL_REGISTERS may modify t0 on some platforms -

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

Line 93 in e5987bb
lw t0, 2 * portWORD_SIZE( sp ) /* Load additional registers into accessible temporary registers. */

How about setting the FPU state to OFF at the time of task creation?

FreeRTOS-Kernel/portable/GCC/RISC-V/portASM.S

Line 233 in e5987bb
li t1, MSTATUS_FS_CLEAN

Simply setting FS to OFF may trigger an exception, so additional preliminary or supporting operations are required.

[Saiiijchan]

If I understand correctly, portasmRESTORE_ADDITIONAL_REGISTERS will affect your FS state and cause mstatus to change.
How about re-reading mstatus from the task stack to t0？

[aggarg]

@Saiiijchan Yes, you are right. Because of the way portasmSAVE_ADDITIONAL_REGISTERS and portasmRESTORE_ADDITIONAL_REGISTERS are implemented, it requires changing the location of mstatus slot in the context.

@3reality-wangzhijie I have made this change in the following PR - #1330. Would you please give it a try and see if it fixes your issue?