issues with seabios on the m2a-vm

[Ward Vandewege]

Hi all,

I'm seeing some weird behavior with seabios on the asus m2a-vm. The machine
boots fine with filo:

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-filo.cap

If I use seabios as a payload and leave CONFIG_BOOTMENU enabled, seabios
hangs on boot - but it's not entirely dead as pushing num lock, caps lock or
scroll lock results in this on the serial console:

i8042 ctr old=00000061 new=00000060
  i8042 ctr old=00000061 new=00000060

I tracked the hang down to the line

while (get_keystroke(0) >= 0)
        ;

in src/boot.c. Here's the boot log:

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-hang.cap

If I disable CONFIG_BOOTMENU, seabios does not hang but things go pretty
badly:

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-no-menu.cap

The IRQs seem to be messed up, and there seem to be some serious SATA issues
with the AHCI driver segfaulting and no sata drives being properly detected
by the kernel.

This is my config.h:

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-no-menu-config.h

Any suggestions for what might be going wrong? Ultimately I'd like to try
seabios with a sata controller whose option rom hangs the coreboot emulator.

Thanks,
Ward.

[Kevin O'Connor]

On Fri, Feb 20, 2009 at 05:18:00PM -0500, Ward Vandewege wrote:
> If I use seabios as a payload and leave CONFIG_BOOTMENU enabled, seabios
> hangs on boot - but it's not entirely dead as pushing num lock, caps lock or
> scroll lock results in this on the serial console:
>
> i8042 ctr old=00000061 new=00000060
>   i8042 ctr old=00000061 new=00000060
>
> I tracked the hang down to the line
>   while (get_keystroke(0) >= 0)
>     ;

Okay - that's a new one.  Something went wrong in the keyboard code.
Can you increase the keyboard debug level by setting the following in
src/config.h:

#define DEBUG_ISR_09 1
#define DEBUG_HDL_16 1

I wonder if something corrupted the BDA.  Have you tried with option
roms disabled?
> If I disable CONFIG_BOOTMENU, seabios does not hang but things go pretty
> badly:
>
> http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-no-menu.cap
>
> The IRQs seem to be messed up, and there seem to be some serious SATA issues
> with the AHCI driver segfaulting and no sata drives being properly detected
> by the kernel.

The IRQs are probably messed up because the bios tables aren't being
copied.  Please apply the patch below to coreboot (after adjusting the
memory size for your machine).

-Kevin

--- src/arch/i386/boot/tables.c (revision 3955)
+++ src/arch/i386/boot/tables.c (working copy)
@@ -43,8 +43,8 @@
        unsigned long low_table_start, low_table_end, new_low_table_end;
        unsigned long rom_table_start, rom_table_end;

-       rom_table_start = 0xf0000; 
-       rom_table_end =   0xf0000;
+       rom_table_start = ((1024-32)*1024*1024) - 64*1024;
+       rom_table_end = rom_table_start;
        /* Start low addr at 16 bytes instead of 0 because of a buglet
         * in the generic linux unzip code, as it tests for the a20 line.
         */
@@ -68,9 +68,10 @@
        post_code(0x96);

/* The smp table must be in 0-1K, 639K-640K, or 960K-1M */
-       new_low_table_end = write_smp_table(low_table_end); // low_table_end is 0x10 
at this point
+       rom_table_end = write_smp_table(rom_table_end);
+       rom_table_end = (rom_table_end+1023) & ~1023;

-#if HAVE_MP_TABLE==1
+#if 0 // HAVE_MP_TABLE==1
         /* Don't write anything in the traditional x86 BIOS data segment,
          * for example the linux kernel smp need to use 0x467 to pass reset vector
          * or use 0x40e/0x413 for EBDA finding...

[Peter Stuge]

Kevin O'Connor wrote:
> Please apply the patch below to coreboot (after adjusting the
> memory size for your machine).

Maybe this change should be encapsulated in a new option in v2.

And the memory size should be available at run time so we don't have
to hard code it.

//Peter

[Myles Watson]

On Sat, Feb 21, 2009 at 4:10 AM, Peter Stuge  wrote:
> Kevin O'Connor wrote:
> > Please apply the patch below to coreboot (after adjusting the
> > memory size for your machine).
>
> Maybe this change should be encapsulated in a new option in v2.
>
> And the memory size should be available at run time so we don't have
> to hard code it.

I think that's what Stefan's patch does.

Ward:

Have you tried using Stefan's patch?  I sent a patch later that
applies on top of Stefan's for Opterons to automatically find the
right spot for the tables.

I have a JMicron SATA card that breaks the emulator in Coreboot but
works fine with SeaBIOS.  Hopefully yours will too.

Thanks,
Myles

[Ward Vandewege]

On Fri, Feb 20, 2009 at 10:41:17PM -0500, Kevin O'Connor wrote:
> On Fri, Feb 20, 2009 at 05:18:00PM -0500, Ward Vandewege wrote:
> > If I use seabios as a payload and leave CONFIG_BOOTMENU enabled, seabios
> > hangs on boot - but it's not entirely dead as pushing num lock, caps lock or
> > scroll lock results in this on the serial console:
> >
> > i8042 ctr old=00000061 new=00000060
> >   i8042 ctr old=00000061 new=00000060
> >
> > I tracked the hang down to the line
> >   while (get_keystroke(0) >= 0)
> >     ;
>
> Okay - that's a new one.  Something went wrong in the keyboard code.
> Can you increase the keyboard debug level by setting the following in
> src/config.h:
>
> #define DEBUG_ISR_09 1
> #define DEBUG_HDL_16 1
>
> I wonder if something corrupted the BDA.  Have you tried with option
> roms disabled?

Yes. Log here:

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-menu-no-optionroms-...
> > If I disable CONFIG_BOOTMENU, seabios does not hang but things go pretty
> > badly:
> >
> > http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-no-menu.cap
> >
> > The IRQs seem to be messed up, and there seem to be some serious SATA issues
> > with the AHCI driver segfaulting and no sata drives being properly detected
> > by the kernel.
>
> The IRQs are probably messed up because the bios tables aren't being
> copied.  Please apply the patch below to coreboot (after adjusting the
> memory size for your machine).

The patch you sent assumed one GB of ram, right? I reduced the amount of ram
in the system to 1GB. The patch does not appear to help.

Logs here:

Boot menu enabled (keeps printing the debug output indefinitely):

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-menu-extra-kbd-logg...

Boot menu disabled (boots somewhat, like before):

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-no-menu-extra-kbd-d...

Any further thoughts?

Thanks,
Ward.

[Kevin O'Connor]

On Mon, Feb 23, 2009 at 05:13:12PM -0500, Ward Vandewege wrote:
> On Fri, Feb 20, 2009 at 10:41:17PM -0500, Kevin O'Connor wrote:
> > The IRQs are probably messed up because the bios tables aren't being
> > copied.  Please apply the patch below to coreboot (after adjusting the
> > memory size for your machine).
>
> The patch you sent assumed one GB of ram, right? I reduced the amount of ram
> in the system to 1GB.

Oh, you don't need to reduce memory.  Ideally, one would change the
value in tables.c to reflect the top of memory for the machine, but
even that isn't strictly necessary.
> The patch does not appear to help.

The patch didn't take:

Writing IRQ routing tables to 0xf0000...write_pirq_routing_table done.
ACPI: Writing ACPI tables at f0400...
[...]
ACPI:    * DSDT @ 000f0832 Length 267a
[...]
Payload is overwriting Coreboot tables.
[...]
Attempting to find coreboot table
init SMBIOS tables
Found 2 cpu(s)

Coreboot should have reported addresses in high-memory and SeaBIOS
should have reported something like:

Copying PIR from 3dff0000 to 000fad30
Copying ACPI RSDP from 3dff0400 to 000fade0

after the "Attempting to find coreboot table" report.

Did you rebuild from scratch after patching coreboot?  For example, I
use the following for epia-cn:

cd ~/src/coreboot/coreboot-v2/targets ; rm -r via/epia-cn/via_epia_cn ; ./buildtarget via/epia-cn/ ; cd via/epia-cn/via_epia_cn ; make ; cat ~/src/coreboot/tiny/tiny-cn/video.bios.bin coreboot.rom > coreboot.final.rom
> Logs here:
>
> Boot menu enabled (keeps printing the debug output indefinitely):
>
> http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-seabios-menu-extra-kbd-logg...

Okay - I'm at a loss to explain what's going on.  I wonder if there is
a compiler bug.  Can you place the out/rom16.o and out/rom32.o files
somewhere where I can grab them?  Also, the patch below will help with
debugging.

-Kevin

--- a/src/util.c
+++ b/src/util.c
@@ -193,10 +193,12 @@ usleep(u32 usec)
 static int
 check_for_keystroke()
 {
+    dprintf(1, "check_for_keystroke\n");
     struct bregs br;
     memset(&br, 0, sizeof(br));
     br.ah = 1;
     call16_int(0x16, &br);
+    dprintf(1, "check_for_keystroke fl=%x\n", br.flags);
     return !(br.flags & F_ZF);
 }

@@ -204,9 +206,11 @@ check_for_keystroke()
 static int
 get_raw_keystroke()
 {
+    dprintf(1, "get_raw_keystroke\n");
     struct bregs br;
     memset(&br, 0, sizeof(br));
     call16_int(0x16, &br);
+    dprintf(1, "get_raw_keystroke key=%x\n", br.ah);
     return br.ah;
 }

@@ -215,10 +219,13 @@ int
 get_keystroke(int msec)
 {
     for (;;) {
+        dprintf(1, "get_keystroke 1\n");
         if (check_for_keystroke())
             return get_raw_keystroke();
+        dprintf(1, "get_keystroke 2\n");
         if (msec <= 0)
             return -1;
+        dprintf(1, "get_keystroke 3 msec=%d\n", msec);
         usleep(50*1000);
         msec -= 50;
     }

[Ward Vandewege]

On Mon, Feb 23, 2009 at 09:06:01PM -0500, Kevin O'Connor wrote:
> On Mon, Feb 23, 2009 at 05:13:12PM -0500, Ward Vandewege wrote:
> > On Fri, Feb 20, 2009 at 10:41:17PM -0500, Kevin O'Connor wrote:
> > > The IRQs are probably messed up because the bios tables aren't being
> > > copied.  Please apply the patch below to coreboot (after adjusting the
> > > memory size for your machine).
> >
> > The patch you sent assumed one GB of ram, right? I reduced the amount of ram
> > in the system to 1GB.
>
> Oh, you don't need to reduce memory.  Ideally, one would change the
> value in tables.c to reflect the top of memory for the machine, but
> even that isn't strictly necessary.

Right, it was just easier this way.
> > The patch does not appear to help.
>
> The patch didn't take:

Yeah, I don't know what I did - but I sure didn't patch the right tree.

Here are logs where the patch *did* get applied.

With menu enabled:

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-menu-with-cb-and-seabios-pa...

With menu disabled:

http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-no-menu-with-cb-and-seabios...
> Okay - I'm at a loss to explain what's going on.  I wonder if there is
> a compiler bug.  Can you place the out/rom16.o and out/rom32.o files
> somewhere where I can grab them?  Also, the patch below will help with
> debugging.

Sure:

http://ward.vandewege.net/coreboot/m2a-vm/rom16.o
  http://ward.vandewege.net/coreboot/m2a-vm/rom32.o

These are from the 'with menus' build of seabios.

Thanks,
Ward.

[Myles Watson]

On Thu, Feb 26, 2009 at 11:53 AM, Ward Vandewege  wrote:
> On Mon, Feb 23, 2009 at 09:06:01PM -0500, Kevin O'Connor wrote:
> > On Mon, Feb 23, 2009 at 05:13:12PM -0500, Ward Vandewege wrote:
> > > On Fri, Feb 20, 2009 at 10:41:17PM -0500, Kevin O'Connor wrote:
> > > > The IRQs are probably messed up because the bios tables aren't being
> > > > copied.  Please apply the patch below to coreboot (after adjusting the
> > > > memory size for your machine).
> > >
> > > The patch you sent assumed one GB of ram, right? I reduced the amount of ram
> > > in the system to 1GB.
> >
> > Oh, you don't need to reduce memory.  Ideally, one would change the
> > value in tables.c to reflect the top of memory for the machine, but
> > even that isn't strictly necessary.
>
> Right, it was just easier this way.
> > > The patch does not appear to help.
> >
> > The patch didn't take:
>
> Yeah, I don't know what I did - but I sure didn't patch the right tree.
>
> Here are logs where the patch *did* get applied.
>
> With menu enabled:
>
> http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-menu-with-cb-and-seabios-pa...
>
> With menu disabled:
>
> http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-no-menu-with-cb-and-seabios...

In the logs it looks like you're having coreboot run the VGA ROM.  I
had to disable that to get mine to work.

option CONFIG_CONSOLE_VGA=1
option CONFIG_PCI_ROM_RUN=0
option CONFIG_VGA_ROM_RUN=0

If you set CONFIG_CONSOLE_VGA=0 the VGA bits in the bridges don't get
set, but then you have to specifically tell it not to run the ROM.

Hopefully that helps.

Thanks,
Myles

[Kevin O'Connor]

On Thu, Feb 26, 2009 at 01:53:04PM -0500, Ward Vandewege wrote:
> On Mon, Feb 23, 2009 at 09:06:01PM -0500, Kevin O'Connor wrote:
> > Okay - I'm at a loss to explain what's going on.  I wonder if there is
> > a compiler bug.  Can you place the out/rom16.o and out/rom32.o files
> > somewhere where I can grab them?  Also, the patch below will help with
> > debugging.
>
> Sure:
>
> http://ward.vandewege.net/coreboot/m2a-vm/rom16.o
>   http://ward.vandewege.net/coreboot/m2a-vm/rom32.o

Wow, this looks like a compiler bug.  When disassembling I see:

src/util.c:226
   f1597:       58                      popl   %eax
   f1598:       85 ed                   testl  %ebp,%ebp
   f159a:       7e 1c                   jle    f15b8 <get_keystroke+0xe4>

But, %ebp doesn't seem to be set before this point.  Further, the
'msec' parameter should be %eax at the start of the fuction, but %eax
is clobbered near the top of the function.

What version of gcc are you using?

I'm currently using FC10:

$ gcc --version
gcc (GCC) 4.3.2 20081105 (Red Hat 4.3.2-7)
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

-Kevin

[Ward Vandewege]

On Thu, Feb 26, 2009 at 07:38:56PM -0500, Kevin O'Connor wrote:
> On Thu, Feb 26, 2009 at 01:53:04PM -0500, Ward Vandewege wrote:
> > On Mon, Feb 23, 2009 at 09:06:01PM -0500, Kevin O'Connor wrote:
> > > Okay - I'm at a loss to explain what's going on.  I wonder if there is
> > > a compiler bug.  Can you place the out/rom16.o and out/rom32.o files
> > > somewhere where I can grab them?  Also, the patch below will help with
> > > debugging.
> >
> > Sure:
> >
> > http://ward.vandewege.net/coreboot/m2a-vm/rom16.o
> >   http://ward.vandewege.net/coreboot/m2a-vm/rom32.o
>
> Wow, this looks like a compiler bug.  When disassembling I see:
>
> src/util.c:226
>    f1597:       58                      popl   %eax
>    f1598:       85 ed                   testl  %ebp,%ebp
>    f159a:       7e 1c                   jle    f15b8 <get_keystroke+0xe4>
>
> But, %ebp doesn't seem to be set before this point.  Further, the
> 'msec' parameter should be %eax at the start of the fuction, but %eax
> is clobbered near the top of the function.
>
> What version of gcc are you using?

$ gcc --version
gcc (GCC) 4.2.4 (Ubuntu 4.2.4-1ubuntu3)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

This is Hardy, 32 bit.

Thanks,
Ward.

[Kevin O'Connor]

On Thu, Feb 26, 2009 at 08:10:33PM -0500, Ward Vandewege wrote:
> On Thu, Feb 26, 2009 at 07:38:56PM -0500, Kevin O'Connor wrote:
> > Wow, this looks like a compiler bug.  When disassembling I see:
>
> $ gcc --version
> gcc (GCC) 4.2.4 (Ubuntu 4.2.4-1ubuntu3)
> Copyright (C) 2007 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Yeah, I realized I could pull your compiler version out of the .o
files.  I can also confirm that my Ubuntu virtual machines are also
miscompiling seabios.  (What's Ubuntu doing to their compilers?)

Looks like it can't handle ebp being in a clobber list.  Can you see
if the patch below works for you?

-Kevin

--- a/src/util.c
+++ b/src/util.c
@@ -22,7 +22,7 @@ call16(struct bregs *callregs)
 #endif
         : "+a" (callregs), "+m" (*callregs)
         :
-        : "ebx", "ecx", "edx", "esi", "edi", "ebp", "cc", "memory");
+        : "ebx", "ecx", "edx", "esi", "edi", "cc", "memory");
 }

inline void
@@ -36,7 +36,7 @@ call16big(struct bregs *callregs)
         "calll __call16big_from32\n"
         : "+a" (callregs), "+m" (*callregs)
         :
-        : "ebx", "ecx", "edx", "esi", "edi", "ebp", "cc", "memory");
+        : "ebx", "ecx", "edx", "esi", "edi", "cc", "memory");
 }

inline void