Message ID | 20240102101711.10872-2-xry111@xry111.site |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-14230-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id tb2csp4363043dyb; Tue, 2 Jan 2024 02:18:26 -0800 (PST) X-Google-Smtp-Source: AGHT+IH7XRvLZMOkKDs/IoNDtotCW3RIBETnzuPrTw3ymLNvMHXXTptWNuww+YTTO7LcfMrLYl2b X-Received: by 2002:a05:6a20:72a4:b0:197:1749:d621 with SMTP id o36-20020a056a2072a400b001971749d621mr2293376pzk.75.1704190706393; Tue, 02 Jan 2024 02:18:26 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1704190706; cv=none; d=google.com; s=arc-20160816; b=mwjguPCD4Kt3g6PgrUZPW6PEBb6My/52AEn52VBbEVIr53d/zqUGb3afPIgPA+FlGY 4y1ShdXnQXwgsE3T4qdFAn9M2wJJ37jUNhMB8o63HtnZLnMVnry/ktSCx7BRWt1kmXQA 0Xneha/4q8taBlwRqaY2KlbhDy5K1Vf2bfUNRwdhgn5yyWhC6x7OicO29oH36mteOvCv AnXeLeK/+SH6TEhfs+Frj6Mnvojs2h/RKgPLW+wYyDnc9ft0UQjzJYGOXCCnUWCmfRbZ GcMrLWj6e+thUfAIRDvaPnVcs8dCTVu4ufIjuBzMmk9oonxNS8vheA0rgTXt3qdiRLLb 2+Nw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=9evvBPxCKojHJFJD1eXmZlK80bg2cINtJg4H+v8BObw=; fh=bFWT0OjK1S4mdPKBqHOHbI7VZvQcJcj1GEvvcPyamM0=; b=hvuJRcTHlcNN60fkTAg/UM80x8gKYpHGRZoF6/kqxm0lHYV1YiUO0kTR19Ro+hOO5H ncxv78GWAKjSeuLoDeCu3AexH4F1+mjcwHYp6wYa3DnGUKcOJ0q5AXyEsWSmAx8DKm2/ zRT/nAz4kGKdg3BBOJvdArKR4jIRJzQY+8GBajtc5CHIEkev+N72/rZfm9mbyNtHhIvC qTT+Ut4W5rvZi6gs45VUCelWHEQyyVWKbgcqQIuwefC10VDCeeB1kSwDlip88cmHAOpG 34EYpoLLFJiqNE5OmBVK9txY+TuA867E4K8pyf7omdCWoLxb2iN6M5t69iOokZI/qJ86 CGag== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@xry111.site header.s=default header.b=BX985uIF; spf=pass (google.com: domain of linux-kernel+bounces-14230-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-14230-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=xry111.site Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id 5-20020a630f45000000b005cda4c18b54si17214775pgp.364.2024.01.02.02.18.26 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Jan 2024 02:18:26 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-14230-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@xry111.site header.s=default header.b=BX985uIF; spf=pass (google.com: domain of linux-kernel+bounces-14230-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-14230-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=xry111.site Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 30B65282EA6 for <ouuuleilei@gmail.com>; Tue, 2 Jan 2024 10:18:26 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 46C09E555; Tue, 2 Jan 2024 10:17:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=xry111.site header.i=@xry111.site header.b="BX985uIF" X-Original-To: linux-kernel@vger.kernel.org Received: from xry111.site (xry111.site [89.208.246.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 52D82DF6A; Tue, 2 Jan 2024 10:17:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=xry111.site Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=xry111.site DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=xry111.site; s=default; t=1704190660; bh=uf8dTMlebu51EEyODmvtuTNr9cflN5DSEjldk9W9h34=; h=From:To:Cc:Subject:Date:From; b=BX985uIFo3e6UGkmqcICVT/dwtYlVpg8OS6/Uj5fCTXQdj6/pg88XQNip2xKm5YWx LZXtYyQv8zoZ2mxu3sylfE6XgsSp8FHlKcVbOr5MARlXKtE1LocLlArj9eS/hpdPkd 0q3n5fOgCe9CY47rMwF/khW6llIwjkfiU/PuVlkI= Received: from stargazer.. (unknown [IPv6:240e:358:11a9:2200:dc73:854d:832e:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (Client did not present a certificate) (Authenticated sender: xry111@xry111.site) by xry111.site (Postfix) with ESMTPSA id 79D4666BFB; Tue, 2 Jan 2024 05:17:35 -0500 (EST) From: Xi Ruoyao <xry111@xry111.site> To: Huacai Chen <chenhuacai@kernel.org>, WANG Xuerui <kernel@xen0n.name> Cc: Eric Biederman <ebiederm@xmission.com>, Kees Cook <keescook@chromium.org>, Tiezhu Yang <yangtiezhu@loongson.cn>, Jinyang He <hejinyang@loongson.cn>, Jiaxun Yang <jiaxun.yang@flygoat.com>, loongarch@lists.linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Xi Ruoyao <xry111@xry111.site>, stable@vger.kernel.org Subject: [PATCH v2] LoongArch: Fix and simplify fcsr initialization on execve Date: Tue, 2 Jan 2024 18:17:12 +0800 Message-ID: <20240102101711.10872-2-xry111@xry111.site> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1786909602282481319 X-GMAIL-MSGID: 1786973474185191328 |
Series |
[v2] LoongArch: Fix and simplify fcsr initialization on execve
|
|
Commit Message
Xi Ruoyao
Jan. 2, 2024, 10:17 a.m. UTC
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 the issue because it
raises FE_INVALID and makes fcsr0 non-zero.
The problem is we are 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.
They do this for supporting different FP flavors (NaN encodings etc).
which do not exist on LoongArch. I'm not sure how MIPS evades the issue
(or maybe it's just buggy too) but I'll investigate it later.
For LoongArch, just 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.
Closes: https://github.com/loongson-community/discussions/issues/7
Fixes: 803b0fc5c3f2 ("LoongArch: Add process management")
Cc: stable@vger.kernel.org
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
v1 -> v2:
- Still set current->thread.fpu.fcsr to boot_cpu_data.fpu_csr0 instead
of constant 0.
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(-)
Comments
On Tue, 2024-01-02 at 18:17 +0800, Xi Ruoyao wrote: > The only other architecture setting FCSR in SET_PERSONALITY2 is MIPS. > They do this for supporting different FP flavors (NaN encodings etc). > which do not exist on LoongArch. I'm not sure how MIPS evades the issue > (or maybe it's just buggy too) but I'll investigate it later. Phew. I just managed to recommission my 3A4000 and I can reproduce the issue as well with Linux 5.18.1 (the latest kernel release when I decommissioned it) and CONFIG_PREEMPT=y. % cat measure.c #include <fenv.h> int main() { return fetestexcept(FE_INEXACT); } % echo $((1./3)) 0.33333333333333331 % while ./a.out; do ; done (stopped in seconds) I'm building the mainline kernel on the 3A4000 now, will see if the issue still exists...
On Tue, 2024-01-02 at 18:25 +0800, Xi Ruoyao wrote: > On Tue, 2024-01-02 at 18:17 +0800, Xi Ruoyao wrote: > > The only other architecture setting FCSR in SET_PERSONALITY2 is MIPS. > > They do this for supporting different FP flavors (NaN encodings etc). > > which do not exist on LoongArch. I'm not sure how MIPS evades the issue > > (or maybe it's just buggy too) but I'll investigate it later. > > Phew. I just managed to recommission my 3A4000 and I can reproduce the > issue as well with Linux 5.18.1 (the latest kernel release when I > decommissioned it) and CONFIG_PREEMPT=y. > > % cat measure.c > #include <fenv.h> > int main() { return fetestexcept(FE_INEXACT); } > > % echo $((1./3)) > 0.33333333333333331 > > % while ./a.out; do ; done > (stopped in seconds) > > I'm building the mainline kernel on the 3A4000 now, will see if the > issue still exists... Still happening with 6.7.0-rc8. I'm not sure how to fix it for MIPS. Maybe lose_fpu in SET_PERSONALITY2? But to me doing so will be really nasty. Anyway I'll leave this for MIPS maintainers.
On Tue, Jan 2, 2024 at 6:17 PM Xi Ruoyao <xry111@xry111.site> 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 the issue because it > raises FE_INVALID and makes fcsr0 non-zero. > > The problem is we are 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. > They do this for supporting different FP flavors (NaN encodings etc). > which do not exist on LoongArch. I'm not sure how MIPS evades the issue > (or maybe it's just buggy too) but I'll investigate it later. You have already investigated it, so should this message be updated? Huacai > > For LoongArch, just 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. > > Closes: https://github.com/loongson-community/discussions/issues/7 > Fixes: 803b0fc5c3f2 ("LoongArch: Add process management") > Cc: stable@vger.kernel.org > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > --- > > v1 -> v2: > - Still set current->thread.fpu.fcsr to boot_cpu_data.fpu_csr0 instead > of constant 0. > > 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 767d94cce0de..3f9cae615f52 100644 > --- a/arch/loongarch/kernel/process.c > +++ b/arch/loongarch/kernel/process.c > @@ -92,6 +92,7 @@ void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long sp) > clear_used_math(); > regs->csr_era = pc; > regs->regs[3] = sp; > + current->thread.fpu.fcsr = boot_cpu_data.fpu_csr0; > } > > void flush_thread(void) > -- > 2.43.0 >
On Tue, Jan 2, 2024 at 6:48 PM Xi Ruoyao <xry111@xry111.site> wrote: > > On Tue, 2024-01-02 at 18:25 +0800, Xi Ruoyao wrote: > > On Tue, 2024-01-02 at 18:17 +0800, Xi Ruoyao wrote: > > > The only other architecture setting FCSR in SET_PERSONALITY2 is MIPS. > > > They do this for supporting different FP flavors (NaN encodings etc). > > > which do not exist on LoongArch. I'm not sure how MIPS evades the issue > > > (or maybe it's just buggy too) but I'll investigate it later. > > > > Phew. I just managed to recommission my 3A4000 and I can reproduce the > > issue as well with Linux 5.18.1 (the latest kernel release when I > > decommissioned it) and CONFIG_PREEMPT=y. > > > > % cat measure.c > > #include <fenv.h> > > int main() { return fetestexcept(FE_INEXACT); } > > > > % echo $((1./3)) > > 0.33333333333333331 > > > > % while ./a.out; do ; done > > (stopped in seconds) > > > > I'm building the mainline kernel on the 3A4000 now, will see if the > > issue still exists... > > Still happening with 6.7.0-rc8. I'm not sure how to fix it for MIPS. > Maybe lose_fpu in SET_PERSONALITY2? But to me doing so will be really > nasty. Anyway I'll leave this for MIPS maintainers. Disable preemption in SET_PERSONALITY2 and enable in START_THREAD? Huacai > > -- > Xi Ruoyao <xry111@xry111.site> > School of Aerospace Science and Technology, Xidian University
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..3f9cae615f52 100644 --- a/arch/loongarch/kernel/process.c +++ b/arch/loongarch/kernel/process.c @@ -92,6 +92,7 @@ void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long sp) clear_used_math(); regs->csr_era = pc; regs->regs[3] = sp; + current->thread.fpu.fcsr = boot_cpu_data.fpu_csr0; } void flush_thread(void)