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

[Gleb Natapov]

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
 
> >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)?

--
			Gleb.