diff mbox

hw/arm/virt-acpi-build: Fix table revision and some comments

Message ID 1433820378-8336-1-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao June 9, 2015, 3:26 a.m. UTC 
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(-)

Comments

Peter Maydell June 19, 2015, 12:39 p.m. UTC | #1 
On 9 June 2015 at 04:26, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 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
>       */
>
> @@ -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 */
>      acpi_add_table(table_offsets, tables_blob);
>      build_fadt(tables_blob, tables->linker, dsdt);
>
> --
> 2.0.4

I can put this in target-arm.next if somebody will review/ack it.

thanks
-- PMM
Andrew Jones June 19, 2015, 6:59 p.m. UTC | #2 
----- 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
> 
> 
> 
> 
>
Andrew Jones June 19, 2015, 7:06 p.m. UTC | #3 
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
Michael S. Tsirkin June 21, 2015, 10:02 a.m. UTC | #4 
On Tue, Jun 09, 2015 at 11:26:18AM +0800, Shannon Zhao wrote:
> 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>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  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
>       */
>  
> @@ -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 */
>      acpi_add_table(table_offsets, tables_blob);
>      build_fadt(tables_blob, tables->linker, dsdt);
>  
> -- 
> 2.0.4
>
Peter Maydell June 23, 2015, 4 p.m. UTC | #5 
On 9 June 2015 at 04:26, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 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(-)



Applied to target-arm.next (with the merge conflict noted by Andrew
fixed up), thanks.

-- PMM
diff mbox

Patch

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
      */
 
@@ -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 */
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, dsdt);