Re: [Qemu-devel] [PATCH] Propagate OEM ID info into other tables when using SLIC

[Laszlo Ersek <lersek@redhat.com>]

On 01/18/16 06:38, Xiao Guangrong wrote:
> Hi,
> 
> Is this you wanted?
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg345911.html

Right, there's now some renewed interest in this issue; I guess probably
because of <https://github.com/tianocore/edk2/issues/5>.

We have a long-standing RHBZ for it (see link in the edk2 issue tracker
entry), but Aleksei reported a bug in the upstream QEMU tracker too
<https://bugs.launchpad.net/qemu/+bug/1533848>. I think Steven noticed
the upstream QEMU bug report, and resurrected & reworked the Debain patch.

My series is a bit different -- I don't like pushing down special
knowledge into build_header(). That's a very generic utility function;
the knowledge should be in the caller(s).

If it's okay with you, Steven, let's continue with my series. I got good
comments from Michael Tsirkin; once we have testing feedback in
<https://bugzilla.redhat.com/show_bug.cgi?id=1248758>, I plan to post v2.

Of course, if Michael prefers your patch, I'm game to drop mine.

Thanks
Laszlo

> On 01/16/2016 04:19 AM, Steven Newbury wrote:
>> In order to support Windows 7 "Activation", the OEM ID info must match
>> in SLIC and RSDT, and for UEFI, FACP.  The OEM ID from the SLIC is only
>> applied when oemtableid is not specified expliicitly.
>>
>> This was originally based on the patch from Michael Tokarev but has
>> been significantly re-worked, and re-based.
>>
>> Signed-off-by: Steven Newbury <steve@snewbury.org.uk>
>>
>> ---
>>   hw/acpi/aml-build.c         | 19 ++++++++++++++++---
>>   hw/acpi/core.c              | 11 +++++++++++
>>   hw/i386/acpi-build.c        |  9 ++++++++-
>>   include/hw/acpi/acpi_slic.h | 11 +++++++++++
>>   qemu-options.hx             |  2 ++
>>   5 files changed, 48 insertions(+), 4 deletions(-)
>>   create mode 100644 include/hw/acpi/acpi_slic.h
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 78e1290..bc16dc2 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -29,6 +29,9 @@
>>   #include "qemu/bswap.h"
>>   #include "qemu/bitops.h"
>>   #include "hw/acpi/bios-linker-loader.h"
>> +#include "hw/acpi/acpi_slic.h"
>> +
>> +extern const struct slic_info oem_data;
>>
>>   static GArray *build_alloc_array(void)
>>   {
>> @@ -1435,13 +1438,23 @@ build_header(GArray *linker, GArray
>> *table_data,
>>       memcpy(&h->signature, sig, 4);
>>       h->length = cpu_to_le32(len);
>>       h->revision = rev;
>> -    memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
>>
>>       if (oem_table_id) {
>>           strncpy((char *)h->oem_table_id, oem_table_id, sizeof(h-
>>> oem_table_id));
>>       } else {
>> -        memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
>> -        memcpy(h->oem_table_id + 4, sig, 4);
>> +        /* When including the system SLIC Win7 requires all OEM info
>> to match
>> +        (including sig) in SLIC, RSDT and FACP.  Other tables could
>> match,
>> +        but is unnecessary to pass "Activation". Overriden by above.
>> */
>> +        if (oem_data.has_slic) {
>> +            memcpy(h->oem_id, &oem_data.oem_id, 6);
>> +            memcpy(h->oem_table_id, &oem_data.oem_table_id, 8);
>> +            if (memcmp(sig, "RSDT", 4) != 0 && memcmp(sig, "FACP", 4)
>> != 0)
>> +                memcpy(h->oem_table_id + 4, sig, 4);
>> +        } else {
>> +            memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
>> +            memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
>> +            memcpy(h->oem_table_id + 4, sig, 4);
>> +        }
>>       }
>>
>>       h->oem_revision = cpu_to_le32(1);
>> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
>> index 21e113d..cbef437 100644
>> --- a/hw/acpi/core.c
>> +++ b/hw/acpi/core.c
>> @@ -22,6 +22,7 @@
>>   #include "hw/hw.h"
>>   #include "hw/i386/pc.h"
>>   #include "hw/acpi/acpi.h"
>> +#include "hw/acpi/acpi_slic.h"
>>   #include "hw/nvram/fw_cfg.h"
>>   #include "qemu/config-file.h"
>>   #include "qapi/opts-visitor.h"
>> @@ -29,6 +30,9 @@
>>   #include "qapi-visit.h"
>>   #include "qapi-event.h"
>>
>> +struct slic_info oem_data;
>> +
>> +
>>   struct acpi_table_header {
>>       uint16_t _length;         /* our length, not actual part of the
>> hdr */
>>                                 /* allows easier parsing for fw_cfg
>> clients */
>> @@ -227,6 +231,13 @@ 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);
>> +
>> +    /* Copy OEM fields from SLIC for use in all relevant tables
>> +       (oem_id[6] + tableid[4] + tableid(sig)[4] = 14 bytes) */
>> +    if ((!oem_data.has_slic) && (memcmp(ext_hdr->sig, "SLIC", 4) ==
>> 0)) {
>> +       memcpy(&oem_data.oem_id, ext_hdr->oem_id, 14);
>> +       oem_data.has_slic = 1;
>> +    }
>>   }
>>
>>   void acpi_table_add(const QemuOpts *opts, Error **errp)
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 78758e2..fbe4f3a 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -55,6 +55,7 @@
>>   #include "hw/timer/hpet.h"
>>
>>   #include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/acpi_slic.h"
>>
>>   #include "qapi/qmp/qint.h"
>>   #include "qom/qom-qobject.h"
>> @@ -122,6 +123,8 @@ typedef struct AcpiBuildPciBusHotplugState {
>>       bool pcihp_bridge_en;
>>   } AcpiBuildPciBusHotplugState;
>>
>> +extern const struct slic_info oem_data;
>> +
>>   static
>>   int acpi_add_cpu_info(Object *o, void *opaque)
>>   {
>> @@ -2542,7 +2545,11 @@ build_rsdp(GArray *rsdp_table, GArray *linker,
>> unsigned rsdt)
>>                                true /* fseg memory */);
>>
>>       memcpy(&rsdp->signature, "RSD PTR ", 8);
>> -    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
>> +    if (oem_data.has_slic) {
>> +        memcpy(rsdp->oem_id, &oem_data.oem_id, 6);
>> +    } else {
>> +        memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
>> +    }
>>       rsdp->rsdt_physical_address = cpu_to_le32(rsdt);
>>       /* Address to be filled by Guest linker */
>>       bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
>> diff --git a/include/hw/acpi/acpi_slic.h b/include/hw/acpi/acpi_slic.h
>> new file mode 100644
>> index 0000000..bc04a71
>> --- /dev/null
>> +++ b/include/hw/acpi/acpi_slic.h
>> @@ -0,0 +1,11 @@
>> +#ifndef QEMU_HW_ACPI_SLIC_H
>> +#define QEMU_HW_ACPI_SLIC_H
>> +
>> +struct slic_info {
>> +    bool has_slic;
>> +    char oem_id[6];
>> +    char oem_table_id[8];
>> +};
>> +
>> +#endif /* !QEMU_HW_ACPI_SLIC_H */
>> +
>> \ No newline at end of file
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 215d00d..f1488a1 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1472,6 +1472,8 @@ ACPI headers (possible overridden by other
>> options).
>>   For data=, only data
>>   portion of the table is used, all header information is specified in
>> the
>>   command line.
>> +If a SLIC table is supplied to qemu, then the oem_id from the SLIC
>> table
>> +will be copied into the RSDT and FACP tables (this is a Debian
>> addition).
>>   ETEXI
>>
>>   DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>> -- 
>> 2.6.4
>>
>