[PATCH] x86: fdiv bug detection fix

[PATCH] x86: fdiv bug detection fix

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>

The fdiv detection code writes s32 integer into
the boot_cpu_data.fdiv_bug. 
However, the boot_cpu_data.fdiv_bug is only char (s8)
field so the detection overwrites already set fields for
other bugs, e.g. the f00f bug field.

Use local s32 variable to receive result.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---

This is a partial fix to Bugzilla #9928  - fixes wrong 
information about the f00f bug (tested) and probably
for coma bug (I have no cpu to test this).

diff -urp linux-alsa/arch/x86/kernel/cpu/bugs.c linux-new/arch/x86/kernel/cpu/bugs.c
--- linux-alsa/arch/x86/kernel/cpu/bugs.c	2008-07-31 12:40:31.000000000 +0200
+++ linux-new/arch/x86/kernel/cpu/bugs.c	2008-07-31 23:24:07.722657435 +0200
@@ -50,6 +50,8 @@ static double __initdata y = 3145727.0;
  */
 static void __init check_fpu(void)
 {
+	s32 fdiv_bug;
+
 	if (!boot_cpu_data.hard_math) {
 #ifndef CONFIG_MATH_EMULATION
 		printk(KERN_EMERG "No coprocessor found and no math emulation present.\n");
@@ -74,8 +76,10 @@ static void __init check_fpu(void)
 		"fistpl %0\n\t"
 		"fwait\n\t"
 		"fninit"
-		: "=m" (*&boot_cpu_data.fdiv_bug)
+		: "=m" (*&fdiv_bug)
 		: "m" (*&x), "m" (*&y));
+
+	boot_cpu_data.fdiv_bug = fdiv_bug;
 	if (boot_cpu_data.fdiv_bug)
 		printk("Hmm, FPU with FDIV bug.\n");
 }

Re: [PATCH] x86: fdiv bug detection fix

[Andrew Morton]

On Thu, 31 Jul 2008 23:43:44 +0200
Krzysztof Helt <krzysztof.h1@wp.pl> wrote:

> From: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> The fdiv detection code writes s32 integer into
> the boot_cpu_data.fdiv_bug. 
> However, the boot_cpu_data.fdiv_bug is only char (s8)
> field so the detection overwrites already set fields for
> other bugs, e.g. the f00f bug field.
> 
> Use local s32 variable to receive result.
> 
> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> ---
> 
> This is a partial fix to Bugzilla #9928  - fixes wrong 
> information about the f00f bug (tested) and probably
> for coma bug (I have no cpu to test this).
> 
> diff -urp linux-alsa/arch/x86/kernel/cpu/bugs.c linux-new/arch/x86/kernel/cpu/bugs.c
> --- linux-alsa/arch/x86/kernel/cpu/bugs.c	2008-07-31 12:40:31.000000000 +0200
> +++ linux-new/arch/x86/kernel/cpu/bugs.c	2008-07-31 23:24:07.722657435 +0200
> @@ -50,6 +50,8 @@ static double __initdata y = 3145727.0;
>   */
>  static void __init check_fpu(void)
>  {
> +	s32 fdiv_bug;
> +
>  	if (!boot_cpu_data.hard_math) {
>  #ifndef CONFIG_MATH_EMULATION
>  		printk(KERN_EMERG "No coprocessor found and no math emulation present.\n");
> @@ -74,8 +76,10 @@ static void __init check_fpu(void)
>  		"fistpl %0\n\t"
>  		"fwait\n\t"
>  		"fninit"
> -		: "=m" (*&boot_cpu_data.fdiv_bug)
> +		: "=m" (*&fdiv_bug)
>  		: "m" (*&x), "m" (*&y));
> +
> +	boot_cpu_data.fdiv_bug = fdiv_bug;
>  	if (boot_cpu_data.fdiv_bug)
>  		printk("Hmm, FPU with FDIV bug.\n");
>  }

hm, the code seems to have been that way for quite some time.  I wonder
why nobody noticed this before.  

Re: [PATCH] x86: fdiv bug detection fix

[Jeremy Fitzhardinge]

Andrew Morton wrote:
>>  		"fwait\n\t"
>>  		"fninit"
>> -		: "=m" (*&boot_cpu_data.fdiv_bug)
>> +		: "=m" (*&fdiv_bug)
>>  		: "m" (*&x), "m" (*&y));
>>     

