[coreboot] [PATCH] Seabios on Virtutech Simics x86-440bx model

Magnus Christensson mch at virtutech.com
Thu Nov 5 11:24:17 CET 2009


On 11/05/2009 10:54 AM, Gleb Natapov wrote:
> On Thu, Nov 05, 2009 at 10:30:49AM +0100, Magnus Christensson wrote:
>    
>> On 11/04/2009 03:40 PM, Gleb Natapov wrote:
>>      
>>> On Tue, Nov 03, 2009 at 10:30:12PM -0500, Kevin O'Connor wrote:
>>>        
>>>> On Tue, Nov 03, 2009 at 04:31:32PM +0100, Magnus Christensson wrote:
>>>>          
>>>>> I'm attaching patches for seabios to make it work on the Virtutech
>>>>> Simics x86-440bx model. Please let me know if there is some other list
>>>>> that is preferred for seabios patches.
>>>>>
>>>>> Patches 1-6 and 9 are not really related to the Virtutech model at all,
>>>>> so those would be prime candidates to be included in the mainline
>>>>> version.
>>>>>            
>>> l>   Thanks Magnus.
>>>        
>>>> Gleb I'm not sure if you're on the coreboot mailing list - can you
>>>> take a look at these patches as well?  A URL is at:
>>>>
>>>> http://permalink.gmane.org/gmane.linux.bios/55487
>>>>
>>>>          
>>> Can I get them in a mbox format somewhere? Want to tested them to be
>>> sure. From review:
>>> 1:
>>>   OK
>>>
>>> 2:
>>>   What is the reason for this? x86info --mptable on my 4 core AMD shows
>>> MP Table:
>>> #       APIC ID Version State           Family  Model   Step    Flags
>>> #        0       0x10    BSP, usable     16      4       1     0x178bfbff
>>> #        1       0x10    AP, usable      16      4       1     0x178bfbff
>>> #        2       0x10    AP, usable      16      4       1     0x178bfbff
>>> #        3       0x10    AP, usable      16      4       1     0x178bfbff
>>>        
>> I can think of two reasons for only listing one CPU per package. The
>> first would be performance, in that the OS needs to be aware of
>> multi-threading in order not to slow down high priority tasks with
>> stuff that is usually free when running on multiple processors (like
>> spinlocks). The second reason would be that logical CPUs in the same
>> package share some MSRs, and an OS that isn't aware of that may
>> cease to work in such an environment.
>>
>> Related link:
>>
>> http://software.intel.com/en-us/articles/hyper-threading-implications-and-setup-on-microsoft-operating-systems/
>>
>>      
> This list talks about HT. Why not add entry for each core though? I am
> not aware of any MSRs shared between cores
>    
Some MSRs are shared between cores, for example lots of the memory check 
MSRs in Nehalem, although an OS that knows about MCs would probably also 
know something about how they are shared. Adding multiple cores to the 
MPS tables would probably not break anything. One reason why they are 
still filtered out in at least some cases could be that multiple cores 
and threads are flagged with the same bit (the HTT bit).

>
>    
>> > From a41c206097e801de29668f99ae9b9342614c716d Mon Sep 17 00:00:00 2001
>> From: Magnus Christensson<mch at virtutech.com>
>> Date: Tue, 3 Nov 2009 12:50:09 +0100
>> Subject: [PATCH 2/8] Only add the first logical CPU in each physical CPU to the MPS tables.
>>
>> ---
>>   src/mptable.c |   26 ++++++++++++++++++++++----
>>   1 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mptable.c b/src/mptable.c
>> index 805fe1b..525188d 100644
>> --- a/src/mptable.c
>> +++ b/src/mptable.c
>> @@ -44,16 +44,32 @@ mptable_init(void)
>>       config->spec = 4;
>>       memcpy(config->oemid, CONFIG_CPUNAME8, sizeof(config->oemid));
>>       memcpy(config->productid, "0.1         ", sizeof(config->productid));
>> -    config->entrycount = MaxCountCPUs + 2 + 16;
>>       config->lapic = BUILD_APIC_ADDR;
>>
>>       // CPU definitions.
>>       u32 cpuid_signature, ebx, ecx, cpuid_features;
>>       cpuid(1,&cpuid_signature,&ebx,&ecx,&cpuid_features);
>>       struct mpt_cpu *cpus = (void*)&config[1];
>> -    int i;
>> -    for (i = 0; i<  MaxCountCPUs; i++) {
>> +    int i, actual_cpu_count;
>> +    for (i = 0, actual_cpu_count = 0; i<  MaxCountCPUs; i++) {
>>           struct mpt_cpu *cpu =&cpus[i];
>> +        int log_cpus = (ebx>>  16)&  0xff;
>> +
>> +        /* Only populate the MPS tables with the first logical CPU in each
>> +           package */
>> +        if ((cpuid_features&  (1<<  28))&&
>> +            log_cpus>  1&&
>> +            ((log_cpus<= 2&&  (i&  1) != 0) ||
>> +             (log_cpus<= 4&&  (i&  3) != 0) ||
>> +             (log_cpus<= 8&&  (i&  7) != 0) ||
>> +             (log_cpus<= 16&&  (i&  15) != 0) ||
>> +             (log_cpus<= 32&&  (i&  31) != 0) ||
>> +             (log_cpus<= 64&&  (i&  63) != 0) ||
>> +             (log_cpus<= 128&&  (i&  127) != 0)))
>> +            continue;
>> +
>>      
> Isn't this the same as (i&  (log_cpus - 1) != 0)?
>    
Yes, as long as log_cpus is a power of 2. But not if it's for example 3.

M.

> --
> 			Gleb.
>
>    





More information about the coreboot mailing list