Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753980AbcLFRwg (ORCPT ); Tue, 6 Dec 2016 12:52:36 -0500 Received: from mail-vk0-f46.google.com ([209.85.213.46]:36311 "EHLO mail-vk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751710AbcLFRwc (ORCPT ); Tue, 6 Dec 2016 12:52:32 -0500 MIME-Version: 1.0 In-Reply-To: <20161206094000.ql5gjky7ag3b6j7v@pd.tnic> References: <20161206094000.ql5gjky7ag3b6j7v@pd.tnic> From: Andy Lutomirski Date: Tue, 6 Dec 2016 09:52:11 -0800 Message-ID: Subject: Re: [RFC PATCH 5/6] x86/fpu: Fix CPUID-less FPU detection To: Borislav Petkov Cc: Andy Lutomirski , X86 ML , One Thousand Gnomes , "linux-kernel@vger.kernel.org" , Brian Gerst , Matthew Whitehead Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2600 Lines: 68 On Tue, Dec 6, 2016 at 1:40 AM, Borislav Petkov wrote: > On Mon, Dec 05, 2016 at 05:01:14PM -0800, Andy Lutomirski wrote: >> The old code didn't work at all because it adjusted the current caps >> instead of the forced caps. Anything it did would be undone later >> during cpu identification. Fix that and, while we're at it, improve >> the logging and don't bother running it if CPUID is available. >> >> Signed-off-by: Andy Lutomirski >> --- >> arch/x86/kernel/fpu/init.c | 28 ++++++++++++++++------------ >> 1 file changed, 16 insertions(+), 12 deletions(-) >> >> diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c >> index 60dece392b3a..75e1bf3b0319 100644 >> --- a/arch/x86/kernel/fpu/init.c >> +++ b/arch/x86/kernel/fpu/init.c >> @@ -50,29 +50,33 @@ void fpu__init_cpu(void) >> >> /* >> * The earliest FPU detection code. >> - * >> - * Set the X86_FEATURE_FPU CPU-capability bit based on >> - * trying to execute an actual sequence of FPU instructions: >> */ >> static void fpu__init_system_early_generic(struct cpuinfo_x86 *c) >> { >> - unsigned long cr0; >> - u16 fsw, fcw; >> + if (!boot_cpu_has(X86_FEATURE_CPUID) && >> + !test_bit(X86_FEATURE_FPU, (unsigned long *)cpu_caps_cleared)) { > > Flip test and save an indentation level. How? There's that bit at the bottom to worry about. > >> + /* >> + * Set the X86_FEATURE_FPU CPU-capability bit based on >> + * trying to execute an actual sequence of FPU instructions: >> + */ >> >> - fsw = fcw = 0xffff; >> + unsigned long cr0; >> + u16 fsw, fcw; >> >> - cr0 = read_cr0(); >> - cr0 &= ~(X86_CR0_TS | X86_CR0_EM); >> - write_cr0(cr0); >> + fsw = fcw = 0xffff; >> + >> + cr0 = read_cr0(); >> + cr0 &= ~(X86_CR0_TS | X86_CR0_EM); >> + write_cr0(cr0); >> >> - if (!test_bit(X86_FEATURE_FPU, (unsigned long *)cpu_caps_cleared)) { >> asm volatile("fninit ; fnstsw %0 ; fnstcw %1" >> : "+m" (fsw), "+m" (fcw)); >> + pr_info("x86/fpu: FSW=0x%04hx FCW=0x%04hx\n", fsw, fcw); > > Why do we want those in dmesg? Maybe KERN_DEBUG? For debugging, since this code appears busted in every version of Linux I looked at. It's certainly been busted for quite a few years. And this line won't print on any CPU with CPUID, so it's not going to cause widespread spam. I kind of line the idea of being able to ask users of these ancient CPUs to just send in their logs. --Andy