Skip to contentSkip to conversation

PentiumPro/II split in x86 config

RFC PATCH In review J.A. Magallon 12 participants 20 messages May 27, 2002 – May 29, 2002

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 PentiumPro/II split in x86 config

Hello.

Patch below splits the config build option for PentiumPro and PentiumII to make them different, so gcc (see next patch) can generate pII specific code. It also kills the lock errata protection in PII code, so it only applies to PPro, not PII.

Due to Alan's advice, it also adds a check that will panic if a PII or higher kernel is run on a PPro or lesser (plz, I put that code in the place I thought it was the best, but probably I'm wrong...).

It still keeps -march=i686. I will change it on a next patch.

Comments ?

Patch follows (91-split-ppro-config):

arch/i386/Makefile renamed from arch/i386/Makefile.orig +4 -0
@@ -50,6 +50,10 @@
 CFLAGS += -march=i686
 endif
 
+ifdef CONFIG_MPENTIUMII
+CFLAGS += -march=i686
+endif
+
 ifdef CONFIG_MPENTIUMIII
 CFLAGS += -march=i686
 endif
arch/i386/config.in renamed from arch/i386/config.in.orig +9 -1
@@ -32,7 +32,8 @@
 	 586/K5/5x86/6x86/6x86MX		CONFIG_M586 \
 	 Pentium-Classic			CONFIG_M586TSC \
 	 Pentium-MMX				CONFIG_M586MMX \
-	 Pentium-Pro/Celeron/Pentium-II		CONFIG_M686 \
+	 Pentium-Pro			CONFIG_M686 \
+	 Pentium-II/Celeron		CONFIG_MPENTIUMII \
 	 Pentium-III/Celeron(Coppermine)	CONFIG_MPENTIUMIII \
 	 Pentium-4				CONFIG_MPENTIUM4 \
 	 K6/K6-II/K6-III			CONFIG_MK6 \
@@ -99,6 +99,13 @@
    define_bool CONFIG_X86_USE_PPRO_CHECKSUM y
    define_bool CONFIG_X86_PPRO_FENCE y
 fi
+if [ "$CONFIG_MPENTIUMII" = "y" ]; then
+   define_int  CONFIG_X86_L1_CACHE_SHIFT 5
+   define_bool CONFIG_X86_TSC y
+   define_bool CONFIG_X86_GOOD_APIC y
+   define_bool CONFIG_X86_PGE y
+   define_bool CONFIG_X86_USE_PPRO_CHECKSUM y
+fi
 if [ "$CONFIG_MPENTIUMIII" = "y" ]; then
    define_int  CONFIG_X86_L1_CACHE_SHIFT 5
    define_bool CONFIG_X86_TSC y
arch/i386/defconfig renamed from arch/i386/defconfig.orig +1 -0
@@ -27,6 +27,7 @@
 # CONFIG_M586TSC is not set
 # CONFIG_M586MMX is not set
 # CONFIG_M686 is not set
+# CONFIG_MPENTIUMII is not set
 CONFIG_MPENTIUMIII=y
 # CONFIG_MPENTIUM4 is not set
 # CONFIG_MK6 is not set
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c.orig +10 -0
@@ -2092,6 +2092,14 @@
 
 extern void trap_init_f00f_bug(void);
 
