public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] LoongArch: Fix and simplify fcsr initialization on execve()" failed to apply to 6.1-stable tree
@ 2024-01-22 19:17 gregkh
  2024-01-22 19:28 ` [PATCH 6.1.y] LoongArch: Fix and simplify fcsr initialization on execve() Xi Ruoyao
  0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2024-01-22 19:17 UTC (permalink / raw)
  To: xry111, chenhuacai; +Cc: stable


The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x c2396651309eba291c15e32db8fbe44c738b5921
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024012237-handed-control-646a@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..

Possible dependencies:

c2396651309e ("LoongArch: Fix and simplify fcsr initialization on execve()")
bd3c5798484a ("LoongArch: Add Loongson Binary Translation (LBT) extension support")
616500232e63 ("LoongArch: Add vector extensions support")
38bb46f94544 ("LoongArch: Prepare for assemblers with proper FCSR class support")
c23e7f01cf62 ("LoongArch: Relay BCE exceptions to userland as SIGSEGV with si_code=SEGV_BNDERR")
1a69f7a161a7 ("LoongArch: ptrace: Expose hardware breakpoints to debuggers")
edffa33c7bb5 ("LoongArch: Add hardware breakpoints/watchpoints support")
41596803302d ("LoongArch: Make -mstrict-align configurable")
c5ac25e0d78a ("LoongArch: Strip guess unwinder out from prologue unwinder")
5bb8d34449c4 ("LoongArch: Use correct sp value to get graph addr in stack unwinders")
429a9671f235 ("LoongArch: Get frame info in unwind_start() when regs is not available")
28ac0a9e04d7 ("LoongArch: modules/ftrace: Initialize PLT at load time")
a51ac5246d25 ("LoongArch/ftrace: Add HAVE_FUNCTION_GRAPH_RET_ADDR_PTR support")
8778ba2c8a5d ("LoongArch/ftrace: Add HAVE_DYNAMIC_FTRACE_WITH_REGS support")
5fcfad3d41cc ("LoongArch/ftrace: Add dynamic function graph tracer support")
4733f09d8807 ("LoongArch/ftrace: Add dynamic function tracer support")
dbe3ba3018ec ("LoongArch/ftrace: Add basic support")
9151dde40356 ("LoongArch: module: Use got/plt section indices for relocations")
19e5eb15b00c ("LoongArch: Add alternative runtime patching mechanism")
61a6fccc0bd2 ("LoongArch: Add unaligned access support")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From c2396651309eba291c15e32db8fbe44c738b5921 Mon Sep 17 00:00:00 2001
From: Xi Ruoyao <xry111@xry111.site>
Date: Wed, 17 Jan 2024 12:43:08 +0800
Subject: [PATCH] LoongArch: Fix and simplify fcsr initialization on execve()

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>

diff --git a/arch/loongarch/include/asm/elf.h b/arch/loongarch/include/asm/elf.h
index 9b16a3b8e706..f16bd42456e4 100644
--- a/arch/loongarch/include/asm/elf.h
+++ b/arch/loongarch/include/asm/elf.h
@@ -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 */
diff --git a/arch/loongarch/kernel/elf.c b/arch/loongarch/kernel/elf.c
index 183e94fc9c69..0fa81ced28dc 100644
--- a/arch/loongarch/kernel/elf.c
+++ b/arch/loongarch/kernel/elf.c
@@ -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;
-}
diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
index 767d94cce0de..f2ff8b5d591e 100644
--- a/arch/loongarch/kernel/process.c
+++ b/arch/loongarch/kernel/process.c
@@ -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);


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 6.1.y] LoongArch: Fix and simplify fcsr initialization on execve()
  2024-01-22 19:17 FAILED: patch "[PATCH] LoongArch: Fix and simplify fcsr initialization on execve()" failed to apply to 6.1-stable tree gregkh
@ 2024-01-22 19:28 ` Xi Ruoyao
  2024-01-22 19:50   ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Xi Ruoyao @ 2024-01-22 19:28 UTC (permalink / raw)
  To: stable; +Cc: WANG Xuerui, Xi Ruoyao, Huacai Chen

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>
(cherry picked from commit c2396651309eba291c15e32db8fbe44c738b5921)
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---

The conflict is because 6.1.y does not have LBT support, thus there is
no lose_lbt() line.  Resolved manually.

 arch/loongarch/include/asm/elf.h | 5 -----
 arch/loongarch/kernel/elf.c      | 5 -----
 arch/loongarch/kernel/process.c  | 1 +
 3 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/loongarch/include/asm/elf.h b/arch/loongarch/include/asm/elf.h
index 9b16a3b8e706..f16bd42456e4 100644
--- a/arch/loongarch/include/asm/elf.h
+++ b/arch/loongarch/include/asm/elf.h
@@ -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 */
diff --git a/arch/loongarch/kernel/elf.c b/arch/loongarch/kernel/elf.c
index 183e94fc9c69..0fa81ced28dc 100644
--- a/arch/loongarch/kernel/elf.c
+++ b/arch/loongarch/kernel/elf.c
@@ -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;
-}
diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
index 90a5de746332..1259bc312979 100644
--- a/arch/loongarch/kernel/process.c
+++ b/arch/loongarch/kernel/process.c
@@ -82,6 +82,7 @@ void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long sp)
 	euen = regs->csr_euen & ~(CSR_EUEN_FPEN);
 	regs->csr_euen = euen;
 	lose_fpu(0);
+	current->thread.fpu.fcsr = boot_cpu_data.fpu_csr0;
 
 	clear_thread_flag(TIF_LSX_CTX_LIVE);
 	clear_thread_flag(TIF_LASX_CTX_LIVE);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.1.y] LoongArch: Fix and simplify fcsr initialization on execve()
  2024-01-22 19:28 ` [PATCH 6.1.y] LoongArch: Fix and simplify fcsr initialization on execve() Xi Ruoyao
@ 2024-01-22 19:50   ` Greg KH
  0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2024-01-22 19:50 UTC (permalink / raw)
  To: Xi Ruoyao; +Cc: stable, WANG Xuerui, Huacai Chen

On Tue, Jan 23, 2024 at 03:28:04AM +0800, Xi Ruoyao wrote:
> 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>
> (cherry picked from commit c2396651309eba291c15e32db8fbe44c738b5921)
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
> 
> The conflict is because 6.1.y does not have LBT support, thus there is
> no lose_lbt() line.  Resolved manually.

Now queued up, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-01-22 19:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-22 19:17 FAILED: patch "[PATCH] LoongArch: Fix and simplify fcsr initialization on execve()" failed to apply to 6.1-stable tree gregkh
2024-01-22 19:28 ` [PATCH 6.1.y] LoongArch: Fix and simplify fcsr initialization on execve() Xi Ruoyao
2024-01-22 19:50   ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox