[PATCH] riscv: fpu: refine FPU save flow

[PATCH] riscv: fpu: refine FPU save flow

[Hui Min Mina Chou]

From: Charles Ci-Jyun Wu <dminus@andestech.com>

When Kernel first time run to arch_dup_task_struct(),
it will check if sstatus.FS is dirty. If it is dirty,
then it will do FPU save flow. But this field is
floating currently. Meanwhile if the combination between
platform(HW) and Kernel(SW) about FPU configuration
is mismatch. eq: The platform is without FPU and Kernel
is with FPU. Then Kernel may trigger illegal instruction
here.

Hence it shall check by has_fpu before do FPU save flow
in arch_dup_task_struct().

Signed-off-by: Rick Chen <rick@andestech.com>
Signed-off-by: Charles Ci-Jyun Wu <dminus@andestech.com>
---
 arch/riscv/kernel/process.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 31a392993cb4..a27c21c7d9c6 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -199,7 +199,8 @@ void arch_release_task_struct(struct task_struct *tsk)
 
 int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 {
-	fstate_save(src, task_pt_regs(src));
+	if (has_fpu())
+		fstate_save(src, task_pt_regs(src));
 	*dst = *src;
 	/* clear entire V context, including datap for a new task */
 	memset(&dst->thread.vstate, 0, sizeof(struct __riscv_v_ext_state));
-- 
2.34.1

Re: [PATCH] riscv: fpu: refine FPU save flow

[Vivian Wang]

On 2/5/26 13:53, Hui Min Mina Chou wrote:
> From: Charles Ci-Jyun Wu <dminus@andestech.com>
>
> When Kernel first time run to arch_dup_task_struct(),
> it will check if sstatus.FS is dirty. If it is dirty,
> then it will do FPU save flow. But this field is
> floating currently. Meanwhile if the combination between
> platform(HW) and Kernel(SW) about FPU configuration
> is mismatch. eq: The platform is without FPU and Kernel
> is with FPU. Then Kernel may trigger illegal instruction
> here.

This doesn't really make sense. fstate_save checks for ((regs->status &
SR_FS) == SR_FS_DIRTY). Do you mean that a platform can be !has_fpu(),
yet come up with a task_pt_regs(...)->status that's SR_FS?

Do you have some reproduction steps on how this bug can be triggered?
QEMU should support disabling F/D.

If by "floating" you mean task_pt_regs(src)->status is uninitialized
somehow, that's the bug that needs to be fixed. Adding a band-aid here
just confuses everyone without addressing the problem.

Vivian "dramforever" Wang

Re: [PATCH] riscv: fpu: refine FPU save flow

[Vivian Wang]

On 2/9/26 16:34, Vivian Wang wrote:
> [...]
>
> This doesn't really make sense. fstate_save checks for ((regs->status &
> SR_FS) == SR_FS_DIRTY). Do you mean that a platform can be !has_fpu(),
> yet come up with a task_pt_regs(...)->status that's SR_FS?

Meant to say: SR_FS set to SR_FS_DIRTY

Re: [PATCH] riscv: fpu: refine FPU save flow

[Mina Chou]

Hi Vivian,

Thanks for the review.

You are right; the sstatus was caused by task_pt_regs(p) being incorrectly
handled in older kernels, but the issue has already been fixed by:

riscv: prevent stack corruption by reserving task_pt_regs(p) early 
(merged in Linux v6.5).

I should have checked the latest master before sending this over. 
My apologies for the noise, please ignore this patch.

Best regards,
Mina