+static void __init check_intel_compat(struct cpuinfo_x86 *c)
+{
+#if defined(CONFIG_MPENTIUMII) || defined(CONFIG_MPENTIUMIII) || defined(CONFIG_MPENTIUM4)
+	if (c->x86 <= 5)
+		panic("Kernel is unsafe/incompatible with this CPU model. Check your build settings !\n");
+#endif
+}
+
 static void __init init_intel(struct cpuinfo_x86 *c)
 {
 #ifndef CONFIG_M686
@@ -2100,6 +2108,8 @@
 	char *p = NULL;
 	unsigned int l1i = 0, l1d = 0, l2 = 0, l3 = 0; /* Cache sizes */
 
+	check_intel_compat(c);
+
 #ifndef CONFIG_M686
 	/*
 	 * All current models of Pentium and Pentium with MMX technology CPUs
Signature · 4 lines
J.A. Magallon                           #  Let the source be with you...        
mailto:[email protected]
Mandrake Linux release 8.3 (Cooker) for i586
Linux werewolf 2.4.19-pre8-jam4 #2 SMP dom may 26 11:20:42 CEST 2002 i686
#1 lore raw

arch/i386/kernel/setup.c line 2100

Quoted 6 lines · J.A. Magallon
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c.orig +6 -0
+static void __init check_intel_compat(struct cpuinfo_x86 *c)
+{
+#if defined(CONFIG_MPENTIUMII) || defined(CONFIG_MPENTIUMIII) || defined(CONFIG_MPENTIUM4)
+	if (c->x86 <= 5)
+		panic("Kernel is unsafe/incompatible with this CPU model. Check your build settings !\n");
+#endif

The PPro is also model 6 but a different family (1 if I remember rightly)

#2 lore raw
In reply to J.A. Magallon
Quoted 12 lines · On 2002.05.27 Alan Cox wrote:

arch/i386/kernel/setup.c line 2100

Quoted 6 lines · J.A. Magallon
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c.orig +6 -0
+static void __init check_intel_compat(struct cpuinfo_x86 *c)
+{
+#if defined(CONFIG_MPENTIUMII) || defined(CONFIG_MPENTIUMIII) || defined(CONFIG_MPENTIUM4)
+	if (c->x86 <= 5)
+		panic("Kernel is unsafe/incompatible with this CPU model. Check your build settings !\n");
+#endif

The PPro is also model 6 but a different family (1 if I remember rightly)

Opps... typo.
Yes:

{ X86_VENDOR_INTEL, 6,
{ "Pentium Pro A-step", "Pentium Pro", NULL, "Pentium II (Klamath)",
NULL, "Pentium II (Deschutes)", "Mobile Pentium II", "Pentium III (Katmai)", "Pentium III (Coppermine)", NULL, "Pentium III (Cascades)", NULL, NULL, NULL, NULL }},

Corrected patch on the way...

Signature · 4 lines
J.A. Magallon                           #  Let the source be with you...        
mailto:[email protected]
Mandrake Linux release 8.3 (Cooker) for i586
Linux werewolf 2.4.19-pre8-jam4 #2 SMP dom may 26 11:20:42 CEST 2002 i686
#3 lore raw
In reply to J.A. Magallon

arch/i386/kernel/setup.c line 2101

Quoted 10 lines · J.A. Magallon

Due to Alan's advice, it also adds a check that will panic if a PII or higher kernel is run on a PPro or lesser (plz, I put that code in the place I thought it was the best, but probably I'm wrong...).

arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c.orig +7 -0
+static void __init check_intel_compat(struct cpuinfo_x86 *c)
+{
+#if defined(CONFIG_MPENTIUMII) || defined(CONFIG_MPENTIUMIII) || defined(CONFIG_MPENTIUM4)
+	if (c->x86 <= 5)
+		panic("Kernel is unsafe/incompatible with this CPU model. Check your build settings !\n");
+#endif
+}

Don't rely on that working. When you compile with -march=i686, gcc emits cmov instructions all over the place, including in printk code. The first cmov takes a fault and tries to panic, the panic code uses printk which hits a second cmov and the machine is dead with no messages.

#4 lore raw
In reply to J.A. Magallon

Em Mon, May 27, 2002 at 11:59:11PM +0200, J.A. Magallon escreveu:

Quoted 10 lines

Opps... typo.
Yes:

{ X86_VENDOR_INTEL, 6,
{ "Pentium Pro A-step", "Pentium Pro", NULL, "Pentium II (Klamath)",
NULL, "Pentium II (Deschutes)", "Mobile Pentium II", "Pentium III (Katmai)", "Pentium III (Coppermine)", NULL, "Pentium III (Cascades)", NULL, NULL, NULL, NULL }},

Corrected patch on the way...

Hi,

Since you're working on this could I suggest that you use labeled
elements, this gccism make the initialization above way more cleaner, safer and easy to read :-) This is being used in the kernel in places like the FSes, the TCP/IP stack and lots of other places.

We don't need all the NULLs, as uninitialized entries will be zeroed
out by the compiler.

Here is how it would look like:

arch/i386/kernel/setup.c +28 -9
@@ -2245,15 +2245,34 @@
 /* This table only is used unless init_<vendor>() below doesn't set it; */
 /* in particular, if CPUID levels 0x80000002..4 are supported, this isn't used */
 static struct cpu_model_info cpu_models[] __initdata = {
-	{ X86_VENDOR_INTEL,	4,
-	  { "486 DX-25/33", "486 DX-50", "486 SX", "486 DX/2", "486 SL", 
-	    "486 SX/2", NULL, "486 DX/2-WB", "486 DX/4", "486 DX/4-WB", NULL, 
-	    NULL, NULL, NULL, NULL, NULL }},
-	{ X86_VENDOR_INTEL,	5,
-	  { "Pentium 60/66 A-step", "Pentium 60/66", "Pentium 75 - 200",
-	    "OverDrive PODP5V83", "Pentium MMX", NULL, NULL,
-	    "Mobile Pentium 75 - 200", "Mobile Pentium MMX", NULL, NULL, NULL,
-	    NULL, NULL, NULL, NULL }},
+	{
+		vendor:	     X86_VENDOR_INTEL,
+		family:	     4,
+		model_names: {
+				[0] = "486 DX-25/33",
+				[1] = "486 DX-50",
+				[2] = "486 SX",
+				[3] = "486 DX/2",
+				[4] = "486 SL", 
+				[5] = "486 SX/2",
+				[7] = "486 DX/2-WB",
+				[8] = "486 DX/4",
+				[9] = "486 DX/4-WB",
+		},
+	},
+	{
+		vendor:	     X86_VENDOR_INTEL,
+		family:	     5,
+		model_names: {
+				[0] = "Pentium 60/66 A-step",
+				[1] = "Pentium 60/66",
+				[2] = "Pentium 75 - 200",
+				[3] = "OverDrive PODP5V83",
+				[4] = "Pentium MMX",
+				[7] = "Mobile Pentium 75 - 200",
+				[8] = "Mobile Pentium MMX",
+		}
+	},
 	{ X86_VENDOR_INTEL,	6,
 	  { "Pentium Pro A-step", "Pentium Pro", NULL, "Pentium II (Klamath)", 
 	    NULL, "Pentium II (Deschutes)", "Mobile Pentium II",

Best Regards,

- Arnaldo, with the janitor hat on for the time being 8)

#5 lore raw

Review

Quoted 6 lines · Arnaldo Carvalho de Melo

Since you're working on this could I suggest that you use labeled
elements, this gccism make the initialization above way more cleaner, safer and easy to read :-) This is being used in the kernel in places like the FSes, the TCP/IP stack and lots of other places.

(quoted diff) +2 -0
+		vendor:	     X86_VENDOR_INTEL,
+		family:	     5,

Better still, use the C language standard:

		.vendor = X86_VENDOR_INTEL,
		.family = 5,
#6 lore raw
In reply to Arnaldo Carvalho de Melo

Em Tue, May 28, 2002 at 12:05:54PM +1000, Keith Owens escreveu:

Quoted 11 lines

Review

Quoted 6 lines · Arnaldo Carvalho de Melo

Since you're working on this could I suggest that you use labeled
elements, this gccism make the initialization above way more cleaner, safer and easy to read :-) This is being used in the kernel in places like the FSes, the TCP/IP stack and lots of other places.

(quoted diff) +2 -0
+		vendor:	     X86_VENDOR_INTEL,
+		family:	     5,

Better still, use the C language standard:

Quoted 2 lines
		.vendor = X86_VENDOR_INTEL,
		.family = 5,

oops, gcc accepts that, good to know that this is standard C, but for the kernel, I think that this doesn't matter as gcc is the only compiler that understands the GCC Language 8) Or am I wrong? I'd love to be... 8)

