linux-kernel - Re: regression: massive trouble with fpu rework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 27 Jun 2015 10:18:55 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Mike Galbraith <umgwanakikbuti@...il.com>
Cc:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	Borislav Petkov <bp@...en8.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: regression: massive trouble with fpu rework


* Mike Galbraith <umgwanakikbuti@...il.com> wrote:

> Hi Ingo,
> 
> My i7-4790 box is having one hell of a time with this merge window, is
> dead in the water.  The netconsole log below is v4.1-7254-gc13c81006314,
> but trouble begins at bisected point much earlier.  If I turn off kvm,
> such that I can kinda sorta boot, systemd says many services "enter
> failed state", box is pretty much a doorstop.  Though I can get to a
> prompt, I can't login.  If kvm is enabled, it explodes as soon as it
> autoloads (wtf does it do that when it's not being used?)
> 
> Bisecting to the beginning of my woes takes me to the below.  Before
> that, it doesn't matter if kvm is enabled or not, all is well.  Below
> the current gripage with kvm disabled, find the kvm explosion, and
> another explosion as I approach the beginning of my box's woes.
> 
> 067051ccd209623cb56152cf4cb06616ee2bcc5c is the first bad commit
> commit 067051ccd209623cb56152cf4cb06616ee2bcc5c
> Author: Ingo Molnar <mingo@...nel.org>
> Date:   Sat Apr 25 08:27:44 2015 +0200
> 
>     x86/fpu: Do system-wide setup from fpu__detect()
>     
>     fpu__cpu_init() is called on every CPU, so it is the wrong place
>     to call fpu__init_system() from. Call it from fpu__detect():
>     this is early CPU init code, but we already have CPU features detected,
>     so we can call the system-wide FPU init code from here.
>     
>     Reviewed-by: Borislav Petkov <bp@...en8.de>
>     Cc: Andy Lutomirski <luto@...capital.net>
>     Cc: Dave Hansen <dave.hansen@...ux.intel.com>
>     Cc: Fenghua Yu <fenghua.yu@...el.com>
>     Cc: H. Peter Anvin <hpa@...or.com>
>     Cc: Linus Torvalds <torvalds@...ux-foundation.org>
>     Cc: Oleg Nesterov <oleg@...hat.com>
>     Cc: Peter Zijlstra <peterz@...radead.org>
>     Cc: Thomas Gleixner <tglx@...utronix.de>
>     Signed-off-by: Ingo Molnar <mingo@...nel.org>

Just as a quick workaround, if you add back a per CPU init fpu__init_system() 
call, as per the disgusting hack below, do things get happier?

( You might trigger a few WARN_ON_ONCE() whinges, especially if you have 
  CONFIG_X86_DEBUG_FPU=y, but those should be one-time warnings that are not 
  fatal. )

Totally untested, unfortunately.

My theory of the bug is that there is something that needs to be set up per CPU, 
which is a side effect of fpu__init_system(), and which the new fpu__init_cpu() 
does not capture. If this patch helps then the real fix would be to figure out 
that side effect.

Thanks,

	Ingo

 arch/x86/kernel/fpu/init.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index fc878fee6a51..421babb08fe6 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -4,6 +4,9 @@
 #include <asm/fpu/internal.h>
 #include <asm/tlbflush.h>
 
+#undef __init
+#define __init
+
 /*
  * Initialize the TS bit in CR0 according to the style of context-switches
  * we are using:
@@ -44,13 +47,18 @@ static void fpu__init_cpu_generic(void)
 /*
  * Enable all supported FPU features. Called when a CPU is brought online:
  */
-void fpu__init_cpu(void)
+static void __fpu__init_cpu(void)
 {
 	fpu__init_cpu_generic();
 	fpu__init_cpu_xstate();
 	fpu__init_cpu_ctx_switch();
 }
 
+void fpu__init_cpu(void)
+{
+	fpu__init_system(NULL);
+}
+
 /*
  * The earliest FPU detection code.
  *
@@ -267,13 +275,14 @@ static void __init fpu__init_system_ctx_switch(void)
  */
 void __init fpu__init_system(struct cpuinfo_x86 *c)
 {
-	fpu__init_system_early_generic(c);
+	if (c)
+		fpu__init_system_early_generic(c);
 
 	/*
 	 * The FPU has to be operational for some of the
 	 * later FPU init activities:
 	 */
-	fpu__init_cpu();
+	__fpu__init_cpu();
 
 	/*
 	 * But don't leave CR0::TS set yet, as some of the FPU setup
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/