[coreboot] [PATCH] Seabios on Virtutech Simics x86-440bx model
Magnus Christensson
mch at virtutech.com
Thu Nov 5 15:00:45 CET 2009
On 11/05/2009 02:04 PM, Gleb Natapov wrote:
> On Thu, Nov 05, 2009 at 02:02:09PM +0100, Magnus Christensson wrote:
>
>> On 11/05/2009 11:49 AM, Gleb Natapov wrote:
>>
>>> On Thu, Nov 05, 2009 at 11:24:17AM +0100, Magnus Christensson wrote:
>>>
>>>> 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).
>>>>
>>>>
>>> OK. I don't think it is very important for modern guests anyway.
>>>
>> Right. Anything modern should look at the ACPI tables.
>>
>>
>>>>>>> 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.
>>>>
>>>>
>>> How about this:
>>>
>>> static inline unsigned long fls(unsigned long word)
>>> {
>>> asm("bsr %1,%0"
>>> : "=r" (word)
>>> : "rm" (word));
>>> return word + 1;
>>> }
>>>
>>> log_cpus = 1UL<< fls(atoi(log_cpus) - 1)
>>>
>>> (i& (log_cpus - 1) != 0)
>>>
>> Why atoi? Other than that it looks correct (assuming that the inline
>> asm syntax is correct which I'm not fluent in).
>>
>>
> atoi because of wrong cut&paste :) Drop it. Asm is from Linux so syntax is correct.
>
Ok. Changed patches attached.
M.
More information about the coreboot
mailing list