Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753196AbcCKJI6 (ORCPT ); Fri, 11 Mar 2016 04:08:58 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:36171 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751407AbcCKJIo (ORCPT ); Fri, 11 Mar 2016 04:08:44 -0500 Date: Fri, 11 Mar 2016 10:08:40 +0100 From: Ingo Molnar To: Andy Lutomirski Cc: Borislav Petkov , Andy Shevchenko , "linux-kernel@vger.kernel.org" , "x86@kernel.org" , Fenghua Yu , Linus Torvalds , "H. Peter Anvin" , Thomas Gleixner , Andrew Morton , Dave Hansen , Oleg Nesterov , "Yu, Yu-cheng" Subject: Re: Got FPU related warning on Intel Quark during boot Message-ID: <20160311090840.GA8486@gmail.com> References: <20160310111935.GB13102@gmail.com> <20160310125610.GA26708@pd.tnic> <20160310145940.GB26708@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3295 Lines: 111 * Andy Lutomirski wrote: > On Thu, Mar 10, 2016 at 6:59 AM, Borislav Petkov wrote: > > On Thu, Mar 10, 2016 at 03:31:43PM +0200, Andy Shevchenko wrote: > >> Looks like it lacks that one. > >> > >> # grep -i fxsr /proc/cpuinfo; echo $? > >> 1 > > > > Ok, so looking at where the warning comes from: > > > > [ 14.714533] WARNING: CPU: 0 PID: 823 at arch/x86/include/asm/fpu/internal.h:163 fpu__clear+0x8c/0x160 > > > > static inline void copy_kernel_to_fxregs(struct fxregs_state *fx) > > { > > int err; > > > > if (config_enabled(CONFIG_X86_32)) { > > err = check_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx)); > > ^^^^^^^^^^^^^^^^^ > > } else { > > > > ... > > > > /* Copying from a kernel buffer to FPU registers should never fail: */ > > WARN_ON_FPU(err); > > > > > > and the stacktrace is pretty clear: > > > > flush_thread > > |-> fpu__clear(&tsk->thread.fpu); > > |-> we are eager by default here: > > > > if (!use_eager_fpu() || !static_cpu_has(X86_FEATURE_FPU)) { > > /* FPU state will be reallocated lazily at the first use. */ > > fpu__drop(fpu); > > } else { > > > > --> we're in that branch. > > > > copy_init_fpstate_to_fpregs(); > > |-> copy_kernel_to_fxregs() > > > > > > I think we should use FRSTOR on quark, i.e., copy_kernel_to_fregs(). > > > > Does this untested wild guess even work? > > > > --- > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > > index dea8e76d60c6..bbafe5e8a1a6 100644 > > --- a/arch/x86/kernel/fpu/core.c > > +++ b/arch/x86/kernel/fpu/core.c > > @@ -474,8 +474,11 @@ static inline void copy_init_fpstate_to_fpregs(void) > > { > > if (use_xsave()) > > copy_kernel_to_xregs(&init_fpstate.xsave, -1); > > - else > > + else if (static_cpu_has(X86_FEATURE_FXSR)) > > copy_kernel_to_fxregs(&init_fpstate.fxsave); > > + else > > + copy_kernel_to_fregs(&init_fpstate.fsave); > > + > > } > > > > This looks wrong, too: > > /* > * Once per bootup FPU initialization sequences that will run on most x86 CPUs: > */ > static void __init fpu__init_system_generic(void) > { > /* > * Set up the legacy init FPU context. (xstate init might overwrite this > * with a more modern format, if the CPU supports it.) > */ > fpstate_init_fxstate(&init_fpstate.fxsave); <-- wrong format on pre-FXSR CPUs Indeed: static inline void fpstate_init_fxstate(struct fxregs_state *fx) { fx->cwd = 0x37f; fx->mxcsr = MXCSR_DEFAULT; } I assumed that the fxstate init is outside the legacy FPU context area, but they overlap: fx->cwd is the first two bytes, and fx->mxcsr overlaps the middle of the legacy area. We do a later fpstate_init_fstate(), which does: static inline void fpstate_init_fstate(struct fregs_state *fp) { fp->cwd = 0xffff037fu; fp->swd = 0xffff0000u; fp->twd = 0xffffffffu; fp->fos = 0xffff0000u; } which accidentally overwrites the cwd bit - but AFAICS fx->mxcsw overlaps the first legacy FPU register? So yes, this needs to be fixed too. Thanks, Ingo