- Arnaldo

#7 lore raw
Quoted 8 lines · Arnaldo Carvalho de Melo

Em Tue, May 28, 2002 at 12:05:54PM +1000, Keith Owens escreveu:

Quoted 3 lines

Better still, use the C language standard:

		.vendor = X86_VENDOR_INTEL,

oops, gcc accepts that, good to know that this is standard C, but for the kernel, I think that this doesn't matter as gcc is the only compiler that understands the GCC Language 8) Or am I wrong? I'd love to be... 8)

The kernel can only be compiled with gcc, but some bits of the build are compiled and run on the host, using the host compiler. Avoid using gccisms where there is a standard way of doing it.

#8 lore raw
Quoted 3 lines · Keith Owens

The kernel can only be compiled with gcc, but some bits of the build are compiled and run on the host, using the host compiler. Avoid using gccisms where there is a standard way of doing it.

That particular gccism completely infests the kernel, so there seems little point in avoiding it in favor of the uglier standard syntax.

-Miles

Signature · 1 line
I have seen the enemy, and he is us.  -- Pogo
#9 lore raw
In reply to Keith Owens

Em Tue, May 28, 2002 at 11:55:47AM +0900, Miles Bader escreveu:

Quoted 7 lines
Quoted 3 lines · Keith Owens

