Re: [Qemu-devel] [RFC] copy OEM ACPI parameters from SLIC table to RSDT

[Michael S. Tsirkin]

On Sun, Apr 06, 2014 at 01:49:11PM +0400, Michael Tokarev wrote:
> When building RSDT table, pick OEM ID fields from uer-supplied SLIC
> table instead of using hard-coded QEMU defaults.  This way, say,
> OEM version of Windows7 can be run inside qemu using the same OEM
> activation as on bare metal, by pointing at system firmware:
> 
>   -acpitable file=/sys/firmware/acpi/tables/SLIC

Right, so this doesn't work in 1.7 either, right?
It's not a regression?

> Windows7 requires that OEM ID in RSDT matches those in SLIC to
> consider SLIC to be valid.

Which fields need to match which, exactly?

> This is somewhat hackish approach, but it works fairy well in
> practice.
> 
> I'm not asking to apply this directly, but instead am trying to
> show what's needed to get win to work.  Maybe a new command-line
> option for that will do, maybe something else.
> 
> Thanks,
> 
> /mjt

I think it's kind of reasonable - seems better than
adding more flags - but I'd like to avoid
poking at acpi_tables array in acpi-build.c

How about an API to set/query OEM ID?


> ---
>  hw/acpi/core.c       |    5 +++++
>  hw/i386/acpi-build.c |    7 +++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 79414b4..a8a3f26 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -53,6 +53,7 @@ static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE - 
> ACPI_TABLE_PFX_SIZE] =
>  
>  char unsigned *acpi_tables;
>  size_t acpi_tables_len;
> +size_t slic_table_offset;
>  
>  static QemuOptsList qemu_acpi_opts = {
>      .name = "acpi",
> @@ -226,6 +227,10 @@ static void acpi_table_install(const char unsigned 
> *blob, size_t bloblen,
>      /* recalculate checksum */
>      ext_hdr->checksum = acpi_checksum((const char unsigned *)ext_hdr +
>                                        ACPI_TABLE_PFX_SIZE, 
> acpi_payload_size);
> +
> +    if (memcmp(ext_hdr->sig, "SLIC", 4) == 0) {
> +       slic_table_offset = acpi_tables_len - acpi_payload_size;
> +    }
>  }
>  
>  void acpi_table_add(const QemuOpts *opts, Error **errp)
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a5d3fbf..9e0e16a 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -224,6 +224,8 @@ static void acpi_get_pci_info(PcPciInfo *info)
>  #define ACPI_BUILD_TABLE_FILE "etc/acpi/tables"
>  #define ACPI_BUILD_RSDP_FILE "etc/acpi/rsdp"
>  
> +extern size_t slic_table_offset;
> +
>  static void
>  build_header(GArray *linker, GArray *table_data,
>               AcpiTableHeader *h, const char *sig, int len, uint8_t rev)
> @@ -237,6 +239,11 @@ build_header(GArray *linker, GArray *table_data,
>      h->oem_revision = cpu_to_le32(1);
>      memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
>      h->asl_compiler_revision = cpu_to_le32(1);
> +    if (memcmp(sig, "RSDT", 4) == 0 && slic_table_offset) {
> +      /* for win7: OEM info in RSDT and SLIC should be the same */
> +      AcpiTableHeader *s = (AcpiTableHeader *)(acpi_tables + 
> slic_table_offset);
> +      memcpy(h->oem_id, s->oem_id, 6 + 4 + 4);


what does 6 + 4 +4 mean?
I see:
    uint8_t  oem_id [6];             /* OEM identification */ \
    uint8_t  oem_table_id [8];       /* OEM table identification */ \
    uint32_t oem_revision;           /* OEM revision number */ \

Do table id have to match? It seems a bit wrong to have two tables
with the same id.

> +    }
>      h->checksum = 0;
>      /* Checksum to be filled in by Guest linker */
>      bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE,
> -- 
> 1.7.10.4
>