lkml.org 
[lkml]   [2008]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] x86: better CPU identification without the CPUID
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


\
 
 \ /
  Last update: 2008-09-14 11:07    [W:0.064 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site