[PATCH] moving F0 0F bug check to bugs.h

[PATCH] moving F0 0F bug check to bugs.h

[Zwane Mwaikambo]

Is there any reason why the F0 0F bug check isn't in bugs.h? We only
check for the bug once so we might as well move it to check the boot cpu
only in bugs.h. Diffed against 2.4.14-pre6, please consider for
applying.

Regards,
	Zwane Mwaikambo


diff -urbB linux-2.4.14-pre6-orig/arch/i386/kernel/setup.c linux-2.4.14-pre6-zm/arch/i386/kernel/setup.c
--- linux-2.4.14-pre6-orig/arch/i386/kernel/setup.c	Sun Nov 18 15:18:05 2001
+++ linux-2.4.14-pre6-zm/arch/i386/kernel/setup.c	Sun Nov 18 15:28:54 2001
@@ -2025,29 +2025,10 @@

 static void __init init_intel(struct cpuinfo_x86 *c)
 {
-#ifndef CONFIG_M686
-	static int f00f_workaround_enabled = 0;
-#endif
 	char *p = NULL;
 	unsigned int l1i = 0, l1d = 0, l2 = 0, l3 = 0; /* Cache sizes */

-#ifndef CONFIG_M686
-	/*
-	 * All current models of Pentium and Pentium with MMX technology CPUs
-	 * have the F0 0F bug, which lets nonpriviledged users lock up the system.
-	 * Note that the workaround only should be initialized once...
-	 */
-	c->f00f_bug = 0;
-	if ( c->x86 == 5 ) {
-		c->f00f_bug = 1;
-		if ( !f00f_workaround_enabled ) {
-			trap_init_f00f_bug();
-			printk(KERN_NOTICE "Intel Pentium with F0 0F bug - workaround enabled.\n");
-			f00f_workaround_enabled = 1;
-		}
-	}
-#endif
-
+	c->f00f_bug = boot_cpu_data.f00f_bug;	/* to avoid confusion */

 	if (c->cpuid_level > 1) {
 		/* supports eax=2  call */
diff -urbB linux-2.4.14-pre6-orig/include/asm-i386/bugs.h linux-2.4.14-pre6-zm/include/asm-i386/bugs.h
--- linux-2.4.14-pre6-orig/include/asm-i386/bugs.h	Mon Nov  5 22:42:12 2001
+++ linux-2.4.14-pre6-zm/include/asm-i386/bugs.h	Sun Nov 18 15:27:54 2001
@@ -144,6 +144,23 @@
 }

 /*
+ * All current models of Pentium and Pentium with MMX technology CPUs
+ * have the F0 0F bug, which lets nonpriviledged users lock up the system.
+ * Note that the workaround only should be initialized once...
+ */
+static void __init check_f00f(void)
+{
+#ifndef CONFIG_M686
+	boot_cpu_data.f00f_bug = 0;
+	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) && (boot_cpu_data.x86 == 5)) {
+		boot_cpu_data.f00f_bug = 1;
+		trap_init_f00f_bug();
+		printk(KERN_NOTICE "Intel Pentium with F0 0F bug - workaround enabled.\n");
+	}
+#endif
+}
+
+/*
  * Check whether we are able to run this kernel safely on SMP.
  *
  * - In order to run on a i386, we need to be compiled for i386
@@ -213,5 +230,6 @@
 	check_fpu();
 	check_hlt();
 	check_popad();
+	check_f00f();
 	system_utsname.machine[1] = '0' + (boot_cpu_data.x86 > 6 ? 6 : boot_cpu_data.x86);
 }

Re: [PATCH] moving F0 0F bug check to bugs.h

[Dave Jones]

On Sun, 18 Nov 2001, Zwane Mwaikambo wrote:

> Is there any reason why the F0 0F bug check isn't in bugs.h?

hpa moved it (And some others) during his 2.3.99 big cleanup of setup.c
and friends.

> We only check for the bug once so we might as well move it to check
> the boot cpu only in bugs.h.

Whilst not an ideal solution, some people do silly things like
putting a P150 and a P166 clocked to 150 into SMP boxes.
It could be possible for 1 CPU to have the bug whilst another doesn't.

It's questionable we should support such nightmare scenarios, but
as this is __initcode anyway, it's not that big a deal.

regards,

Dave.

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [PATCH] moving F0 0F bug check to bugs.h

[Zwane Mwaikambo]

On Sun, 18 Nov 2001, Dave Jones wrote:

> hpa moved it (And some others) during his 2.3.99 big cleanup of setup.c
> and friends.

Hmm most of them were gone, but f00f is indeed still there in 2.4.15-pre6

> Whilst not an ideal solution, some people do silly things like
> putting a P150 and a P166 clocked to 150 into SMP boxes.
> It could be possible for 1 CPU to have the bug whilst another doesn't.

This particular bug hits all 586s so we're safe in this regard.

> It's questionable we should support such nightmare scenarios, but
> as this is __initcode anyway, it's not that big a deal.

ahh but isn't that the beauty of Linux ;)

Regards,
	Zwane Mwaikambo

diff against 2.4.15-pre6 again since i missed the f00f_trap. Also because
i kept referring it to 2.4.14-pre6 i really mean 2.4.15-pre6

diff -urbB linux-2.4.14-pre6-orig/arch/i386/kernel/setup.c linux-2.4.14-pre6-zm/arch/i386/kernel/setup.c
--- linux-2.4.14-pre6-orig/arch/i386/kernel/setup.c	Sun Nov 18 15:18:05 2001
+++ linux-2.4.14-pre6-zm/arch/i386/kernel/setup.c	Sun Nov 18 16:22:58 2001
@@ -2020,34 +2020,12 @@
 	set_bit(X86_FEATURE_CX8, &c->x86_capability);
 }

-
-extern void trap_init_f00f_bug(void);
-
 static void __init init_intel(struct cpuinfo_x86 *c)
 {
-#ifndef CONFIG_M686
-	static int f00f_workaround_enabled = 0;
-#endif
 	char *p = NULL;
 	unsigned int l1i = 0, l1d = 0, l2 = 0, l3 = 0; /* Cache sizes */

