2008-07-31 21:45:23

by Krzysztof Helt

[permalink] [raw]
Subject: [PATCH] x86: fdiv bug detection fix

From: Krzysztof Helt <[email protected]>

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 <[email protected]>
---

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");
}


2008-07-31 21:50:20

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] x86: fdiv bug detection fix

On Thu, 31 Jul 2008 23:43:44 +0200
Krzysztof Helt <[email protected]> wrote:

> From: Krzysztof Helt <[email protected]>
>
> 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 <[email protected]>
> ---
>
> 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.

2008-07-31 21:57:30

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86: fdiv bug detection fix


* Krzysztof Helt <[email protected]> wrote:

> From: Krzysztof Helt <[email protected]>
>
> 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

2008-07-31 22:04:36

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH] x86: fdiv bug detection fix

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

2008-07-31 22:13:58

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH] x86: fdiv bug detection fix

On Thu, Jul 31, 2008 at 02:49:20PM -0700, Andrew Morton wrote:
> On Thu, 31 Jul 2008 23:43:44 +0200
> Krzysztof Helt <[email protected]> wrote:
>
> > From: Krzysztof Helt <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> >
> > 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