LoongArch: Fix and simplify fcsr initialization on execve()
authorXi Ruoyao <xry111@xry111.site>
Wed, 17 Jan 2024 04:43:08 +0000 (12:43 +0800)
committerHuacai Chen <chenhuacai@loongson.cn>
Wed, 17 Jan 2024 04:43:08 +0000 (12:43 +0800)
There has been a lingering bug in LoongArch Linux systems causing some
GCC tests to intermittently fail (see Closes link).  I've made a minimal
reproducer:

    zsh% cat measure.s
    .align 4
    .globl _start
    _start:
        movfcsr2gr  $a0, $fcsr0
        bstrpick.w  $a0, $a0, 16, 16
        beqz        $a0, .ok
        break       0
    .ok:
        li.w        $a7, 93
        syscall     0
    zsh% cc mesaure.s -o measure -nostdlib
    zsh% echo $((1.0/3))
    0.33333333333333331
    zsh% while ./measure; do ; done

This while loop should not stop as POSIX is clear that execve must set
fenv to the default, where FCSR should be zero.  But in fact it will
just stop after running for a while (normally less than 30 seconds).
Note that "$((1.0/3))" is needed to reproduce this issue because it
raises FE_INVALID and makes fcsr0 non-zero.

The problem is we are currently relying on SET_PERSONALITY2() to reset
current->thread.fpu.fcsr.  But SET_PERSONALITY2() is executed before
start_thread which calls lose_fpu(0).  We can see if kernel preempt is
enabled, we may switch to another thread after SET_PERSONALITY2() but
before lose_fpu(0).  Then bad thing happens: during the thread switch
the value of the fcsr0 register is stored into current->thread.fpu.fcsr,
making it dirty again.

The issue can be fixed by setting current->thread.fpu.fcsr after
lose_fpu(0) because lose_fpu() clears TIF_USEDFPU, then the thread
switch won't touch current->thread.fpu.fcsr.

The only other architecture setting FCSR in SET_PERSONALITY2() is MIPS.
I've ran a similar test on MIPS with mainline kernel and it turns out
MIPS is buggy, too.  Anyway MIPS do this for supporting different FP
flavors (NaN encodings, etc.) which do not exist on LoongArch.  So for
LoongArch, we can simply remove the current->thread.fpu.fcsr setting
from SET_PERSONALITY2() and do it in start_thread(), after lose_fpu(0).

The while loop failing with the mainline kernel has survived one hour
after this change on LoongArch.

Fixes: 803b0fc5c3f2baa ("LoongArch: Add process management")
Closes: https://github.com/loongson-community/discussions/issues/7
Link: https://lore.kernel.org/linux-mips/7a6aa1bbdbbe2e63ae96ff163fab0349f58f1b9e.camel@xry111.site/
Cc: stable@vger.kernel.org
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
arch/loongarch/include/asm/elf.h
arch/loongarch/kernel/elf.c
arch/loongarch/kernel/process.c

index 9b16a3b8e70608c8765f838cfd21925d4fe51145..f16bd42456e4ccf3ad6c8917165176b8ef5d8f05 100644 (file)
@@ -241,8 +241,6 @@ void loongarch_dump_regs64(u64 *uregs, const struct pt_regs *regs);
 do {                                                                   \
        current->thread.vdso = &vdso_info;                              \
                                                                        \
-       loongarch_set_personality_fcsr(state);                          \
-                                                                       \
        if (personality(current->personality) != PER_LINUX)             \
                set_personality(PER_LINUX);                             \
 } while (0)
@@ -259,7 +257,6 @@ do {                                                                        \
        clear_thread_flag(TIF_32BIT_ADDR);                              \
                                                                        \
        current->thread.vdso = &vdso_info;                              \
-       loongarch_set_personality_fcsr(state);                          \
                                                                        \
        p = personality(current->personality);                          \
        if (p != PER_LINUX32 && p != PER_LINUX)                         \
@@ -340,6 +337,4 @@ extern int arch_elf_pt_proc(void *ehdr, void *phdr, struct file *elf,
 extern int arch_check_elf(void *ehdr, bool has_interpreter, void *interp_ehdr,
                          struct arch_elf_state *state);
 
-extern void loongarch_set_personality_fcsr(struct arch_elf_state *state);
-
 #endif /* _ASM_ELF_H */
index 183e94fc9c69ce8761f3d70b730558bd7f26ff55..0fa81ced28dcdd053cf79f0aaffa8b127df482e1 100644 (file)
@@ -23,8 +23,3 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
 {
        return 0;
 }
-
-void loongarch_set_personality_fcsr(struct arch_elf_state *state)
-{
-       current->thread.fpu.fcsr = boot_cpu_data.fpu_csr0;
-}
index 767d94cce0de07d74892733b339a55dd5e6ded0e..f2ff8b5d591e4fd638109d2c98d75543c01a112c 100644 (file)
@@ -85,6 +85,7 @@ void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long sp)
        regs->csr_euen = euen;
        lose_fpu(0);
        lose_lbt(0);
+       current->thread.fpu.fcsr = boot_cpu_data.fpu_csr0;
 
        clear_thread_flag(TIF_LSX_CTX_LIVE);
        clear_thread_flag(TIF_LASX_CTX_LIVE);