-#ifndef CONFIG_M686
-	/*
-	 * All current models of Pentium and Pentium with MMX technology CPUs
-	 * have the F0 0F bug, which lets nonpriviledged users lock up the system.
-	 * Note that the workaround only should be initialized once...
-	 */
-	c->f00f_bug = 0;
-	if ( c->x86 == 5 ) {
-		c->f00f_bug = 1;
-		if ( !f00f_workaround_enabled ) {
-			trap_init_f00f_bug();
-			printk(KERN_NOTICE "Intel Pentium with F0 0F bug - workaround enabled.\n");
-			f00f_workaround_enabled = 1;
-		}
-	}
-#endif
-
+	c->f00f_bug = boot_cpu_data.f00f_bug;	/* to avoid confusion */

 	if (c->cpuid_level > 1) {
 		/* supports eax=2  call */
Only in linux-2.4.14-pre6-zm/arch/i386/kernel: setup.c~
diff -urbB linux-2.4.14-pre6-orig/include/asm-i386/bugs.h linux-2.4.14-pre6-zm/include/asm-i386/bugs.h
--- linux-2.4.14-pre6-orig/include/asm-i386/bugs.h	Mon Nov  5 22:42:12 2001
+++ linux-2.4.14-pre6-zm/include/asm-i386/bugs.h	Sun Nov 18 16:23:30 2001
@@ -144,6 +144,26 @@
 }

 /*
+ * All current models of Pentium and Pentium with MMX technology CPUs
+ * have the F0 0F bug, which lets nonpriviledged users lock up the system.
+ * Note that the workaround only should be initialized once...
+ */
+
+extern void trap_init_f00f_bug(void);
+
+static void __init check_f00f(void)
+{
+#ifndef CONFIG_M686
+	boot_cpu_data.f00f_bug = 0;
+	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) && (boot_cpu_data.x86 == 5)) {
+		boot_cpu_data.f00f_bug = 1;
+		trap_init_f00f_bug();
+		printk(KERN_NOTICE "Intel Pentium with F0 0F bug - workaround enabled.\n");
+	}
+#endif
+}
+
+/*
  * Check whether we are able to run this kernel safely on SMP.
  *
  * - In order to run on a i386, we need to be compiled for i386
@@ -213,5 +233,6 @@
 	check_fpu();
 	check_hlt();
 	check_popad();
+	check_f00f();
 	system_utsname.machine[1] = '0' + (boot_cpu_data.x86 > 6 ? 6 : boot_cpu_data.x86);
 }
Only in linux-2.4.14-pre6-zm/include/asm-i386: bugs.h~

Re: [PATCH] moving F0 0F bug check to bugs.h

[Dave Jones]

On Sun, 18 Nov 2001, Zwane Mwaikambo wrote:

> > hpa moved it (And some others) during his 2.3.99 big cleanup of setup.c
> > and friends.
> Hmm most of them were gone, but f00f is indeed still there in 2.4.15-pre6

The move went the other way. bugs.h -> setup.c
As well as f00f, things like the K6 bug check got moved there too.

tbh, I don't agree with hiding such code in a .h file. A better approach
may have been to split off a bugs.c file in arch/i386/kernel that gets
called after we've done CPU identification.

I'd like to shrink setup.c by moving a lot of things out of there.
If you look at what we currently have in that file..

- (unmaintained?) SGI visws support.
- bootmem allocator.
- CPU identification.
- CPU feature enabling/disabling.
- CPU errata workarounds.

Whilst you get to know your way around the 3000 or so lines after hacking
there a few times, it's constantly growing with each new CPU we support.
I think it's going to come to a point where this has to be split up to
some extent to keep it maintainable.

Something on my list for 2.5. I've already got patches doing some of this,
but theres still work to do.

> This particular bug hits all 586s so we're safe in this regard.

Good point. I had forgotten that. (And likewise, SMP K6's are unlikely)
In which case, I guess the reason was to get code 'hidden' in header
files in a common place.

> > It's questionable we should support such nightmare scenarios, but
> > as this is __initcode anyway, it's not that big a deal.
> ahh but isn't that the beauty of Linux ;)

Of course 8)

regards,
Dave.

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [PATCH] moving F0 0F bug check to bugs.h

[Zwane Mwaikambo]

On Sun, 18 Nov 2001, Dave Jones wrote:

> - (unmaintained?) SGI visws support.
> - bootmem allocator.
> - CPU identification.
> - CPU feature enabling/disabling.
> - CPU errata workarounds.
>
> Whilst you get to know your way around the 3000 or so lines after hacking
> there a few times, it's constantly growing with each new CPU we support.
> I think it's going to come to a point where this has to be split up to
> some extent to keep it maintainable.

Indeed the whole setup.c is a bit confusing, a cpusetup.c file or somesuch
would make things a bit simpler with maybe an exported function to setup.c
in the nature of cpu_detect(...). In your 2.5 todo, are you going
to move the errata/bugs to the same cpusetup file or a seperate one? Cause
now that i look at it, we got errata (e.g. AMD T13) in setup.c f00f etc,
then popad and friends in bugs.h.

Thanks,
	Zwane Mwaikambo

Re: [PATCH] moving F0 0F bug check to bugs.h

[Dave Jones]

On Sun, 18 Nov 2001, Zwane Mwaikambo wrote:

> Indeed the whole setup.c is a bit confusing, a cpusetup.c file or somesuch
> would make things a bit simpler with maybe an exported function to setup.c
> in the nature of cpu_detect(...). In your 2.5 todo, are you going
> to move the errata/bugs to the same cpusetup file or a seperate one?

The thing is, some identification code needs errata workarounds to happen
(such as the cache sizing code for eg). There comes a point where
splitting stuff up into lots of little files goes too far and becomes
a hindrance rather than a benefit.

When the slicing & dicing happens, I'll take a look at the sizes of
the bits, and see if it's worthwhile.

Dave.

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [PATCH] moving F0 0F bug check to bugs.h

[Alan Cox]

> > We only check for the bug once so we might as well move it to check
> > the boot cpu only in bugs.h.
> 
> Whilst not an ideal solution, some people do silly things like
> putting a P150 and a P166 clocked to 150 into SMP boxes.
> It could be possible for 1 CPU to have the bug whilst another doesn't.

Perhaps we should add the bugs as bit flags to the capability masks - but
inverted (ie "no foof bug" etc) that way they'll come out with the rest of
the SMP bug handling logic

Re: [PATCH] moving F0 0F bug check to bugs.h

[George Greer]

On Sun, 18 Nov 2001, Dave Jones wrote:

>> We only check for the bug once so we might as well move it to check
>> the boot cpu only in bugs.h.
>
>Whilst not an ideal solution, some people do silly things like
>putting a P150 and a P166 clocked to 150 into SMP boxes.
>It could be possible for 1 CPU to have the bug whilst another doesn't.

All Pentium and Pentium/MMX have it, even the 233/MMX.  So unless you stuff
a Pentium 1 and a Pentium Pro in the same machine somehow, all will be
affected.

processor       : 1
vendor_id       : GenuineIntel
cpu family      : 5
model           : 4
model name      : Pentium MMX
stepping        : 3
cpu MHz         : 232.109
fdiv_bug        : no
hlt_bug         : no
sep_bug         : no
f00f_bug        : yes
coma_bug        : no
fpu             : yes
fpu_exception   : yes
cpuid level     : 1
wp              : yes
flags           : fpu vme de pse tsc msr mce cx8 apic mmx
bogomips        : 463.66

-George Greer

Re: [PATCH] moving F0 0F bug check to bugs.h

[H. Peter Anvin]

Followup to:  <Pine.LNX.4.30.0111181512230.29315-100000@Appserv.suse.de>
By author:    Dave Jones <davej@suse.de>
In newsgroup: linux.dev.kernel
>
> On Sun, 18 Nov 2001, Zwane Mwaikambo wrote:
> 
> > Is there any reason why the F0 0F bug check isn't in bugs.h?
> 
> hpa moved it (And some others) during his 2.3.99 big cleanup of setup.c
> and friends.
> 

bugs.h should die, ideally.  There is no good reason the checking for
features and the checking for bugs is done using different mechanisms,
and the bugs.h mechanism is *MUCH* uglier.  However, I was planning to
wait for 2.5 with that one.

     -hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt	<amsp@zytor.com>