Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi-build: Fix table revision and some comments

[Andrew Jones <drjones@redhat.com>]

On Fri, Jun 19, 2015 at 02:59:32PM -0400, Andrew Jones wrote:
> 
> 
> ----- Original Message -----
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > The table revision is not the ACPI spec version. Fix the wrong revision
> > and also some comments.
> > 
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> >  hw/arm/virt-acpi-build.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index a9373cc..a036a00 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -349,7 +349,7 @@ build_mcfg(GArray *table_data, GArray *linker,
> > VirtGuestInfo *guest_info)
> >      mcfg->allocation[0].end_bus_number = (memmap[VIRT_PCIE_ECAM].size
> >                                            / PCIE_MMCFG_SIZE_MIN) - 1;
> >  
> > -    build_header(linker, table_data, (void *)mcfg, "MCFG", len, 5);
> > +    build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1);
> >  }
> >  
> >  /* GTDT */
> > @@ -375,7 +375,7 @@ build_gtdt(GArray *table_data, GArray *linker)
> >  
> >      build_header(linker, table_data,
> >                   (void *)(table_data->data + gtdt_start), "GTDT",
> > -                 table_data->len - gtdt_start, 5);
> > +                 table_data->len - gtdt_start, 2);
> >  }
> >  
> >  /* MADT */
> > @@ -412,7 +412,7 @@ build_madt(GArray *table_data, GArray *linker,
> > VirtGuestInfo *guest_info,
> >  
> >      build_header(linker, table_data,
> >                   (void *)(table_data->data + madt_start), "APIC",
> > -                 table_data->len - madt_start, 5);
> > +                 table_data->len - madt_start, 3);
> >  }
> >  
> >  /* FADT */
> > @@ -469,7 +469,7 @@ build_dsdt(GArray *table_data, GArray *linker,
> > VirtGuestInfo *guest_info)
> >      g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
> >      build_header(linker, table_data,
> >          (void *)(table_data->data + table_data->len - dsdt->buf->len),
> > -        "DSDT", dsdt->buf->len, 5);
> > +        "DSDT", dsdt->buf->len, 2);
> >      free_aml_allocator();
> >  }
> >  
> > @@ -507,6 +507,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info,
> > AcpiBuildTables *tables)
> >       * FADT
> >       * GTDT
> >       * MADT
> > +     * MCFG
> >       * DSDT
> >       */
> 
> Do we need this comment with a list of tables? I forgot to add SPCR
> there.
> 
> >  
> > @@ -514,7 +515,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info,
> > AcpiBuildTables *tables)
> >      dsdt = tables_blob->len;
> >      build_dsdt(tables_blob, tables->linker, guest_info);
> >  
> > -    /* FADT MADT GTDT pointed to by RSDT */
> > +    /* FADT MADT GTDT MCFG pointed to by RSDT */
> 
> This patch isn't based on the latest version, and this comment will
> have a merge conflict due to SPCR being there now.
> 
> >      acpi_add_table(table_offsets, tables_blob);
> >      build_fadt(tables_blob, tables->linker, dsdt);
> >  
> > --
> > 2.0.4
> > 

Forgot to say that I reviewed the table version number changes, and
they look good.

drew