The kernel can only be compiled with gcc, but some bits of the build are compiled and run on the host, using the host compiler. Avoid using gccisms where there is a standard way of doing it.

That particular gccism completely infests the kernel, so there seems little point in avoiding it in favor of the uglier standard syntax.

Agreed.

What I'll do: continue using the simpler way that only gcc understands but take care to not use gccisms when and if I patch build bits.

- Arnaldo

#10 lore raw
Quoted 6 lines · On Tue, 28 May 2002 00:02:00 -0300 Arnaldo Carvalho de Melo…

Em Tue, May 28, 2002 at 11:55:47AM +0900, Miles Bader escreveu:

Quoted 5 lines
Quoted 3 lines · Keith Owens

The kernel can only be compiled with gcc, but some bits of the build are compiled and run on the host, using the host compiler. Avoid using gccisms where there is a standard way of doing it.

Quoted 7 lines
Quoted 2 lines

That particular gccism completely infests the kernel, so there seems little point in avoiding it in favor of the uglier standard syntax.

Agreed.

What I'll do: continue using the simpler way that only gcc understands but take care to not use gccisms when and if I patch build bits.

WARNING: The older form is deprecated. From the info pages:

Another syntax which has the same meaning, obsolete since GCC 2.5, is
`FIELDNAME:', as shown here:

Janitors wanted,
Rusty.

Signature · 2 lines
   there are those who do and those who hang on and you don't see too
   many doers quoting their contemporaries.  -- Larry McVoy
#11 lore raw
In reply to Keith Owens
Quoted 6 lines · On 2002.05.28 Arnaldo Carvalho de Melo wrote:

Em Tue, May 28, 2002 at 11:55:47AM +0900, Miles Bader escreveu:

Quoted 5 lines
Quoted 3 lines · Keith Owens

The kernel can only be compiled with gcc, but some bits of the build are compiled and run on the host, using the host compiler. Avoid using gccisms where there is a standard way of doing it.

Quoted 8 lines
Quoted 2 lines

That particular gccism completely infests the kernel, so there seems little point in avoiding it in favor of the uglier standard syntax.

Agreed.

What I'll do: continue using the simpler way that only gcc understands but take care to not use gccisms when and if I patch build bits.

Problem is that named initializers '.xx =' are ISO C99, so problably they are not supported in gcc till 3.0...the old way is working with older compilers.

Signature · 4 lines
J.A. Magallon                           #  Let the source be with you...        
mailto:[email protected]
Mandrake Linux release 8.3 (Cooker) for i586
Linux werewolf 2.4.19-pre8-jam4 #2 SMP dom may 26 11:20:42 CEST 2002 i686
#12 lore raw
Quoted 3 lines · J.A. Magallon

Problem is that named initializers '.xx =' are ISO C99, so problably they are not supported in gcc till 3.0...the old way is working with older compilers.

"probably"? Why not check the facts. 2.95.3 implements ".name =" initialisers, and 2.95.3 is also listed in 2.4.18's Documentation/Changes as the oldest acceptable compiler.

I agree with Keith that we really should prefer standard C solutions over gcc-specific hacks _when_they_exist_.

Also note that gcc is no longer the only compiler able to compile the kernel. Intel claims that their icc6 compiler has correctly compiled 2.4.18 with only minor tweaks needed. They do implement inline asm() nowadays, but alas not &&label and computed gotos.

/Mikael

#13 lore raw
Quoted 2 lines · [email protected] said:

They do implement inline asm() nowadays, but alas not &&label and
computed gotos.

The only places I've seen &&label used are where it's been passed into an inline asm which has jumped to it... but gcc has optimised the label and the code following it away because it was never used (inline asm isn't allowed to do that).

--
dwmw2

#14 lore raw
Quoted 6 lines · David Woodhouse
Quoted 2 lines · [email protected] said:

They do implement inline asm() nowadays, but alas not &&label and computed gotos.

The only places I've seen &&label used

In the kernel or elsewhere? It's certainly useful. Here it's used to make up for 'case' not supporting strings, avoiding a cascade of 'else if'.

///////////////////////////////////////////////////////////////////////

static const char *set_personality(void){
char *s;
size_t sl;
char buf[16];
personality_table_struct findme = { buf, NULL}; personality_table_struct *found;
static const personality_table_struct personality_table[] = {

  {"390",      &&case_390},
  {"aix",      &&case_aix},
  {"gnu",      &&case_gnu},
  {"hp",       &&case_hp},
  {"hpux",     &&case_hpux},
  {"irix",     &&case_irix},
  {"os390",    &&case_os390},
  {"s390",     &&case_s390},
  {"sco",      &&case_sco},
  {"sgi",      &&case_sgi},
  {"unknown",  &&case_unknown}
  };
  const int personality_table_count
    = sizeof(personality_table)/sizeof(personality_table_struct);
  personality = 0;
  s = getenv("CMD_ENV");
  if(!s || !*s) s="unknown";   /* "Do The Right Thing[tm]" */
  sl = strlen(s);
  if(sl > 15) return "Environment specified an unknown personality.";
  strncpy(buf, s, sl);
  buf[sl] = '\0';
  saved_personality_text = strdup(buf);

found = bsearch(&findme, personality_table, personality_table_count,
sizeof(personality_table_struct), compare_personality_table_structs
);

if(!found) return "Environment specified an unknown personality.";

goto *(found->jump); /* See gcc extension info. :-) */

  case_gnu:
    personality = PER_GOOD_o | PER_CUMUL_MARKED | PER_OLD_m;
    // other stuff...
    return NULL;

  case_unknown: /* defaults, but also check inferior environment variables */
    if(
      getenv("UNIX95")     /* Irix */
      || getenv("POSIXLY_CORRECT")  /* most gnu stuff */
      || (getenv("POSIX2") && !strcmp(getenv("POSIX2"), "on")) /* Unixware 7 */
    ) personality = PER_BROKEN_o;
    return NULL;

case_aix:

     // blah, blah...
     return NULL;
  case_irix:
  case_sgi:
    s = getenv("_XPG");
    if(s && s[0]>'0' && s[0]<='9') personality = PER_BROKEN_o;
    else personality = PER_IRIX_l;
    return NULL;

  case_os390:  /* IBM's OS/390 OpenEdition on the S/390 mainframe */
  case_s390:
  case_390:
    personality = PER_390_j;
    return NULL;

case_hp:
case_hpux:
case_sco:

    personality = PER_BROKEN_o;
    return NULL;
}

//////////////////////////////////////////////////////////////////

#15 lore raw
In reply to Keith Owens
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"J" == J A Magallon <J.A.> writes:

J> Problem is that named initializers '.xx =' are ISO C99, so J> problably they are not supported in gcc till 3.0...the old way J> is working with older compilers.

I actually tried it :). gcc 2.95 supports named initializers as well:

$ cat a.c
struct foo {
  int x;
  int y;
};

struct foo bar = {
.y = 2,
.x = 1
};

int main() {
printf("%d\n", bar.x);
return 0;
}

$ gcc a.c
$ ./a.out
1
$ gcc --version
2.95.2

Best,
Roland

#16 lore raw
In reply to Keith Owens
Quoted 2 lines · Mikael Pettersson

I agree with Keith that we really should prefer standard C solutions over gcc-specific hacks _when_they_exist_.

Since portability to other compilers is really not an option with linux, it's a lot more important that the kernel code use a _consistent_ convention than it use a `standard' one.

