On 11/10/2009 01:44 AM, Kevin O'Connor wrote:
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'm not sure how work is divided between Coreboot and Seabios. Does
Coreboot do all the machine specific initialization? Then the LINT LVTs
should already have been initialized.
I'll commit patch 5 once patch 4 is clear.
We'd probably like to make that conditional if we skip the
initialization. Something like:
if (!CONFIG_COREBOOT || (readl(APIC_LINT0) == <expected LINT0 value>) &&
readl(APIC_LINT1) == <expected LINT1 value>)) {
add LINT entries
}
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.
I'll rework that.
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?
Is KVM also using the QEMU port interface from paravirt.c? If so, we
could do the following:
if (qemu_cfg_use_cmos_smp_count()) {
u8 cmos_smp_count = inb_cmos(CMOS_BIOS_SMP_COUNT);
while (cmos_smp_count + 1 != readl(&CountCPUs))
;
} else {
msleep(10);
}
What does the QEMU paravirt device return for an unknown request?
Assuming 0, we need to invert the query to stay compatible (the
Virtutech version of the paravirt device would return 1 to use msleep
instead of the count in CMOS).
int qemu_cfg_use_cmos_smp_count(void)
{
u16 v;
if (!qemu_cfg_present)
return 0;
qemu_cfg_read_entry(&v, QEMU_CFG_SKIP_CMOS_SMP_COUNT, sizeof(v));
return !v;
}
M.
--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
