Re: [Qemu-devel] [PATCH 4/4] pc: set the OEM fields in the RSDT and the FADT from the SLIC

[Michael S. Tsirkin]

On Thu, Jan 14, 2016 at 02:36:57AM +0100, Laszlo Ersek wrote:
> The Microsoft spec about the SLIC and MSDM ACPI tables at
> <http://go.microsoft.com/fwlink/p/?LinkId=234834> requires the OEM ID and
> OEM Table ID fields to be consistent between the SLIC and the RSDT/XSDT.
> That further affects the FADT, because a similar match between the FADT
> and the RSDT/XSDT is required by the ACPI spec in general.
> 
> The stashed SLIC OEM identifiers can be ignored with the new
> 
>   -machine heed-slic-oem=no
> 
> option.

I'd prefer "use-slic-oem".

But do we really expect people to use this?
Less knobs would be better ...

> Cc: "Michael S. Tsirkin" <address@hidden> (supporter:ACPI/SMBIOS)
> Cc: Igor Mammedov <address@hidden> (supporter:ACPI/SMBIOS)
> Cc: Paolo Bonzini <address@hidden> (maintainer:X86)
> Cc: Richard W.M. Jones <address@hidden>
> Cc: Aleksei Kovura <address@hidden>
> Cc: Michael Tokarev <address@hidden>
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
>  include/hw/i386/pc.h |  2 ++
>  hw/i386/acpi-build.c | 22 ++++++++++++++++++----
>  hw/i386/pc.c         | 19 +++++++++++++++++++
>  qemu-options.hx      | 10 +++++++++-
>  4 files changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 588a33c..a762c29 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -56,6 +56,7 @@ struct PCMachineState {
>      OnOffAuto vmport;
>      OnOffAuto smm;
>      bool nvdimm;
> +    bool heed_slic_oem;
>  
>      /* RAM information (sizes, addresses, configuration): */
>      ram_addr_t below_4g_mem_size, above_4g_mem_size;
> @@ -67,6 +68,7 @@ struct PCMachineState {
>  #define PC_MACHINE_VMPORT           "vmport"
>  #define PC_MACHINE_SMM              "smm"
>  #define PC_MACHINE_NVDIMM           "nvdimm"
> +#define PC_MACHINE_HEED_SLIC_OEM    "heed-slic-oem"
>  
>  /**
>   * PCMachineClass:
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 6408362..cf2aafc 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -337,7 +337,8 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, 
> AcpiPmInfo *pm)
>  /* FADT */
>  static void
>  build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
> -           unsigned facs, unsigned dsdt)
> +           unsigned facs, unsigned dsdt,
> +           const char *oem_id, const char *oem_table_id)
>  {
>      AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
>  
> @@ -358,7 +359,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo 
> *pm,
>      fadt_setup(fadt, pm);
>  
>      build_header(linker, table_data,
> -                 (void *)fadt, "FACP", sizeof(*fadt), 1, NULL, NULL);
> +                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, 
> oem_table_id);
>  }
>  
>  static void
> @@ -2621,6 +2622,17 @@ void acpi_build(PcGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>      uint8_t *u;
>      size_t aml_len = 0;
>      GArray *tables_blob = tables->table_data;
> +    char *slic_oem_id = NULL;
> +    char *slic_oem_table_id = NULL;
> +    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> +    bool heed_slic_oem = object_property_get_bool(OBJECT(pcms),
> +                                                  PC_MACHINE_HEED_SLIC_OEM,
> +                                                  &error_abort);
> +
> +    if (heed_slic_oem) {
> +        slic_oem_id = acpi_slic_oem_id;
> +        slic_oem_table_id = acpi_slic_oem_table_id;
> +    }
>  
>      acpi_get_cpu_info(&cpu);
>      acpi_get_pm_info(&pm);
> @@ -2654,7 +2666,8 @@ void acpi_build(PcGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  
>      /* ACPI tables pointed to by RSDT */
>      acpi_add_table(table_offsets, tables_blob);
> -    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt);
> +    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
> +               slic_oem_id, slic_oem_table_id);
>  
>      ssdt = tables_blob->len;
>      acpi_add_table(table_offsets, tables_blob);
> @@ -2705,7 +2718,8 @@ void acpi_build(PcGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  
>      /* RSDT is pointed to by RSDP */
>      rsdt = tables_blob->len;
> -    build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
> +    build_rsdt(tables_blob, tables->linker, table_offsets,
> +               slic_oem_id, slic_oem_table_id);
>  
>      /* RSDP is in FSEG memory, so allocate it separately */
>      build_rsdp(tables->rsdp, tables->linker, rsdt);
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c36b8cf..3e7a72a 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1887,6 +1887,20 @@ static void pc_machine_set_nvdimm(Object *obj, bool 
> value, Error **errp)
>      pcms->nvdimm = value;
>  }
>  
> +static bool pc_machine_get_heed_slic_oem(Object *obj, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +
> +    return pcms->heed_slic_oem;
> +}
> +
> +static void pc_machine_set_heed_slic_oem(Object *obj, bool value, Error 
> **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +
> +    pcms->heed_slic_oem = value;
> +}
> +
>  static void pc_machine_initfn(Object *obj)
>  {
>      PCMachineState *pcms = PC_MACHINE(obj);
> @@ -1926,6 +1940,11 @@ static void pc_machine_initfn(Object *obj)
>      pcms->nvdimm = false;
>      object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
>                               pc_machine_set_nvdimm, &error_abort);
> +
> +    pcms->heed_slic_oem = true;
> +    object_property_add_bool(obj, PC_MACHINE_HEED_SLIC_OEM,
> +                             pc_machine_get_heed_slic_oem,
> +                             pc_machine_set_heed_slic_oem, &error_abort);
>  }
>  
>  static void pc_machine_reset(void)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 215d00d..e49964c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>      "                aes-key-wrap=on|off controls support for AES key 
> wrapping (default=on)\n"
>      "                dea-key-wrap=on|off controls support for DEA key 
> wrapping (default=on)\n"
>      "                suppress-vmdesc=on|off disables self-describing 
> migration (default=off)\n"
> -    "                nvdimm=on|off controls NVDIMM support (default=off)\n",
> +    "                nvdimm=on|off controls NVDIMM support (default=off)\n"
> +    "                heed_slic_oem=on|off adapts RSDT and FADT OEM 
> identifiers to external SLIC (default=on)\n",
>      QEMU_ARCH_ALL)
>  STEXI
>  @item -machine address@hidden,address@hidden,...]]
> @@ -84,6 +85,13 @@ controls whether DEA wrapping keys will be created to allow
>  execution of DEA cryptographic functions.  The default is on.
>  @item nvdimm=on|off
>  Enables or disables NVDIMM support. The default is off.
> address@hidden heed_slic_oem=on|off
> +If the user provides an external SLIC ACPI table with the -acpitable option,
> +then heed_slic_oem=on will adapt the OEM ID and OEM Table ID fields of the
> +auto-generated RSDT and FADT tables to the same fields in the external SLIC.
> +When heed_slic_oem is turned off, the RSDT and FADT tables will have general,
> +QEMU-branded OEM ID and OEM Table ID values. The default is on. heed_slic_oem
> +makes no difference if no SLIC table is provided by the user.
>  @end table
>  ETEXI
>  
> -- 
> 1.8.3.1