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

[Kevin O'Connor]

On Thu, Nov 05, 2009 at 03:00:45PM +0100, Magnus Christensson wrote:
> Ok. Changed patches attached.

Thanks Magnus.  I've committed patches 1-3 and 6.  I have a question
on patch 4:

> @@ -91,6 +93,12 @@ smp_probe(void)
>      u32 val = readl(APIC_SVR);
>      writel(APIC_SVR, val | APIC_ENABLED);
>  
> +    /* Set LINT0 as Ext_INT, level triggered */
> +    writel(APIC_LINT0, 0x8700);
> +
> +    /* Set LINT1 as NMI, level triggered */
> +    writel(APIC_LINT1, 0x8400);

This will get run on real hardware when used with coreboot.  Is this
safe on all machines?  If not, it should be wrapped in an if
(!CONFIG_COREBOOT) clause.

I'll commit patch 5 once patch 4 is clear.

On patch 6 - I've been trying to avoid using #if statements - can you
rework the patch with regular C if() clauses?  Also, can you rename
VIRTUTECH_PC_SHADOW to have a CONFIG_ prefix and place it with the
rest of the CONFIG_ items in the config.h file.  It would be nice if
simutech could be auto-detected, but that's not a requirement.

Finally, on patch 7:

> @@ -105,7 +105,7 @@ smp_probe(void)
>      writel(APIC_ICR_LOW, 0x000C4600 | sipi_vector);
>  
>      // Wait for other CPUs to process the SIPI.
> -    if (CONFIG_COREBOOT) {
> +    if (CONFIG_COREBOOT || !USE_CMOS_BIOS_SMP_COUNT) {
>          msleep(10);
>      } else {
>          u8 cmos_smp_count = inb_cmos(CMOS_BIOS_SMP_COUNT);

I'm wondering if this should just say something like:

if (!kvm_para_available() && !qemu_para_available())
   msleep(10);
...

That is, instead of defining a compile time parameter, I wonder if the
default should be to msleep and only use the cmos method when qemu is
detected - the cmos thing is really qemu specific anyway.  Gleb - do
you know a good way to determine if we're running on qemu?

-Kevin