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