linux-kernel - Re: [RFC PATCH 5/6] x86/fpu: Fix CPUID-less FPU detection

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]

Message-ID: <20161206094000.ql5gjky7ag3b6j7v@pd.tnic>
Date:   Tue, 6 Dec 2016 10:40:00 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     x86@...nel.org, One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Brian Gerst <brgerst@...il.com>,
        Matthew Whitehead <tedheadster@...il.com>
Subject: Re: [RFC PATCH 5/6] x86/fpu: Fix CPUID-less FPU detection

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 <luto@...nel.org>
> ---
>  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.

> +		/*
> +		 * 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?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.