[Qemu-devel] E5-2620v2 - emulation stop error

Commit Message

Kevin O'Connor March 11, 2015, 6:40 p.m. UTC

On Wed, Mar 11, 2015 at 05:59:04PM +0000, Dr. David Alan Gilbert wrote:
> * Kevin O'Connor ([e-mail]) wrote:
> > On Wed, Mar 11, 2015 at 04:52:03PM +0000, Dr. David Alan Gilbert wrote:
> > > * Kevin O'Connor ([e-mail]) wrote:
> > > > So, I couldn't get this to fail on my older AMD machine at all with
> > > > the default SeaBIOS code.  But, when I change the code with the patch
> > > > below, it failed right away.
> > [...]
> > > > And the failed debug output looks like:
> > > > 
> > > > SeaBIOS (version rel-1.8.0-7-gd23eba6-dirty-20150311_121819-morn.localdomain)
> > > > [...]
> > > > cmos_smp_count0=20
> > > > [...]
> > > > cmos_smp_count=1
> > > > cmos_smp_count2=1/20
> > > > Found 1 cpu(s) max supported 20 cpu(s)
> > > > 
> > > > I'm going to check the assembly for a compiler error, but is it
> > > > possible QEMU is returning incorrect data in cmos index 0x5f?
> > 
> > I checked the SeaBIOS assembler and it looks sane.  So, I think the
> > question is, why is QEMU sometimes returning a 0 instead of 127 from
> > cmos 0x5f.
> 
> My reading of the logs I've just created is that qemu doesn't think
> it's ever being asked to read 5f in the failed case:
> 
> good:
> 
> pc_cmos_init 5f setting smp_cpus=20
> cmos: read index=0x0f val=0x00
> cmos: read index=0x34 val=0x00
> cmos: read index=0x35 val=0x3f
> cmos: read index=0x38 val=0x30
> cmos: read index=0x3d val=0x12
> cmos: read index=0x38 val=0x30
> cmos: read index=0x0b val=0x02
> cmos: read index=0x0d val=0x80
> cmos: read index=0x5f val=0x13  Yeh!
> cmos: read index=0x0f val=0x00
> cmos: read index=0x0f val=0x00
> cmos: read index=0x0f val=0x00
> 
> bad:
> pc_cmos_init 5f setting smp_cpus=20
> cmos: read index=0x0f val=0x00
> cmos: read index=0x34 val=0x00
> cmos: read index=0x35 val=0x3f
> cmos: read index=0x38 val=0x30
> cmos: read index=0x3d val=0x12
> cmos: read index=0x38 val=0x30
> cmos: read index=0x0b val=0x02
> cmos: read index=0x0d val=0x80  Oh!
> cmos: read index=0x0f val=0x00
> cmos: read index=0x0f val=0x00
> cmos: read index=0x0f val=0x00

For what it's worth, I can't seem to trigger the problem if I move the
cmos read above the SIPI/LAPIC code (see patch below).

I used this command line:

while true; do (sleep 5; echo -e '\001cq\n')| ../qemu/qemu-git/x86_64-softmmu/qemu-system-x86_64 -chardev file,path=foo.`date +%s`,id=seabios -device isa-debugcon,iobase=0x402,chardev=seabios -machine pc-i440fx-2.0,accel=kvm -m 1024 -smp 128 -nographic -device sga -L test 2>&1 | tee /tmp/qemu.op; grep "internal error" /tmp/qemu.op -q && break; done

This is on an "AMD Phenom(tm) II X6 1090T Processor" machine.

-Kevin


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Kevin O'Connor March 11, 2015, 6:45 p.m. UTC | #1

On Wed, Mar 11, 2015 at 02:40:39PM -0400, Kevin O'Connor wrote:
> For what it's worth, I can't seem to trigger the problem if I move the
> cmos read above the SIPI/LAPIC code (see patch below).

Ugh!

That's a seabios bug.  Main processor modifies the rtc index
(rtc_read()) while APs try to clear the NMI bit by modifying the rtc
index (romlayout.S:transition32).

I'll put together a fix.

-Kevin
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- a/src/fw/smp.c
+++ b/src/fw/smp.c
@@ -107,6 +107,8 @@  smp_setup(void)
                | (((u32)entry_smp - BUILD_BIOS_ADDR) << 8));
     *(u64*)BUILD_AP_BOOT_ADDR = new;
 
+    u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
+
     // enable local APIC
     u32 val = readl(APIC_SVR);
     writel(APIC_SVR, val | APIC_ENABLED);
@@ -127,7 +129,7 @@  smp_setup(void)
     writel(APIC_ICR_LOW, 0x000C4600 | sipi_vector);
 
     // Wait for other CPUs to process the SIPI.
-    u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
+    dprintf(1, "cmos_smp_count=%d\n", cmos_smp_count);
     while (cmos_smp_count != CountCPUs)
         asm volatile(
             // Release lock and allow other processors to use the stack.
@@ -140,6 +142,8 @@  smp_setup(void)
             : "+m" (SMPLock), "+m" (SMPStack)
             : : "cc", "memory");
     yield();
+    dprintf(1, "cmos_smp_count2=%d/%d\n", cmos_smp_count
+            , rtc_read(CMOS_BIOS_SMP_COUNT) + 1);
 
     // Restore memory.
     *(u64*)BUILD_AP_BOOT_ADDR = old;
diff --git a/src/post.c b/src/post.c
index 9ea5620..dc11c72 100644
--- a/src/post.c
+++ b/src/post.c
@@ -170,6 +170,7 @@  platform_hardware_setup(void)
     clock_setup();
 
     // Platform specific setup
+    dprintf(1, "cmos_smp_count0=%d\n", rtc_read(CMOS_BIOS_SMP_COUNT) + 1);
     qemu_platform_setup();
     coreboot_platform_setup();
 }