diff mbox

Hang when using 9p mounts after last Seabios update

Message ID 20111001165043.GA16833@morn.localdomain
State New
Headers show

Commit Message

Kevin O'Connor Oct. 1, 2011, 4:50 p.m. UTC 
On Thu, Sep 22, 2011 at 12:45:11PM +0100, Daniel P. Berrange wrote:
> On 0.14, 0.15 releaes, this all works just fine. On current GIT master,
> the guest OS will hang during boot.
[...]
> To reproduce this you will need my custom initrd for mounting 9p filesystems
> as the root FS. You can get that here:
> 
>   http://berrange.com/~dan/qemu-serial-hang-demo.tar.gz

Thanks for the detailed report.

I've confirmed the issue and tracked it down.  The current SeaBIOS
code gets confused during alignment checking if there are no prefmem
regions found.

The patch below should fix the issue.

Richard, can you also check to see if this seabios patch fixes your
issue?

-Kevin


Author: Kevin O'Connor <kevin@koconnor.net>
Date:   Sat Oct 1 12:35:32 2011 -0400

    Fix alignment bug in pci_bios_init_root_regions().
    
    If there are no memory allocations for a given type then the "max" bar
    size is zero.  However, ALIGN_DOWN does not handle an alignment of
    zero properly.  Catch and handle the zero case.
    
    Signed-off-by: Kevin O'Connor <kevin@koconnor.net>

Comments

Richard W.M. Jones Oct. 1, 2011, 5:32 p.m. UTC | #1 
On Sat, Oct 01, 2011 at 12:50:43PM -0400, Kevin O'Connor wrote:
> On Thu, Sep 22, 2011 at 12:45:11PM +0100, Daniel P. Berrange wrote:
> > On 0.14, 0.15 releaes, this all works just fine. On current GIT master,
> > the guest OS will hang during boot.
> [...]
> > To reproduce this you will need my custom initrd for mounting 9p filesystems
> > as the root FS. You can get that here:
> > 
> >   http://berrange.com/~dan/qemu-serial-hang-demo.tar.gz
> 
> Thanks for the detailed report.
> 
> I've confirmed the issue and tracked it down.  The current SeaBIOS
> code gets confused during alignment checking if there are no prefmem
> regions found.
> 
> The patch below should fix the issue.
> 
> Richard, can you also check to see if this seabios patch fixes your
> issue?

Yes, I can confirm that this fixes the issue.

What I did to test this:

 - tried my boot test, and it failed as before

 - git clone git://git.linuxtogo.org/home/kevin/seabios.git

 - noticed that this patch is already applied to git

 - make clean; make

 - cp out/bios.bin ../qemu/pc-bios/bios.bin

 - repeated my boot test, and it was successful

Thanks!

Rich.
Daniel P. Berrangé Oct. 11, 2011, 8:30 a.m. UTC | #2 
On Sat, Oct 01, 2011 at 12:50:43PM -0400, Kevin O'Connor wrote:
> On Thu, Sep 22, 2011 at 12:45:11PM +0100, Daniel P. Berrange wrote:
> > On 0.14, 0.15 releaes, this all works just fine. On current GIT master,
> > the guest OS will hang during boot.
> [...]
> > To reproduce this you will need my custom initrd for mounting 9p filesystems
> > as the root FS. You can get that here:
> > 
> >   http://berrange.com/~dan/qemu-serial-hang-demo.tar.gz
> 
> Thanks for the detailed report.
> 
> I've confirmed the issue and tracked it down.  The current SeaBIOS
> code gets confused during alignment checking if there are no prefmem
> regions found.
> 
> The patch below should fix the issue.

Thanks, I have tested Seabios 1.6.3 which includes that patch, and
can confirm that it does fix the hang I saw.

Anthony/Gerd: we can get QEMU master updated to Seabios 1.6.3 before
the 1.0 release ?

> Author: Kevin O'Connor <kevin@koconnor.net>
> Date:   Sat Oct 1 12:35:32 2011 -0400
> 
>     Fix alignment bug in pci_bios_init_root_regions().
>     
>     If there are no memory allocations for a given type then the "max" bar
>     size is zero.  However, ALIGN_DOWN does not handle an alignment of
>     zero properly.  Catch and handle the zero case.
>     
>     Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> 
> diff --git a/src/pciinit.c b/src/pciinit.c
> index a857da0..0d8758e 100644
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -536,7 +536,7 @@ static void pci_bios_init_bus_bases(struct pci_bus *bus)
>      }
>  }
>  
> -#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align))
> +#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
>  
>  static int pci_bios_init_root_regions(u32 start, u32 end)
>  {

Regards,
Daniel
Krumme, Chris Oct. 11, 2011, 4:45 p.m. UTC | #3 
On 10/11/2011 03:30 AM, Daniel P. Berrange wrote:
> On Sat, Oct 01, 2011 at 12:50:43PM -0400, Kevin O'Connor wrote:
>> On Thu, Sep 22, 2011 at 12:45:11PM +0100, Daniel P. Berrange wrote:
>>> On 0.14, 0.15 releaes, this all works just fine. On current GIT master,
>>> the guest OS will hang during boot.
>> [...]
>>> To reproduce this you will need my custom initrd for mounting 9p filesystems
>>> as the root FS. You can get that here:
>>>
>>>    http://berrange.com/~dan/qemu-serial-hang-demo.tar.gz
>> Thanks for the detailed report.
>>
>> I've confirmed the issue and tracked it down.  The current SeaBIOS
>> code gets confused during alignment checking if there are no prefmem
>> regions found.
>>
>> The patch below should fix the issue.
> Thanks, I have tested Seabios 1.6.3 which includes that patch, and
> can confirm that it does fix the hang I saw.
>
> Anthony/Gerd: we can get QEMU master updated to Seabios 1.6.3 before
> the 1.0 release ?
>
>> Author: Kevin O'Connor<kevin@koconnor.net>
>> Date:   Sat Oct 1 12:35:32 2011 -0400
>>
>>      Fix alignment bug in pci_bios_init_root_regions().
>>
>>      If there are no memory allocations for a given type then the "max" bar
>>      size is zero.  However, ALIGN_DOWN does not handle an alignment of
>>      zero properly.  Catch and handle the zero case.
>>
>>      Signed-off-by: Kevin O'Connor<kevin@koconnor.net>
>>
>> diff --git a/src/pciinit.c b/src/pciinit.c
>> index a857da0..0d8758e 100644
>> --- a/src/pciinit.c
>> +++ b/src/pciinit.c
>> @@ -536,7 +536,7 @@ static void pci_bios_init_bus_bases(struct pci_bus *bus)
>>       }
>>   }
>>
>> -#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align))
>> +#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)

Hello,

This is adding a GNUism, can this be:  (max)?(max):1

Thanks

Chris

>>
>>   static int pci_bios_init_root_regions(u32 start, u32 end)
>>   {
> Regards,
> Daniel
diff mbox

Patch

diff --git a/src/pciinit.c b/src/pciinit.c
index a857da0..0d8758e 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -536,7 +536,7 @@  static void pci_bios_init_bus_bases(struct pci_bus *bus)
     }
 }
 
-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align))
+#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
 
 static int pci_bios_init_root_regions(u32 start, u32 end)
 {