This (*&foo) construct is strange.  Just "m" (foo) should be enough.

>> +
>> +	boot_cpu_data.fdiv_bug = fdiv_bug;
>>  	if (boot_cpu_data.fdiv_bug)
>>  		printk("Hmm, FPU with FDIV bug.\n");
>>  }
>>     
>
> hm, the code seems to have been that way for quite some time.  I wonder
> why nobody noticed this before.  
>   

It would trash f00f_bug, coma_bug and some padding.  You'd have to be 
running a Cyrix or Intel chip dating from somewhere around 1997-1998 to 
even be subject to those bugs, and even if you were, they wouldn't hurt 
day to day.  And if it snoops for those bugs after the fdiv bug, then 
they'd get updated properly anyway.

Change looks correct.

    J

Re: [PATCH] x86: fdiv bug detection fix

[Willy Tarreau]

On Thu, Jul 31, 2008 at 02:49:20PM -0700, Andrew Morton wrote:
> On Thu, 31 Jul 2008 23:43:44 +0200
> Krzysztof Helt <krzysztof.h1@wp.pl> wrote:
> 
> > From: Krzysztof Helt <krzysztof.h1@wp.pl>
> > 
> > The fdiv detection code writes s32 integer into
> > the boot_cpu_data.fdiv_bug. 
> > However, the boot_cpu_data.fdiv_bug is only char (s8)
> > field so the detection overwrites already set fields for
> > other bugs, e.g. the f00f bug field.
> > 
> > Use local s32 variable to receive result.
> > 
> > Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> > ---
> > 
> > This is a partial fix to Bugzilla #9928  - fixes wrong 
> > information about the f00f bug (tested) and probably
> > for coma bug (I have no cpu to test this).
> > 
> > diff -urp linux-alsa/arch/x86/kernel/cpu/bugs.c linux-new/arch/x86/kernel/cpu/bugs.c
> > --- linux-alsa/arch/x86/kernel/cpu/bugs.c	2008-07-31 12:40:31.000000000 +0200
> > +++ linux-new/arch/x86/kernel/cpu/bugs.c	2008-07-31 23:24:07.722657435 +0200
> > @@ -50,6 +50,8 @@ static double __initdata y = 3145727.0;
> >   */
> >  static void __init check_fpu(void)
> >  {
> > +	s32 fdiv_bug;
> > +
> >  	if (!boot_cpu_data.hard_math) {
> >  #ifndef CONFIG_MATH_EMULATION
> >  		printk(KERN_EMERG "No coprocessor found and no math emulation present.\n");
> > @@ -74,8 +76,10 @@ static void __init check_fpu(void)
> >  		"fistpl %0\n\t"
> >  		"fwait\n\t"
> >  		"fninit"
> > -		: "=m" (*&boot_cpu_data.fdiv_bug)
> > +		: "=m" (*&fdiv_bug)
> >  		: "m" (*&x), "m" (*&y));
> > +
> > +	boot_cpu_data.fdiv_bug = fdiv_bug;
> >  	if (boot_cpu_data.fdiv_bug)
> >  		printk("Hmm, FPU with FDIV bug.\n");
> >  }
> 
> hm, the code seems to have been that way for quite some time.

Indeed! since boot_cpu_data.fdiv_bug was declared as an int in 2.4, I
think it might have changed when a lot of code was moved from asm/*.h
to arch/*.c.

> I wonder why nobody noticed this before.

For the same reason those bugs were noticed very late in the products'
lifetime I guess : because unless someone actively plays with them on
your machine, you hardly trigger them by accident.

Willy

Re: [PATCH] x86: fdiv bug detection fix

[Ingo Molnar]

* Krzysztof Helt <krzysztof.h1@wp.pl> wrote:

> From: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> The fdiv detection code writes s32 integer into
> the boot_cpu_data.fdiv_bug. 
> However, the boot_cpu_data.fdiv_bug is only char (s8)
> field so the detection overwrites already set fields for
> other bugs, e.g. the f00f bug field.
> 
> Use local s32 variable to receive result.

applied to tip/x86/urgent, thanks Krzysztof!

this bug seems to be rather old - i guess new kernels rarely get tested 
with those CPUs that are affected by those bugs?

	Ingo