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

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

[Michael Tokarev]

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

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

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

---
 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);
+    }
     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

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
> 

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

[Michael Tokarev]

06.04.2014 14:53, Michael S. Tsirkin wrote:
> 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?

It doesn't work in 1.7 too, but it is not a regression.
Before 1.7 I used a very similar approach patching seabios,
for several years.  Here's one of the first versions of this
approach -- http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg03080.html

I used this approach since that time locally.
For 1.7 I used seabios without acpi table loading interface, so
old approach worked fine for me.  Here's the current seabios patch:

--- a/src/fw/acpi.c     2013-11-24 00:03:36.000000000 +0400
+++ b/src/fw/acpi.c     2013-11-24 00:03:48.053886298 +0400
@@ -20,6 +20,8 @@

 u32 acpi_pm1a_cnt VARFSEG;

+static const struct acpi_table_header *slic;
+
 static void
 build_header(struct acpi_table_header *h, u32 sig, int len, u8 rev)
 {
@@ -32,6 +34,9 @@ build_header(struct acpi_table_header *h
     h->oem_revision = cpu_to_le32(1);
     memcpy(h->asl_compiler_id, BUILD_APPNAME4, 4);
     h->asl_compiler_revision = cpu_to_le32(1);
+    if (slic && sig == RSDT_SIGNATURE)
+        /* for win7/vista, RSDT oem should match SLIC, min 14 bytes */
+        memcpy(h->oem_id, slic->oem_id, 6 + 4 + 4);
     h->checksum -= checksum(h, len);
 }

@@ -642,6 +647,8 @@ acpi_setup(void)
             }
         } else {
             ACPI_INIT_TABLE(table);
+            if (table->signature == 0x43494c53/*SLIC*/)
+                slic = table;
         }
         if (tbl_idx == MAX_ACPI_TABLES) {
             warn_noalloc();

>> Windows7 requires that OEM ID in RSDT matches those in SLIC to
>> consider SLIC to be valid.
> 
> Which fields need to match which, exactly?

As can be seen in the patch itself, that's 2 fields - oem_id [4]
and oem_table_id[8], in SLIC and RSDT.  This is enough for win7
at least.

>> 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.
> 
> 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?

Such an api might be useful, and it actually _is_ useful to keep
compatibility with older guests, which is a slightly different
but very important topic (for example, my win8 guest which were
activated in qemu-1.1 does not work anymore in qemu-1.2, exactly
because of changed OEM ID, -- it is the commit which updated
qemu version number which breaks win8 activation here).

But this api wont help when we already have whole SLIC table as
in the example above.


Overall, the more I think about it, the less I consider it useful.
Because it looks like windows vista and 7 era is finished (yes, the
same thing applies to vista as well), now it is win8+ which does
not use this OEM activation mechanism anymore.  Instead, it uses
hw-based activation, for which we need to keep OEM ID in ACPI too.

Maybe back in 2010 when I faced this problem for the first time it
was a good idea to have slic-based adoption for rsdt in qemu, now
it isn't that important anymore.

Thanks,

/mjt

>>  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
>>

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

[Laszlo Ersek]

On 04/06/14 13:08, Michael Tokarev wrote:
> 06.04.2014 14:53, Michael S. Tsirkin wrote:
>> 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?
> 
> It doesn't work in 1.7 too, but it is not a regression.
> Before 1.7 I used a very similar approach patching seabios,
> for several years.  Here's one of the first versions of this
> approach -- http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg03080.html
> 
> I used this approach since that time locally.
> For 1.7 I used seabios without acpi table loading interface, so
> old approach worked fine for me.  Here's the current seabios patch:
> 
> --- a/src/fw/acpi.c     2013-11-24 00:03:36.000000000 +0400
> +++ b/src/fw/acpi.c     2013-11-24 00:03:48.053886298 +0400
> @@ -20,6 +20,8 @@
> 
>  u32 acpi_pm1a_cnt VARFSEG;
> 
> +static const struct acpi_table_header *slic;
> +
>  static void
>  build_header(struct acpi_table_header *h, u32 sig, int len, u8 rev)
>  {
> @@ -32,6 +34,9 @@ build_header(struct acpi_table_header *h
>      h->oem_revision = cpu_to_le32(1);
>      memcpy(h->asl_compiler_id, BUILD_APPNAME4, 4);
>      h->asl_compiler_revision = cpu_to_le32(1);
> +    if (slic && sig == RSDT_SIGNATURE)
> +        /* for win7/vista, RSDT oem should match SLIC, min 14 bytes */
> +        memcpy(h->oem_id, slic->oem_id, 6 + 4 + 4);
>      h->checksum -= checksum(h, len);
>  }
> 
> @@ -642,6 +647,8 @@ acpi_setup(void)
>              }
>          } else {
>              ACPI_INIT_TABLE(table);
> +            if (table->signature == 0x43494c53/*SLIC*/)
> +                slic = table;
>          }
>          if (tbl_idx == MAX_ACPI_TABLES) {
>              warn_noalloc();
> 
>>> Windows7 requires that OEM ID in RSDT matches those in SLIC to
>>> consider SLIC to be valid.
>>
>> Which fields need to match which, exactly?
> 
> As can be seen in the patch itself, that's 2 fields - oem_id [4]
> and oem_table_id[8], in SLIC and RSDT.  This is enough for win7
> at least.
> 
>>> 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.
>>
>> 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?
> 
> Such an api might be useful, and it actually _is_ useful to keep
> compatibility with older guests, which is a slightly different
> but very important topic (for example, my win8 guest which were
> activated in qemu-1.1 does not work anymore in qemu-1.2, exactly
> because of changed OEM ID, -- it is the commit which updated
> qemu version number which breaks win8 activation here).
> 
> But this api wont help when we already have whole SLIC table as
> in the example above.
> 
> 
> Overall, the more I think about it, the less I consider it useful.
> Because it looks like windows vista and 7 era is finished (yes, the
> same thing applies to vista as well), now it is win8+ which does
> not use this OEM activation mechanism anymore.  Instead, it uses
> hw-based activation, for which we need to keep OEM ID in ACPI too.
> 
> Maybe back in 2010 when I faced this problem for the first time it
> was a good idea to have slic-based adoption for rsdt in qemu, now
> it isn't that important anymore.

I'd like to add two points (purely FYI, ie. in no way to contradict
anything you said -- actually, my 2nd point even seems to confirm what
you said):

