[PATCH] x86: fdiv bug detection fix - Kernel

This is a discussion on [PATCH] x86: fdiv bug detection fix - Kernel ; From: Krzysztof Helt 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 ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH] x86: fdiv bug detection fix

  1. [PATCH] x86: fdiv bug detection fix

    From: Krzysztof Helt

    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
    ---

    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");
    }
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] x86: fdiv bug detection fix


    * Krzysztof Helt wrote:

    > From: Krzysztof Helt
    >
    > 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] x86: fdiv bug detection fix

    On Thu, 31 Jul 2008 23:43:44 +0200
    Krzysztof Helt wrote:

    > From: Krzysztof Helt
    >
    > 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
    > ---
    >
    > 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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. 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 wrote:
    >
    > > From: Krzysztof Helt
    > >
    > > 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
    > > ---
    > >
    > > 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

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread