[PATCH] x86: better CPU identification without the CPUID

[PATCH] x86: better CPU identification without the CPUID

[Krzysztof Helt]

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

cpus without the CPUID instruction are identified
as general 386 or 486 while some cpus (mostly made
by Cyrix) provide c_identify function which identify
correctly older cpus using cpu specific registers).

Cyrix cpus are even worse as 5x86 and 6x68 have
the CPUID instruction disabled. The CPUID is
enabled by the c_identify() but the c_identify
is only called when the CPUID is available.

Fix this by calling the c_identify() for all known
cpu families if there is no the CPUID instruction

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

I have tested it on Cyrix Cx486DX2 cpu.

I suspect that coma_bug field and mtrr support is not
set correctly for the Cyrix 6x86 family. I have not a cpu from
the Cyrix 6x68 family to test. 

The code path is (+ where the patch code is added)

if (have_cpuid_p()) {

}
+ } else {
...
+ }

so it does not affect cpus with the CPUID instruction.

Regards,
Krzysztof

diff -urp linux-mm/arch/x86/kernel/cpu/common.c linux-486/arch/x86/kernel/cpu/common.c
--- linux-mm/arch/x86/kernel/cpu/common.c	2008-09-05 23:38:20.295563396 +0200
+++ linux-486/arch/x86/kernel/cpu/common.c	2008-09-12 22:53:27.062265329 +0200
@@ -444,6 +444,17 @@ static void __cpuinit generic_identify(s
 
 		init_scattered_cpuid_features(c);
 		detect_nopl(c);
+	} else {
+		int i;
+		for (i = 0; i < X86_VENDOR_NUM; i++)
+			if (cpu_devs[i] && cpu_devs[i]->c_identify) {
+				c->x86_vendor_id[0] = 0;
+				cpu_devs[i]->c_identify(c);
+				if (c->x86_vendor_id[0]) {
+					get_cpu_vendor(c, 0);
+					break;
+				}
+			}
 	}
 }
 

Re: [PATCH] x86: better CPU identification without the CPUID

[Yinghai Lu]

On Sat, Sep 13, 2008 at 3:56 AM, Krzysztof Helt <krzysztof.h1@wp.pl> wrote:
> From: Krzysztof Helt <krzysztof.h1@wp.pl>
>
> cpus without the CPUID instruction are identified
> as general 386 or 486 while some cpus (mostly made
> by Cyrix) provide c_identify function which identify
> correctly older cpus using cpu specific registers).
>
> Cyrix cpus are even worse as 5x86 and 6x68 have
> the CPUID instruction disabled. The CPUID is
> enabled by the c_identify() but the c_identify
> is only called when the CPUID is available.
>
> Fix this by calling the c_identify() for all known
> cpu families if there is no the CPUID instruction
>
> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> ---
>
> I have tested it on Cyrix Cx486DX2 cpu.
>
> I suspect that coma_bug field and mtrr support is not
> set correctly for the Cyrix 6x86 family. I have not a cpu from
> the Cyrix 6x68 family to test.
>
> The code path is (+ where the patch code is added)
>
> if (have_cpuid_p()) {
>
> }
> + } else {
> ...
> + }
>
> so it does not affect cpus with the CPUID instruction.
>
> Regards,
> Krzysztof
>
> diff -urp linux-mm/arch/x86/kernel/cpu/common.c linux-486/arch/x86/kernel/cpu/common.c
> --- linux-mm/arch/x86/kernel/cpu/common.c       2008-09-05 23:38:20.295563396 +0200
> +++ linux-486/arch/x86/kernel/cpu/common.c      2008-09-12 22:53:27.062265329 +0200
> @@ -444,6 +444,17 @@ static void __cpuinit generic_identify(s
>
>                init_scattered_cpuid_features(c);
>                detect_nopl(c);
> +       } else {
> +               int i;
> +               for (i = 0; i < X86_VENDOR_NUM; i++)
> +                       if (cpu_devs[i] && cpu_devs[i]->c_identify) {
> +                               c->x86_vendor_id[0] = 0;
> +                               cpu_devs[i]->c_identify(c);
> +                               if (c->x86_vendor_id[0]) {
> +                                       get_cpu_vendor(c, 0);
> +                                       break;
> +                               }
> +                       }
>        }
>  }
>
> --

1. can you check tip/master?
2. please check if you can add code in early_identify_cpu
3. need to move c_identify_cpu calling to identify_cpu

YH

Re: [PATCH] x86: better CPU identification without the CPUID

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

On Sat, Sep 13, 2008 at 9:43 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> On Sat, Sep 13, 2008 at 3:56 AM, Krzysztof Helt <krzysztof.h1@wp.pl> wrote:
>> From: Krzysztof Helt <krzysztof.h1@wp.pl>
>>
>> cpus without the CPUID instruction are identified
>> as general 386 or 486 while some cpus (mostly made
>> by Cyrix) provide c_identify function which identify
>> correctly older cpus using cpu specific registers).
>>
>> Cyrix cpus are even worse as 5x86 and 6x68 have
>> the CPUID instruction disabled. The CPUID is
>> enabled by the c_identify() but the c_identify
>> is only called when the CPUID is available.
>>
>> Fix this by calling the c_identify() for all known
>> cpu families if there is no the CPUID instruction

I updated it to tip/master and call that in early_identify_cpu. it
should solve mtrr detection for Cyrix ...

Please check attached patch.

Thanks

YH

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: identify_cpu_without_cpuid.patch --]
[-- Type: text/x-patch; name=identify_cpu_without_cpuid.patch, Size: 2538 bytes --]

[PATCH] x86: identify_cpu_without_cpuid

need to call c_identify() for cpus without cpuid earlier...

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

---
 arch/x86/kernel/cpu/common.c |   63 +++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 23 deletions(-)

Index: linux-2.6/arch/x86/kernel/cpu/common.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/common.c
+++ linux-2.6/arch/x86/kernel/cpu/common.c
@@ -485,6 +485,33 @@ static void __cpuinit get_cpu_cap(struct
 		c->x86_power = cpuid_edx(0x80000007);
 
 }
+
+static void __cpuinit identify_cpu_without_cpuid(struct cpuinfo_x86 *c)
+{
+#ifdef CONFIG_X86_32
+	int i;
+
+	/*
+	 * First of all, decide if this is a 486 or higher
+	 * It's a 486 if we can modify the AC flag
+	 */
+	if (flag_is_changeable_p(X86_EFLAGS_AC))
+		c->x86 = 4;
+	else
+		c->x86 = 3;
+
+	for (i = 0; i < X86_VENDOR_NUM; i++)
+		if (cpu_devs[i] && cpu_devs[i]->c_identify) {
+			c->x86_vendor_id[0] = 0;
+			cpu_devs[i]->c_identify(c);
+			if (c->x86_vendor_id[0]) {
+				get_cpu_vendor(c);
+				break;
+			}
+		}
+#endif
+}
+
 /*
  * Do minimum CPU detection early.
  * Fields really needed: vendor, cpuid_level, family, model, mask,
@@ -503,18 +530,17 @@ static void __init early_identify_cpu(st
 #endif
 	c->x86_cache_alignment = c->x86_clflush_size;
 
-	if (!have_cpuid_p())
-		return;
-
 	memset(&c->x86_capability, 0, sizeof c->x86_capability);
-
 	c->extended_cpuid_level = 0;
 
-	cpu_detect(c);
-
-	get_cpu_vendor(c);
-
-	get_cpu_cap(c);
+	if (!have_cpuid_p()) {
+		identify_cpu_without_cpuid(c);
+		return;
+	} else {
+		cpu_detect(c);
+		get_cpu_vendor(c);
+		get_cpu_cap(c);
+	}
 
 	if (this_cpu->c_early_init)
 		this_cpu->c_early_init(c);
@@ -583,11 +609,13 @@ static void __cpuinit detect_nopl(struct
 
 static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
 {
-	if (!have_cpuid_p())
-		return;
-
 	c->extended_cpuid_level = 0;
 
+	if (!have_cpuid_p()) {
+		identify_cpu_without_cpuid(c);
+		return;
+	}
+
 	cpu_detect(c);
 
 	get_cpu_vendor(c);
@@ -639,17 +667,6 @@ static void __cpuinit identify_cpu(struc
 	c->x86_cache_alignment = c->x86_clflush_size;
 	memset(&c->x86_capability, 0, sizeof c->x86_capability);
 
-	if (!have_cpuid_p()) {
-		/*
-		 * First of all, decide if this is a 486 or higher
-		 * It's a 486 if we can modify the AC flag
-		 */
-		if (flag_is_changeable_p(X86_EFLAGS_AC))
-			c->x86 = 4;
-		else
-			c->x86 = 3;
-	}
-
 	generic_identify(c);
 
 	if (this_cpu->c_identify)

Re: [PATCH] x86: better CPU identification without the CPUID

[Krzysztof Helt]

On Sat, 13 Sep 2008 22:55:10 -0700
"Yinghai Lu" <yhlu.kernel@gmail.com> wrote:

> On Sat, Sep 13, 2008 at 9:43 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> > On Sat, Sep 13, 2008 at 3:56 AM, Krzysztof Helt <krzysztof.h1@wp.pl> wrote:
> >> From: Krzysztof Helt <krzysztof.h1@wp.pl>
> >>
> >> cpus without the CPUID instruction are identified
> >> as general 386 or 486 while some cpus (mostly made
> >> by Cyrix) provide c_identify function which identify
> >> correctly older cpus using cpu specific registers).
> >>
> >> Cyrix cpus are even worse as 5x86 and 6x68 have
> >> the CPUID instruction disabled. The CPUID is
> >> enabled by the c_identify() but the c_identify
> >> is only called when the CPUID is available.
> >>
> >> Fix this by calling the c_identify() for all known
> >> cpu families if there is no the CPUID instruction
> 
> I updated it to tip/master and call that in early_identify_cpu. it
> should solve mtrr detection for Cyrix ...
> 


As I wrote I have no CPU to test this (Cyrix with ARRs). Finally, I am going
 to buy one or two of them during next two weeks so I will test.

I would like to postpone your patch until I (or someone else) test it.

There is some misunderstanding about the patch. There are two issues here.
The early enabling of mtrr registers and nicer and more accurate /proc/cpuinfo
content (and the CPU's name during boot). My patch was only about the latter.
It was not about the early identification or solving the mtrr problem (as without 
testing I am not sure I can correctly do this). Your patch resets the /proc/cpuinfo
content to the old status (before my patch).

> From: "Yinghai Lu" <yhlu.kernel@gmail.com>
>
> [PATCH] x86: identify_cpu_without_cpuid
> 
> need to call c_identify() for cpus without cpuid earlier...
> 
> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>


>  /*
>   * Do minimum CPU detection early.
>   * Fields really needed: vendor, cpuid_level, family, model, mask,
> @@ -503,18 +530,17 @@ static void __init early_identify_cpu(st
>  #endif
>  	c->x86_cache_alignment = c->x86_clflush_size;
>  
> -	if (!have_cpuid_p())
> -		return;
> -
>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
> -
>  	c->extended_cpuid_level = 0;
>  
> -	cpu_detect(c);
> -
> -	get_cpu_vendor(c);
> -
> -	get_cpu_cap(c);
> +	if (!have_cpuid_p()) {
> +		identify_cpu_without_cpuid(c);
> +		return;
> +	} else {
> +		cpu_detect(c);
> +		get_cpu_vendor(c);
> +		get_cpu_cap(c);
> +	}
>  
>  	if (this_cpu->c_early_init)
>  		this_cpu->c_early_init(c);

One wants to call c_early_init() here for Cyrix cpus- otherwise 
the mtrr caps are not set. The correct patch here is only:

 -
 -	if (!have_cpuid_p())
 -		return;

- 	    cpu_detect(c);

+	if (!have_cpuid_p())
+		enable_cpuid_on_some_cpus(); /* pseudo code for now */

+	if (have_cpuid_p())
+ 	    cpu_detect(c);


The enabling of the cpuid instruction should be added there but only the enabling.

> @@ -583,11 +609,13 @@ static void __cpuinit detect_nopl(struct
>  
>  static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
>  {
> -	if (!have_cpuid_p())
> -		return;
> -

I have just checked:
http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git

and it differs here. It is the same as in my patch (who is out of sync?).

> @@ -639,17 +667,6 @@ static void __cpuinit identify_cpu(struc
>  	c->x86_cache_alignment = c->x86_clflush_size;
>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>  
> -	if (!have_cpuid_p()) {
> -		/*
> -		 * First of all, decide if this is a 486 or higher
> -		 * It's a 486 if we can modify the AC flag
> -		 */
> -		if (flag_is_changeable_p(X86_EFLAGS_AC))
> -			c->x86 = 4;
> -		else
> -			c->x86 = 3;
> -	}
> -
>  	generic_identify(c);
>  
>  	if (this_cpu->c_identify)

The identify_cpu can stay the way it was. No changes here needed.
My patch improves the /proc/cpuinfo content. The next patch should
only enable the cpuid instruction on cyrix.

Regards,
Krzysztof

Re: [PATCH] x86: better CPU identification without the CPUID

[Yinghai Lu]

On Sun, Sep 14, 2008 at 1:30 AM, Krzysztof Helt <krzysztof.h1@wp.pl> wrote:
> On Sat, 13 Sep 2008 22:55:10 -0700
> "Yinghai Lu" <yhlu.kernel@gmail.com> wrote:
>
>> On Sat, Sep 13, 2008 at 9:43 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>> > On Sat, Sep 13, 2008 at 3:56 AM, Krzysztof Helt <krzysztof.h1@wp.pl> wrote:
>> >> From: Krzysztof Helt <krzysztof.h1@wp.pl>
>> >>
>> >> cpus without the CPUID instruction are identified
>> >> as general 386 or 486 while some cpus (mostly made
>> >> by Cyrix) provide c_identify function which identify
>> >> correctly older cpus using cpu specific registers).
>> >>
>> >> Cyrix cpus are even worse as 5x86 and 6x68 have
>> >> the CPUID instruction disabled. The CPUID is
>> >> enabled by the c_identify() but the c_identify
>> >> is only called when the CPUID is available.
>> >>
>> >> Fix this by calling the c_identify() for all known
>> >> cpu families if there is no the CPUID instruction
>>
>> I updated it to tip/master and call that in early_identify_cpu. it
>> should solve mtrr detection for Cyrix ...
>>
>
>
> As I wrote I have no CPU to test this (Cyrix with ARRs). Finally, I am going
>  to buy one or two of them during next two weeks so I will test.
>
> I would like to postpone your patch until I (or someone else) test it.
>
> There is some misunderstanding about the patch. There are two issues here.
> The early enabling of mtrr registers and nicer and more accurate /proc/cpuinfo
> content (and the CPU's name during boot). My patch was only about the latter.
> It was not about the early identification or solving the mtrr problem (as without
> testing I am not sure I can correctly do this). Your patch resets the /proc/cpuinfo
> content to the old status (before my patch).
>
>> From: "Yinghai Lu" <yhlu.kernel@gmail.com>
>>
>> [PATCH] x86: identify_cpu_without_cpuid
>>
>> need to call c_identify() for cpus without cpuid earlier...
>>
>> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>
>
>>  /*
>>   * Do minimum CPU detection early.
>>   * Fields really needed: vendor, cpuid_level, family, model, mask,
>> @@ -503,18 +530,17 @@ static void __init early_identify_cpu(st
>>  #endif
>>       c->x86_cache_alignment = c->x86_clflush_size;
>>
>> -     if (!have_cpuid_p())
>> -             return;
>> -
>>       memset(&c->x86_capability, 0, sizeof c->x86_capability);
>> -
>>       c->extended_cpuid_level = 0;
>>
>> -     cpu_detect(c);
>> -
>> -     get_cpu_vendor(c);
>> -
>> -     get_cpu_cap(c);
>> +     if (!have_cpuid_p()) {
>> +             identify_cpu_without_cpuid(c);
>> +             return;
>> +     } else {
>> +             cpu_detect(c);
>> +             get_cpu_vendor(c);
>> +             get_cpu_cap(c);
>> +     }
>>
>>       if (this_cpu->c_early_init)
>>               this_cpu->c_early_init(c);
>
> One wants to call c_early_init() here for Cyrix cpus- otherwise
> the mtrr caps are not set. The correct patch here is only:
>
>  -
>  -      if (!have_cpuid_p())
>  -              return;
>
> -           cpu_detect(c);
>
> +       if (!have_cpuid_p())
> +               enable_cpuid_on_some_cpus(); /* pseudo code for now */
>
> +       if (have_cpuid_p())
> +           cpu_detect(c);
>

could change to

if (!have_cpuid_p()) {
         identify_cpu_without_cpuid(c);

if (!have_cpuid_p())
         return;

cpu_detect(c);
get_cpu_vendor(c);
get_cpu_cap(c);

if (this_cpu->c_early_init)
 this_cpu->c_early_init(c);

will update the patch if Ingo didn't pick the patch.

>
> The enabling of the cpuid instruction should be added there but only the enabling.
>
>> @@ -583,11 +609,13 @@ static void __cpuinit detect_nopl(struct
>>
>>  static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
>>  {
>> -     if (!have_cpuid_p())
>> -             return;
>> -
>
> I have just checked:
> http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git
>
> and it differs here. It is the same as in my patch (who is out of sync?).

tip/master is changed a lot to mainline.

YH

Re: [PATCH] x86: better CPU identification without the CPUID

[Yinghai Lu]

On Sun, Sep 14, 2008 at 2:01 AM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> On Sun, Sep 14, 2008 at 1:30 AM, Krzysztof Helt <krzysztof.h1@wp.pl> wrote:
>> On Sat, 13 Sep 2008 22:55:10 -0700
>> "Yinghai Lu" <yhlu.kernel@gmail.com> wrote:
>>
>> One wants to call c_early_init() here for Cyrix cpus- otherwise
>> the mtrr caps are not set. The correct patch here is only:

please check v2 in another mail.

YH