So before recommending that people use a different syntax than the one historically used, are you going to run over the whole kernel and replace all the existing uses of field:' with .field = ' (and brave the flamewar that it would probably require)?

-Miles

Signature · 1 line
Suburbia: where they tear out the trees and then name streets after them.
#17 lore raw
In reply to Keith Owens

Em Wed, May 29, 2002 at 02:23:50PM +1000, Rusty Russell escreveu:

Quoted 17 lines
Quoted 6 lines · On Tue, 28 May 2002 00:02:00 -0300 Arnaldo Carvalho de Melo…

Em Tue, May 28, 2002 at 11:55:47AM +0900, Miles Bader escreveu:

Quoted 5 lines
Quoted 3 lines · Keith Owens

The kernel can only be compiled with gcc, but some bits of the build are compiled and run on the host, using the host compiler. Avoid using gccisms where there is a standard way of doing it.

Quoted 7 lines
Quoted 2 lines

That particular gccism completely infests the kernel, so there seems little point in avoiding it in favor of the uglier standard syntax.

Agreed.

What I'll do: continue using the simpler way that only gcc understands but take care to not use gccisms when and if I patch build bits.

Quoted 6 lines

WARNING: The older form is deprecated. From the info pages:

Another syntax which has the same meaning, obsolete since GCC 2.5, is
`FIELDNAME:', as shown here:

Janitors wanted,

Ouch, Rusty, I'll reread the info pages and go over all the tree and start submitting patches, starting tomorrow. Expect that sound, multiple times.

Powers that be: holler now or...

- Arnaldo

#18 lore raw
In reply to Arnaldo Carvalho de Melo

Em Tue, May 28, 2002 at 12:24:14PM +1000, Keith Owens escreveu:

Quoted 7 lines
Quoted 5 lines · Arnaldo Carvalho de Melo

Em Tue, May 28, 2002 at 12:05:54PM +1000, Keith Owens escreveu:

Quoted 3 lines

Better still, use the C language standard:

		.vendor = X86_VENDOR_INTEL,
Quoted 7 lines
Quoted 3 lines

oops, gcc accepts that, good to know that this is standard C, but for the kernel, I think that this doesn't matter as gcc is the only compiler that understands the GCC Language 8) Or am I wrong? I'd love to be... 8)

The kernel can only be compiled with gcc, but some bits of the build are compiled and run on the host, using the host compiler. Avoid using gccisms where there is a standard way of doing it.

I see, I was not thinking about those bits of the build, only kernel code, where there are plenty of other gccisms.

/me will take a look at the latest C standard... :-)

- Arnaldo

#19 lore raw
In reply to J.A. Magallon
Quoted 4 lines · Arnaldo Carvalho de Melo

Since you're working on this could I suggest that you use labeled
elements, this gccism make the initialization above way more cleaner, safer and easy to read :-) This is being used in the kernel in places like the FSes, the TCP/IP stack and lots of other places.

Patrick Mochel already did it in his reworking of the arch/i386/kernel cruft It's already merged in my tree, and will hopefully turn up in Linus' tree sometime soon. So far, it's been dropped (probably because it rejected when someone else touched something there)

I'll push the resynced version later today

Dave.

Signature · 2 lines
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs