Re: [Qemu-devel] [PATCH v5 04/10] ACPI: Add vmgenid storage entries to the build tables

[Laszlo Ersek <lersek@redhat.com>]

On 02/05/17 10:11, ben@skyportsystems.com wrote:
> From: Ben Warren <ben@skyportsystems.com>
> 
> This allows them to be centrally initialized and destroyed
> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> ---
>  hw/acpi/aml-build.c         | 2 ++
>  include/hw/acpi/aml-build.h | 1 +
>  2 files changed, 3 insertions(+)

Please replace "storage entries" in the subject line with "blob
storage", and add the following to the commit message:

----
The "AcpiBuildTables.vmgenid" array will be used to construct the
"etc/vmgenid" fw_cfg blob.

Its contents will be linked into fw_cfg after being built on the
pc_machine_done() -> acpi_setup() -> acpi_build() call path, and dropped
without use on the subsequent, guest triggered, acpi_build_update() ->
acpi_build() call path.
----

... This commit message update actually exposes an important flaw in the
implementation (one that entirely escaped me in the previous round!!!),
but as I've now read forward a little bit, into patch 5, I see that
Michael caught it, which is awesome!

So, with the new fw-cfg callback (on which I'll comment under the next
patch), and with the above commit message / subject updates, please add:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 03b6c6c..0f39eaf 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1594,6 +1594,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables)
>      tables->rsdp = g_array_new(false, true /* clear */, 1);
>      tables->table_data = g_array_new(false, true /* clear */, 1);
>      tables->tcpalog = g_array_new(false, true /* clear */, 1);
> +    tables->vmgenid = g_array_new(false, true /* clear */, 1);
>      tables->linker = bios_linker_loader_init();
>  }
>  
> @@ -1603,6 +1604,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
>      g_array_free(tables->rsdp, true);
>      g_array_free(tables->table_data, true);
>      g_array_free(tables->tcpalog, mfre);
> +    g_array_free(tables->vmgenid, mfre);
>  }
>  
>  /* Build rsdt table */
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index dbf63cf..6f4e239 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -210,6 +210,7 @@ struct AcpiBuildTables {
>      GArray *table_data;
>      GArray *rsdp;
>      GArray *tcpalog;
> +    GArray *vmgenid;
>      BIOSLinker *linker;
>  } AcpiBuildTables;
>  
>