diff mbox

[v5,00/23] qemu: generate acpi tables for the guest

Message ID 20130925144850.GA29330@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Sept. 25, 2013, 2:48 p.m. UTC 
On Wed, Sep 25, 2013 at 04:15:03PM +0200, Gerd Hoffmann wrote:
> On Mi, 2013-09-25 at 15:12 +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > This code can also be found here:
> > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi
> > 
> > Crashes on coreboot with -M q35:
> > 
> > qemu-system-x86_64: /home/kraxel/projects/qemu/hw/i386/acpi-build.c:965:
> > acpi_build_update: Assertion `mcfg_size' failed.
> > 
> > I'll go send a coreboot rom with separate mail to not spam the list with
> > the blob.
> 
> Ahem, no need to dig out coreboot, with seabios it happens too if you
> pick the correct version (i.e. one with the table loader patches
> applied).
> 
> cheers,
>   Gerd
> 
> 

Looks like q35 scripts forgot to specify bios binary  :(
I fixed that, pushed.
It seems to work but I will have to redo all q35 testing it seems.

Thanks for the test!

Comments

Gerd Hoffmann Sept. 26, 2013, 6:26 a.m. UTC | #1 
Hi,

> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1ba86d0..d1ccdf7 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -961,8 +961,8 @@ static void acpi_build_update(void *build_opaque, uint32_t offset)
>      if (build_state->mcfg_base) {
>          AcpiMcfgAllocation *a;
>          mcfg_base = qint_get_int(build_state->mcfg_base);
> +        assert(build_state->mcfg_size);
>          mcfg_size = qint_get_int(build_state->mcfg_size);
> -        assert(mcfg_size);
>  
>          a = ACPI_BUILD_STATE_PTR(build_state, off_mcfg_allocation,
>                                   AcpiMcfgAllocation);

Well, that fixes the assert, but it still isn't working correctly.  No
mcfg table in acpi, even though the mcfg bar is programmed correctly.

Seeing this with both seabios+coreboot.

cheers,
  Gerd
Michael S. Tsirkin Sept. 26, 2013, 9:33 p.m. UTC | #2 
On Thu, Sep 26, 2013 at 08:26:51AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 1ba86d0..d1ccdf7 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -961,8 +961,8 @@ static void acpi_build_update(void *build_opaque, uint32_t offset)
> >      if (build_state->mcfg_base) {
> >          AcpiMcfgAllocation *a;
> >          mcfg_base = qint_get_int(build_state->mcfg_base);
> > +        assert(build_state->mcfg_size);
> >          mcfg_size = qint_get_int(build_state->mcfg_size);
> > -        assert(mcfg_size);
> >  
> >          a = ACPI_BUILD_STATE_PTR(build_state, off_mcfg_allocation,
> >                                   AcpiMcfgAllocation);
> 
> Well, that fixes the assert, but it still isn't working correctly.  No
> mcfg table in acpi, even though the mcfg bar is programmed correctly.
> 
> Seeing this with both seabios+coreboot.
> 
> cheers,
>   Gerd
> 

I see the bug now: my tests didn't check the MCFG table.

I'll fix this up, thanks!
diff mbox

Patch

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1ba86d0..d1ccdf7 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -961,8 +961,8 @@  static void acpi_build_update(void *build_opaque, uint32_t offset)
     if (build_state->mcfg_base) {
         AcpiMcfgAllocation *a;
         mcfg_base = qint_get_int(build_state->mcfg_base);
+        assert(build_state->mcfg_size);
         mcfg_size = qint_get_int(build_state->mcfg_size);
-        assert(mcfg_size);
 
         a = ACPI_BUILD_STATE_PTR(build_state, off_mcfg_allocation,
                                  AcpiMcfgAllocation);