(1) In the OVMF ACPI download patch, we skip qemu's RSDT and XSDT on
purpose:

https://github.com/tianocore/edk2/commit/96bbdbc856930abf38d0cc289536ebd11043f80f#diff-7e7e6f95dd6acdc67fcfdc854aa5f46eR603

That's because the EFI_ACPI_TABLE_PROTOCOL, which
- is specified in UEFI-2.4A, Chapter 19,
- has an edk2 reference implementation in
MdeModulePkg/Universal/Acpi/AcpiTableDxe/,
- OVMF uses to install ACPI tables

handles RSDT/XSDT automatically, outside of OVMF's jurisdiction.

In particular, the edk2 reference implementation of
EFI_ACPI_TABLE_PROTOCOL sets the RSDT's OemId and OemTableId fields from
the same in FADT (which qemu does provide, signature "FACP"):

https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c#L695

(2) I recently found this forum post from Chris Evich (CC'd):

  Libvirt + Fedora 20 + Windows 8.1 OEM + UEFI = Oh My!
  http://forums.fedoraforum.org/showthread.php?p=1694052

(scroll up to the start of the thread).

It's real-life evidence that handling the SLIC manually (plus some other
stuff), without adapting the OemId & OemTableId fields in RSDT/XSDT to
SLIC, is sufficient to keep the Windows 8.1 OEM guest happy.

(I found the forum post because I regularly google last week's OVMF
references, to see what's up. (As a side note, it's nice to see that
users have put OVMF's PlatformDxe to use (for setting the GOP
resolution) in less than ten days after that series was committed (SVN
r15375).))

Thanks
Laszlo

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

[Chris Evich]

On 04/06/2014 08:21 AM, Laszlo Ersek wrote:
> (2) I recently found this forum post from Chris Evich (CC'd):
> 
>   Libvirt + Fedora 20 + Windows 8.1 OEM + UEFI = Oh My!
>   http://forums.fedoraforum.org/showthread.php?p=1694052
> 
> (scroll up to the start of the thread).

FWIW: "Works" in my instance was defined as "Windows 8.1 installed as a
VM and successfully passed M$ activation".  As I understand it, there is
some "slop" on the M$ side to account for a degree of hardware variation
between OEM pre-activation and the consumer "phone-home" phase.  My aim
was to do everything I could think of to match as closely as possible
and get it working.  I __did_not_go_deeper_than_that__, meaning
verifying ACPI tables and such inside the guest.

I did note the installer asked me to manually enter the license key,
which I read it's suppose to automatically read out of the MSDS table.
Maybe I needed to pull in stuff from RSDT/XSDT or none of the
table-copying business was working/necessary.  In any case, presumably
if I didn't match enough of the hardware uniqueness (uuid, mac
addresses, etc) it would have been outside the "slop".  Then again, I
suppose they could change the degree of slop at some point, so maybe the
tables will be important.

Anyway, if anyone wants more details, I'm happy to help.

-- 
Chris Evich, RHCA, RHCE, RHCDS, RHCSS
Quality Assurance Engineer
e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214

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

[Michael S. Tsirkin]

On Sun, Apr 06, 2014 at 03:08:46PM +0400, Michael Tokarev wrote:
> 06.04.2014 14:53, Michael S. Tsirkin wrote:
> > 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?
> 
> It doesn't work in 1.7 too, but it is not a regression.
> Before 1.7 I used a very similar approach patching seabios,
> for several years.  Here's one of the first versions of this
> approach -- http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg03080.html
> 
> I used this approach since that time locally.
> For 1.7 I used seabios without acpi table loading interface, so
> old approach worked fine for me.  Here's the current seabios patch:
> 
> --- a/src/fw/acpi.c     2013-11-24 00:03:36.000000000 +0400
> +++ b/src/fw/acpi.c     2013-11-24 00:03:48.053886298 +0400
> @@ -20,6 +20,8 @@
> 
>  u32 acpi_pm1a_cnt VARFSEG;
> 
> +static const struct acpi_table_header *slic;
> +
>  static void
>  build_header(struct acpi_table_header *h, u32 sig, int len, u8 rev)
>  {
> @@ -32,6 +34,9 @@ build_header(struct acpi_table_header *h
>      h->oem_revision = cpu_to_le32(1);
>      memcpy(h->asl_compiler_id, BUILD_APPNAME4, 4);
>      h->asl_compiler_revision = cpu_to_le32(1);
> +    if (slic && sig == RSDT_SIGNATURE)
> +        /* for win7/vista, RSDT oem should match SLIC, min 14 bytes */
> +        memcpy(h->oem_id, slic->oem_id, 6 + 4 + 4);
>      h->checksum -= checksum(h, len);
>  }
> 
> @@ -642,6 +647,8 @@ acpi_setup(void)
>              }
>          } else {
>              ACPI_INIT_TABLE(table);
> +            if (table->signature == 0x43494c53/*SLIC*/)
> +                slic = table;
>          }
>          if (tbl_idx == MAX_ACPI_TABLES) {
>              warn_noalloc();
> 
> >> Windows7 requires that OEM ID in RSDT matches those in SLIC to
> >> consider SLIC to be valid.
> > 
> > Which fields need to match which, exactly?
> 
> As can be seen in the patch itself, that's 2 fields - oem_id [4]

Surely oem_id[6]?

> and oem_table_id[8], in SLIC and RSDT.  This is enough for win7
> at least.
> 
> >> 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.
> > 
> > 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?
> 
> Such an api might be useful, and it actually _is_ useful to keep
> compatibility with older guests, which is a slightly different
> but very important topic (for example, my win8 guest which were
> activated in qemu-1.1 does not work anymore in qemu-1.2, exactly
> because of changed OEM ID, -- it is the commit which updated
> qemu version number which breaks win8 activation here).

I'm guessing 1.1 compatibility isn't perfect, but
this shouldn't happen with recent versions like 1.6 and 2.0 at least:
2.0 should not need re-activation if you run
with -M option compatible with 1.6.

> But this api wont help when we already have whole SLIC table as
> in the example above.

I think I wasn't clear. I merely meant an internal API,
have acpi_table_install call it internally instead of
exposing the offset field.


> 
> Overall, the more I think about it, the less I consider it useful.
> Because it looks like windows vista and 7 era is finished (yes, the
> same thing applies to vista as well),

windows  7 is still in mainstream support.  Extended support is until
2020.  Seems worth supporting.

> now it is win8+ which does
> not use this OEM activation mechanism anymore.  Instead, it uses
> hw-based activation, for which we need to keep OEM ID in ACPI too.

Yes, if this patch is not in 2.0, we need to be careful and
only change ID with new -M flag.

> Maybe back in 2010 when I faced this problem for the first time it
> was a good idea to have slic-based adoption for rsdt in qemu, now
> it isn't that important anymore.
> 
> Thanks,
> 
> /mjt
> 
> >>  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
> >>

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

[Michael Tokarev]

06.04.2014 16:25, Michael S. Tsirkin wrote:
[]
>>>> Windows7 requires that OEM ID in RSDT matches those in SLIC to
>>>> consider SLIC to be valid.
>>>
>>> Which fields need to match which, exactly?
>>
>> As can be seen in the patch itself, that's 2 fields - oem_id [4]
> 
> Surely oem_id[6]?

Yes you're right.

>> and oem_table_id[8], in SLIC and RSDT.  This is enough for win7
>> at least.

All 14 bytes in this place are needed.  Omitting any of those 3
(actually that's 3 not 2) fields -- oem_id[6], oem_table_id[4]
and oem_table_sig[4] makes windows 7 activation void.

I'm sorry I missed this question initially:

>>>> +    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 */ \

In some specs I found oem_table_id[8] is represented by 2 4-byte fields.

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

Yes it should match, whole oem_table_id[8] plus oem_id[6] itself.
I don't know what exactly those identifications mean, but without
any of those 14 bytes windows activation does not work.
oem_revision can be anything, it is not part of the equation.

It is easy to verify by playing with the memcpy() above -- this is
actually how I found the minimal amount of bytes which should match
initially.

Thanks,

/mjt