diff mbox

[RFC] docs: describe QEMU's VMGenID design

Message ID 1440793108-25061-1-git-send-email-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek Aug. 28, 2015, 8:18 p.m. UTC
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gal Hammer <ghammer@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    This is based on the super long private email discussion we had two
    months ago, plus on the IRL discussion between Michael and myself @ the
    KVM Forum 2015.

 docs/specs/vmgenid.txt | 343 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 343 insertions(+)
 create mode 100644 docs/specs/vmgenid.txt

Comments

Eric Blake Sept. 1, 2015, 7:47 p.m. UTC | #1
On 08/28/2015 02:18 PM, Laszlo Ersek wrote:
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gal Hammer <ghammer@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     This is based on the super long private email discussion we had two
>     months ago, plus on the IRL discussion between Michael and myself @ the
>     KVM Forum 2015.
> 
>  docs/specs/vmgenid.txt | 343 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 343 insertions(+)
>  create mode 100644 docs/specs/vmgenid.txt
> 

Grammar review; I may miss some technical details.

> diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
> new file mode 100644
> index 0000000..d4bf132
> --- /dev/null
> +++ b/docs/specs/vmgenid.txt
> @@ -0,0 +1,343 @@
> +Virtual Machine Generation ID Device
> +====================================

Is it worth a preamble giving a specific copyright/license for this
file? Without one, it inherits the default GPLv2+ from the top-level,
and there are some people (although I'm not one) that worry that an
independent implementation of a GPL'd spec must itself be GPL (that is,
specifically choosing a looser license for the spec may make it more
amenable to the OVMF folks).

> +
> +The Microsoft specification entitled "Virtual Machine Generation ID",
> +maintained at <http://go.microsoft.com/fwlink/?LinkId=260709>, defines an ACPI
> +feature that allows the guest OSPM to recognize when it has been returned "to
> +an earlier point in time", eg. by restoral from snapshot, or by incoming

s/eg./e.g./
s/restoral/restoring/

> +migration. Quoting the spec,

[I'm not sure that incoming migration is necessarily returning to an
earlier point in time; although I can certainly see that being the case
when repeatedly doing incoming migration from the same file]

> +
> +    The virtual machine generation ID is a feature whereby the virtual machines

s/machines/machine's/ [oh wait, you're quoting Microsoft's poor grammar]

> +    BIOS will expose a new ID. This is a 128-bit, cryptographically random
> +    integer value identifier that will be different every time the virtual
> +    machine executes from a different configuration file-such as executing from
> +    a recovered snapshot, or executing after restoring from backup. [...]
> +
> +The document you are reading now extracts the requirements set forth by the
> +VMGenID spec for hypervisors that intend to provide the feature, and describes
> +QEMU's implementation. The design below targets both SeaBIOS and OVMF as
> +compatible guest firmwares, without any changes to either of them.
> +

Otherwise, I didn't spot any obvious wording problems.
Laszlo Ersek Sept. 1, 2015, 10:05 p.m. UTC | #2
On 09/01/15 21:47, Eric Blake wrote:
> On 08/28/2015 02:18 PM, Laszlo Ersek wrote:
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Gal Hammer <ghammer@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     This is based on the super long private email discussion we had two
>>     months ago, plus on the IRL discussion between Michael and myself @ the
>>     KVM Forum 2015.
>>
>>  docs/specs/vmgenid.txt | 343 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 343 insertions(+)
>>  create mode 100644 docs/specs/vmgenid.txt
>>
> 
> Grammar review; I may miss some technical details.
> 
>> diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
>> new file mode 100644
>> index 0000000..d4bf132
>> --- /dev/null
>> +++ b/docs/specs/vmgenid.txt
>> @@ -0,0 +1,343 @@
>> +Virtual Machine Generation ID Device
>> +====================================
> 
> Is it worth a preamble giving a specific copyright/license for this
> file? Without one, it inherits the default GPLv2+ from the top-level,
> and there are some people (although I'm not one) that worry that an
> independent implementation of a GPL'd spec must itself be GPL (that is,
> specifically choosing a looser license for the spec may make it more
> amenable to the OVMF folks).

(1) Like you, I don't believe that the copyright license covering a
specification would automatically cover the source code written based on
the specification.

(2) If necessary I (wearing my red fedora) could just relicense this
stuff on the spot, as copyright holder, if the greater edk2 community
had concerns. But, OVMF patches will not be necessary at all, because...
see (3).

(3) I couldn't decide if this text file should go under docs/, or
docs/specs. The tricky fact about this specific vmgenid design is that
the guest will "know" about it, but no *specific* guest code needs to be
written for it.

First, the ACPI linker/loader clients in both SeaBIOS and OVMF should
"just work" (without any changes). In that sense the *specific* design
here is not guest ABI. The ACPI linker/loader is guest ABI instead; this
design is just a "clever" application of the ACPI linker/loader. (I can
call it clever, it's only in part my idea. :))

Second, guest operating systems will need drivers for this device
(obviously), but those drivers are ACPI based, and will have to consult
the Microsoft spec only, not this file.

Given the above two (ie. neither guest firmare developers nor guest OS
developers need read this text file), I'd say this is not guest ABI. It
is (detailed) documentation / specification for some QEMU internals.

Do you think the file should go under docs/, not docs/specs?

> 
>> +
>> +The Microsoft specification entitled "Virtual Machine Generation ID",
>> +maintained at <http://go.microsoft.com/fwlink/?LinkId=260709>, defines an ACPI
>> +feature that allows the guest OSPM to recognize when it has been returned "to
>> +an earlier point in time", eg. by restoral from snapshot, or by incoming
> 
> s/eg./e.g./

Good catch!

> s/restoral/restoring/

Okay.

> 
>> +migration. Quoting the spec,
> 
> [I'm not sure that incoming migration is necessarily returning to an
> earlier point in time; although I can certainly see that being the case
> when repeatedly doing incoming migration from the same file]

Yeah, I thought the same. The MS spec uses the word "replay" or some
such IIRC. I kinda accepted that this part wasn't going to be 100%
precise. :)

> 
>> +
>> +    The virtual machine generation ID is a feature whereby the virtual machines
> 
> s/machines/machine's/ [oh wait, you're quoting Microsoft's poor grammar]

Yep :) I noticed that. I resisted the urge to fix it. :)

> 
>> +    BIOS will expose a new ID. This is a 128-bit, cryptographically random
>> +    integer value identifier that will be different every time the virtual
>> +    machine executes from a different configuration file-such as executing from
>> +    a recovered snapshot, or executing after restoring from backup. [...]
>> +
>> +The document you are reading now extracts the requirements set forth by the
>> +VMGenID spec for hypervisors that intend to provide the feature, and describes
>> +QEMU's implementation. The design below targets both SeaBIOS and OVMF as
>> +compatible guest firmwares, without any changes to either of them.
>> +
> 
> Otherwise, I didn't spot any obvious wording problems.

Thanks for the review!
Laszlo

>
Eric Blake Sept. 1, 2015, 10:22 p.m. UTC | #3
On 09/01/2015 04:05 PM, Laszlo Ersek wrote:

> 
> (3) I couldn't decide if this text file should go under docs/, or
> docs/specs. The tricky fact about this specific vmgenid design is that
> the guest will "know" about it, but no *specific* guest code needs to be
> written for it.
> 
> First, the ACPI linker/loader clients in both SeaBIOS and OVMF should
> "just work" (without any changes). In that sense the *specific* design
> here is not guest ABI. The ACPI linker/loader is guest ABI instead; this
> design is just a "clever" application of the ACPI linker/loader. (I can
> call it clever, it's only in part my idea. :))
> 
> Second, guest operating systems will need drivers for this device
> (obviously), but those drivers are ACPI based, and will have to consult
> the Microsoft spec only, not this file.
> 
> Given the above two (ie. neither guest firmare developers nor guest OS
> developers need read this text file), I'd say this is not guest ABI. It
> is (detailed) documentation / specification for some QEMU internals.
> 
> Do you think the file should go under docs/, not docs/specs?

I don't know if there is a strong explanation for why we have the
separation, but given your argument, docs/ (all qemu-related
documentation that has no more specific location) indeed sounds slightly
better than docs/specs/ (files that independent implementations must pay
attention to).

>>> +migration. Quoting the spec,
>>

>>> +    The virtual machine generation ID is a feature whereby the virtual machines
>>
>> s/machines/machine's/ [oh wait, you're quoting Microsoft's poor grammar]
> 
> Yep :) I noticed that. I resisted the urge to fix it. :)

A pedantic editor might write "machines [sic] BIOS", but that just calls
attention to the blunder with an intent to appear snobbish.  You could
possibly get away with "whereby the virtual [machine's] BIOS" to make
the sentence grammatically correct while still marking your deviations
from the original, and without calling as much attention to the mistake,
but even that feels overboard.  So I agree with your decision to leave
it alone (we've already called enough attention to it in this thread
without needing any further time spent on it).
Michael S. Tsirkin Sept. 3, 2015, 1:49 p.m. UTC | #4
On Fri, Aug 28, 2015 at 10:18:28PM +0200, Laszlo Ersek wrote:
> +R1e. The generation ID shall not live in a page frame that could be mapped with
> +     caching disabled. (In other words, if the generation ID lives in RAM, then
> +     it shall only be mapped as cacheable.)

Same if it's in ROM and MMIO.

The rest of it looks good to me.

Acked-by: Michael S. Tsirkin <mst@redhat.com>
Laszlo Ersek Sept. 3, 2015, 2:24 p.m. UTC | #5
On 09/03/15 15:49, Michael S. Tsirkin wrote:
> On Fri, Aug 28, 2015 at 10:18:28PM +0200, Laszlo Ersek wrote:
>> +R1e. The generation ID shall not live in a page frame that could be mapped with
>> +     caching disabled. (In other words, if the generation ID lives in RAM, then
>> +     it shall only be mapped as cacheable.)
> 
> Same if it's in ROM and MMIO.

Okay, I'll reword it as:

R1e. The generation ID shall not live in a page frame that could be
     mapped with caching disabled. (In other words, regardless of
     whether the generation ID lives in RAM, ROM or MMIO, it shall only
     be mapped as cacheable.)

> The rest of it looks good to me.
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Thanks!
Laszlo
Paolo Bonzini Sept. 7, 2015, 4:30 p.m. UTC | #6
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 02/09/2015 00:22, Eric Blake wrote:
>> Do you think the file should go under docs/, not docs/specs?
> 
> I don't know if there is a strong explanation for why we have the 
> separation, but given your argument, docs/ (all qemu-related 
> documentation that has no more specific location) indeed sounds 
> slightly better than docs/specs/ (files that independent 
> implementations must pay attention to).

Fascinating distinction. :)  I think docs/ is clearer.

Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJV7bumAAoJEL/70l94x66DO5oH/2N3gmhv6kyt3N9wAkdeO6Vm
NZ6jkmNB6QW7envzppWfGkzTvTyA+0LKw8Mh6D0VHxdo+b5nm4+K24Ox62Vs3Pgy
5LU2bffffepjv6Tni/Vs2+t/TF3RY2eFiYwHt6T30ZMit1nRwfkUfz1VMUV/macO
4IBu8RL2UJPeUL8U29OT6oFxhslcDbx9MYOHrfHqFVyyzfYkvmSQ7GYKwpXlVdNj
s88xJyyDmSaNNlLkl5H8ZA19EOwDfCfbNTh6vYx94vEftaiWXlSCBtsryr6SHEIx
LmQe5IjwkxeTixtH4EowDmwVjlodsTn16BMgJZLvKU69vStM0mIHtXETHTSwBmU=
=wdgK
-----END PGP SIGNATURE-----
Laszlo Ersek Sept. 13, 2015, 11:56 a.m. UTC | #7
As the subject suggests, I have terrible news.

I'll preserve the full context here, so that it's easy to scroll back to
the ASL for reference.

I'm also CC'ing edk2-devel, because a number of BIOS developers should
be congregating there.

On 08/28/15 22:18, Laszlo Ersek wrote:
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gal Hammer <ghammer@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
>     This is based on the super long private email discussion we had two
>     months ago, plus on the IRL discussion between Michael and myself @ the
>     KVM Forum 2015.
>
>  docs/specs/vmgenid.txt | 343 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 343 insertions(+)
>  create mode 100644 docs/specs/vmgenid.txt
>
> diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
> new file mode 100644
> index 0000000..d4bf132
> --- /dev/null
> +++ b/docs/specs/vmgenid.txt
> @@ -0,0 +1,343 @@
> +Virtual Machine Generation ID Device
> +====================================
> +
> +The Microsoft specification entitled "Virtual Machine Generation ID",
> +maintained at <http://go.microsoft.com/fwlink/?LinkId=260709>, defines an ACPI
> +feature that allows the guest OSPM to recognize when it has been returned "to
> +an earlier point in time", eg. by restoral from snapshot, or by incoming
> +migration. Quoting the spec,
> +
> +    The virtual machine generation ID is a feature whereby the virtual machines
> +    BIOS will expose a new ID. This is a 128-bit, cryptographically random
> +    integer value identifier that will be different every time the virtual
> +    machine executes from a different configuration file-such as executing from
> +    a recovered snapshot, or executing after restoring from backup. [...]
> +
> +The document you are reading now extracts the requirements set forth by the
> +VMGenID spec for hypervisors that intend to provide the feature, and describes
> +QEMU's implementation. The design below targets both SeaBIOS and OVMF as
> +compatible guest firmwares, without any changes to either of them.
> +
> +Requirements
> +------------
> +
> +These requirements are extracted from the "How to implement virtual machine
> +generation ID support in a virtualization platform" section of the
> +specification, dated August 1, 2012.
> +
> +R1a. The generation ID shall live in an 8-byte aligned buffer.
> +
> +R1b. The buffer holding the generation ID shall be in guest RAM, ROM, or device
> +     MMIO range.
> +
> +R1c. The buffer holding the generation ID shall be kept separate from areas
> +     used by the operating system.
> +
> +R1d. The buffer shall not be covered by an AddressRangeMemory or
> +     AddressRangeACPI entry in the E820 or UEFI memory map.
> +
> +R1e. The generation ID shall not live in a page frame that could be mapped with
> +     caching disabled. (In other words, if the generation ID lives in RAM, then
> +     it shall only be mapped as cacheable.)
> +
> +R2 to R5. [These AML requirements are isolated well enough in the Microsoft
> +          specification for us to simply refer to them here.]
> +
> +R6. The hypervisor shall expose a _HID (hardware identifier) object in the
> +    VMGenId device's scope that is unique to the hypervisor vendor.
> +
> +Generation ID buffer design
> +---------------------------
> +
> +QEMU places the generation ID buffer inside a separate fw_cfg blob that is
> +exposed to the guest OS with the ACPI linker/loader.
> +
> +The structure of the blob is as follows. Offsets, sizes and numeric values are
> +given in decimal; furthermore the latter are encoded in little endian.
> +
> +  Offs  Field               Size  Value
> +  ----  ------------------  ----  ------------------------------------
> +     0  System Description    36
> +        Table Header
> +     0    Signature            4                                "UEFI"
> +     4    Length               4                                    62
> +     8    Revision             1                                     1
> +     9    Checksum             1                                     0
> +    10    OEMID                6        ACPI_BUILD_APPNAME6 ("BOCHS ")
> +    16    OEM Table ID         8                            "QEMUPARM"
> +    24    OEM Revision         4                                     1
> +    28    Creator ID           4          ACPI_BUILD_APPNAME4 ("BXPC")
> +    32    Creator Revision     4                                     1
> +
> +    36  UEFI Table            18
> +        Sub-Header
> +    36    Identifier          16  417a5dff-bf4b-4abc-a839-6593bb41f452
> +    52    DataOffset           2                                    54
> +
> +    54  ADDR base pointer      8                                    62
> +  ....................................................................
> +    62  OVMF SDT Header       36                                zeroes
> +        probe suppressor
> +    98  VMGenID alignment      6                                zeroes
> +        padding
> +   104  generation ID         16                       128-bit VMGenID
> +   120  fw_cfg blob         3976                                zeroes
> +        padding
> +  4096  <end of blob>
> +
> +The fw_cfg blob is divided in two parts conceptually (separated by the dotted
> +line in the diagram). The first part, up to and excluding offset 62, is a
> +"UEFI" ACPI Table, governed by the UEFI specification 2.5, Appendix O. The
> +second part is mainly padding, but it also contains the generation ID.
> +
> +The "UEFI" ACPI Table -- in the first part -- is a "normal" ACPI table whose
> +generic header is defined by the ACPI specification, but for which the UEFI
> +spec defines the "UEFI" signature and adds two more fixed fields, "Identifier"
> +and "DataOffset".
> +
> +- The Identifier field carries a 128-bit GUID, and enables firmware
> +  implementors to install several "UEFI" tables with different internal
> +  structures, enabling OSPM to tell them apart based on the (Type-)Identifier
> +  GUID field.
> +
> +  For the purposes of QEMU's VMGenID implementation, we generated a new GUID
> +  with the "uuidgen" utility. It should be different from all other
> +  "Identifier" values, present and future, but otherwise no other software need
> +  be aware of the concrete GUID value we generated.
> +
> +- The DataOffset field is just an offset into the table where the actual
> +  (Identifier-specific) data starts.
> +
> +  For the purposes of QEMU's VMGenID implementation, we simply set it to the
> +  next (QEMU-specific) field, "ADDR base pointer".
> +
> +Linker/loader commands
> +----------------------
> +
> +The name of the fw_cfg blob is "etc/acpi/qemuparam". The ALLOCATE command that
> +instructs the guest firmware to download this fw_cfg blob specifies an
> +alignment of 4096, and the blob will have size 4096 too.
> +
> +An ADD_POINTER command links the "UEFI" ACPI Table at the start of the blob
> +into the RSDT.
> +
> +Another ADD_POINTER command relocates the "ADDR base pointer" field to the
> +absolute address of the "OVMF SDT Header probe suppressor" field, within the
> +same blob.
> +
> +After this relocation, an ADD_CHECKSUM command updates the Checksum field,
> +covering the entire "UEFI" ACPI Table (which extends up to and excluding offset
> +62).
> +
> +Blob behavior under SeaBIOS
> +---------------------------
> +
> +(Most of the complexity in the blob is ignored when the guest firmware is
> +SeaBIOS.)
> +
> +- SeaBIOS's ACPI linker/loader client allocates the blob in normal RAM
> +  (satisfying R1b).
> +
> +- Because the ALLOCATE command prescribes an alignment of 4KB, and the blob's
> +  size is also 4KB, the allocation covers a standalone page frame in full
> +  (satisfying R1e).
> +
> +- The 128-bit VMGenID field is located at offset 104 within that page,
> +  resulting in a guest-physical address divisible by 8 (satisfying R1a).
> +
> +- The blob is marked as Reserved in the E820 map (satisfying R1c and R1d).
> +
> +- The "UEFI" ACPI Table at the start of the blob is linked into the RSDT,
> +  in-place.
> +
> +- The "ADDR" AML method (see later) is allowed to refer to the "UEFI" ACPI
> +  Table with the DataTableRegion operator, because the table is located in
> +  memory marked as AddressRangeReserved.
> +
> +- The "ADDR base pointer" field points at "OVMF SDT Header probe suppressor",
> +  which is right after the "UEFI" ACPI Table inside the blob. At OSPM runtime,
> +  the "ADDR" AML method reads the "ADDR base pointer" field, and adds 42, to
> +  arrive at the address of the VMGenID field.
> +
> +  blob @ page offset 0              RSDT
> +  +-----------------------+         +-----+
> +  | "UEFI" ACPI Table <---------+   | ... |
> +  | +-------------------+ |     |   | ... |
> +  | | ...               | |     +---- ... |
> +  | | ...               | |         +-----+
> +  | | ADDR base pointer -----+
> +  | +-------------------+ |  |
> +  | probe suppressor <-------+
> +  | VMGenID @ offset 104  |
> +  | padding               |
> +  +-----------------------+
> +
> +Blob behavior under OVMF
> +------------------------
> +
> +The complexity in the blob is required by the two-pass nature of OVMF's ACPI
> +linker/loader client, which in turn comes from the fact that OVMF has to
> +dissect blobs into individual ACPI tables vs. "other things", tracking the
> +ADD_POINTER commands, so that tables can be installed individually, with
> +EFI_ACPI_TABLE_PROTOCOL.
> +
> +- OVMF's ACPI linker/loader client allocates the blob in normal RAM (satisfying
> +  R1b).
> +
> +- Because the ALLOCATE command prescribes an alignment of 4KB, and the blob's
> +  size is also 4KB, the allocation covers a standalone page frame in full
> +  (satisfying R1e).
> +
> +- The 128-bit VMGenID field is located at offset 104 within that page,
> +  resulting in a guest-physical address divisible by 8 (satisfying R1a).
> +
> +- OVMF's ACPI linker/loader allocates the blob in EfiACPIMemoryNVS type memory,
> +  therefore it is marked as such in the UEFI memmap (satisfying R1c and R1d).
> +
> +- OVMF identifies the "UEFI" ACPI Table at the start of the blob in the second
> +  pass, following the ADD_POINTER command that is meant to link the table into
> +  the RSDT. OVMF installs a *copy* of the "UEFI" ACPI Table with
> +  EFI_ACPI_TABLE_PROTOCOL (linking the copy into both RSDT and XSDT). Given the
> +  "UEFI" signature of the table, EFI_ACPI_TABLE_PROTOCOL places the copy of the
> +  table in EfiACPIMemoryNVS type memory.
> +
> +- The "ADDR" AML method (see later) is allowed to refer to the "UEFI" ACPI
> +  Table with the DataTableRegion operator, because the table is located in
> +  memory marked as AddressRangeNVS.
> +
> +- The "ADDR base pointer" field inside the installed table points at "OVMF SDT
> +  Header probe suppressor" in the original blob. Because this field is filled
> +  with zeros, OVMF's table identification heuristics unconditionally reports a
> +  negative when it tracks the relevant ADD_POINTER command to it in the second
> +  pass. Therefore the blob is marked as "hosts something else than just ACPI
> +  tables", and it is preserved permanently (in the same EfiACPIMemoryNVS type
> +  memory where it has been originally allocated).
> +
> +  At OSPM runtime, the "ADDR" AML method reads the "ADDR base pointer" field,
> +  and adds 42, to arrive at the address of the VMGenID field.
> +
> +  blob @ page offset 0               RSDT         XSDT
> +  +-----------------------------+    +-----+      +-----+
> +  | "UEFI" ACPI Table (in blob) |    | ... |      | ... |
> +  | +-------------------------+ |    | ... ---+   | ... ---------------+
> +  | |XXXXXXXXXXXXXXXXXXXXXXXXX| |    +-----+  |   +-----+              |
> +  | |XXXXXXX [unused] XXXXXXXX| |             |                        |
> +  | |XXXXXXXXXXXXXXXXXXXXXXXXX| |             +------------------------+
> +  | +-------------------------+ |                                      |
> +  | probe suppressor <-------------+  "UEFI" ACPI Table (installed) <--+
> +  | VMGenID @ offset 104        |  |  +---------------------------+
> +  | padding                     |  |  | ...                       |
> +  +-----------------------------+  |  | ...                       |
> +                                   +--- ADDR base pointer         |
> +                                      +---------------------------+
> +
> +ACPI device, control methods
> +----------------------------
> +
> +Requirements R2 through R6 of the VMGenID specification are satisfied with the
> +following ACPI logic, exposed by QEMU's ACPI generator in one of the SSDTs, and
> +installed by both guest firmwares as such.
> +
> +The basic idea is that, when the appropriate guest driver calls the ADDR method
> +(see R4), OSPM locates the generation ID field in the 4KB blob that lives in
> +E820 Reserved (SeaBIOS) or EfiACPIMemoryNVS type (OVMF) memory. The
> +guest-physical address of the field is communicated to QEMU via IO ports
> +[0x512..0x519] inclusive. Then QEMU is cued through IO port 0x51A to refresh
> +(and keep refreshing when appropriate) the generation ID at the passed back
> +address. Finally, the method returns the address to the guest driver too, in
> +the format required by R4.
> +
> +    Scope(\_SB) {
> +        Device (VMGI) {
> +            /* satisfy R2 */
> +            Name (_CID, "VM_Gen_Counter")
> +
> +            /* satisfy R3 */
> +            Name (_DDN, "VM_Gen_Counter")
> +
> +            /* satisfy R6 */
> +            Name (_HID, "QEMU0002")
> +
> +            /* the device owns this IO port range */
> +            Name (_CRS, ResourceTemplate () {
> +                IO (Decode16, 0x512, 0x512, 1, 9)
> +            })
> +
> +            /* Device status: present, enabled & decoding resources, should be
> +             * shown in the UI, functioning properly.
> +             */
> +            Name (_STA, 0xF)
> +
> +            /* Satisfy R4.
> +             *
> +             * This method is serialized because it creates named objects.
> +             */
> +            Method (ADDR, 0, Serialized) {
> +                /* The 8-byte integer field defined as ADBP below is the
> +                 * "ADDR base pointer" field in the UEFI ACPI Table.
> +                 *
> +                 * The DataTableRegion() operator locates that ACPI table by
> +                 * scanning the RSDT/XSDT using the (SignatureString,
> +                 * OemIDString, OemTableIDString) triplet as key.
> +                 *
> +                 * Windows XP would normally crash on the DataTableRegion()
> +                 * operator, but it never calls the ADDR method, hence it never
> +                 * reaches or evaluates DataTableRegion().
> +                 */
> +                DataTableRegion (TBLR, "UEFI", "BOCHS", "QEMUPARM")
> +                Field (TBLR, AnyAcc, NoLock, Preserve) {
> +                  Offset (54),
> +                  ADBP, 64
> +                }
> +
> +                /* This is the IO port range exposed in the _CRS above.
> +                 *
> +                 * The first two 4-byte ports are used to communicate the
> +                 * 64-bit guest-physical address of the actual (relocated)
> +                 * 128-bit generation ID field to QEMU, in little endian
> +                 * encoding, so that QEMU can rewrite that field in guest RAM.
> +                 *
> +                 * A write to last 1-byte port signals that the address has
> +                 * been written fully, and QEMU is free to dereference it.
> +                 */
> +                OperationRegion (VMGR, SystemIO, 0x512, 9)
> +                Field (VMGR, DWordAcc, NoLock, Preserve) {
> +                    PTLO, 32,
> +                    PTHI, 32,
> +                    AccessAs (ByteAcc),
> +                    DONE, 8
> +                }
> +
> +                /* The ADBP field points to the "OVMF SDT Header probe
> +                 * suppressor" area in the blob, at offset 62. In order to
> +                 * arrive at the generation ID field at offset 104, we must add
> +                 * 42 dynamically.
> +                 *
> +                 * The RESU buffer below will contain the result of the
> +                 * addition. The ADFU field exposes it as an 8-byte integer
> +                 * (for storing the sum), while the ADLO and ADHI fields enable
> +                 * us to access the result in two separate 4-byte integers.
> +                 * This exact integer width is especially important for
> +                 * composing the package object that the ADDR method must
> +                 * return.
> +                 */
> +                Name (RESU, Buffer (8) {})
> +                CreateQWordField (RESU, 0, ADFU)
> +                CreateDWordField (RESU, 0, ADLO)
> +                CreateDWordField (RESU, 4, ADHI)
> +
> +                Add (ADBP, 42, ADFU)
> +                Store (ADLO, PTLO)
> +                Store (ADHI, PTHI)
> +                Store (0, DONE)
> +                Return (Package (2) { ADLO, ADHI })
> +            }
> +        }
> +    }
> +
> +    /* satisfy R5 */
> +    Scope (\_GPE) {
> +        Method (_E04) {
> +            Notify (\_SB.VMGI, 0x80)
> +        }
> +    }
>

In the last few days, I implemented all of the above (ie. all the ACPI
stuff, but not the actual device model). I was very excited about the
ACPI part, and figured I'd do that first, to see if Windows would like
it, and if so, do the device model as second (reusing as much of Gal's
work as possible).

I intend to post my current patches later, only as an FYI, for
posterity. I validated them extensively *outside of Windows* with the
following methods:

- I used the OVMF debug log to sanity check the linker/loader commands'
  execution, and to glean the actual allocation address of the fw_cfg
  blob containing the UEFI ACPI Data Table and the generation ID field.

- I used a Fedora guest to validate whether both the original allocation
  of the blob, and the UEFI ACPI Data Table separately installed with
  EFI_ACPI_TABLE_PROTOCOL, were in AcpiNVS type memory, according to the
  UEFI memory map. They were.

- I checked the dmesg.

- I used the ACPICA instance, wrapped with Python, and built into GRUB,
  from the BITS ISO (http://biosbits.org/download/), for dynamic
  verification.

  I've recently learned about this incredible tool from
  <http://lwn.net/Articles/655992/>. The killer feature of ACPI testing
  with BITS is that there is no interference from any OS at all, because
  there *is* no OS. BITS runs in a pure firmware environment, both on
  UEFI and on legacy BIOS -- it is ACPICA in Python in GRUB on top of
  the firmware.

  In a nutshell (hat tip to Josh Triplett for usage help), you boot the
  ISO, then select "Python Interpreter" from the GURB menu, then do the
  following:

  >>> import acpi
  >>> acpi.get_objpaths("ADDR")
  >>> acpi.evaluate("\\_SB_.VMGI.ADDR")

  The second command will locate the ADDR control method with full path
  (it performs a "wildcard" search in the ACPI namespace if you like),
  and the third command will actually call and evaluate that method, and
  print the return value.

  ("Unsafe" ioport access is not done by default, but one can pass the
  "unsafe_io=True" keyword arg to the Python method, and then the ioport
  accesses are done too. In any case, I didn't use that for now, because
  the actual device model was still missing, as I've said.)

  The above test proved that my code (and the design behind it) were
  sound; the UEFI ACPI Data Table was looked up just fine, the ADBP
  field was read out of it, incremented by 42 in AML, and the absolute
  address of the final generation ID field (which I cross-referenced
  with the OVMF debug log -- see the allocation address of the blob
  above) was *right*.

Pop the champagne, correct? Not so fast. I booted Windows Server 2012 R2
on top of this thing... and Device Manager showed the familiar yellow
triangle on the "Microsoft Hyper-V Generation Counter" system device.

I collected as much data as I could from Device Manager:

> General
> -------
> This device cannot find enough free resources that it can use. (Code
> 12)
>
> If you want to use this device, you will need to disable one of the
> other devices on this system.
>
> Events
> ------
> * Device configured (wgencounter.inf)
> Device ACPI\QEMU0002\2&daba3ff&1 was configured.
>
> Driver Name: wgencounter.inf
> Class Guid: {4D36E97D-E325-11CE-BFC1-08002BE10318}
> Driver Date: 06/21/2006
> Driver Version: 6.3.9600.16384
> Driver Provider: Microsoft
> Driver Section: VmGenCounter.NT
> Driver Rank: 0xFF2001
> Matching Device Id: VM_Gen_Counter
> Outranked Drivers:
> Device Updated: false
>
> * Device not started (gencounter)
> Device ACPI\QEMU0002\2&daba3ff&1 had a problem starting.
>
> Driver Name: wgencounter.inf
> Class Guid: {4D36E97D-E325-11CE-BFC1-08002BE10318}
> Service: gencounter
> Lower Filters:
> Upper Filters:
> Problem: 0xC
> Status: 0x0
>
> Resources
> ---------
> I/O Range 0512-051A
> Conflicting device list:
> No conflicts.
>
> Details
> -------
> *Problem status
> {Conflicting Address Range}
> The specified address range conflicts with the address space.
> C0000018

Note that there were *no* conflicting devices!

At this point I applied Gal's v15 from the mailing list archive to
Peter's one merge commit on master that just preceded Gal's posting of
v15 in April. That commit was 87a8adc087, and Gal's v15 applied to it
just fine. Then I rebased Gal's v15 to current master (where my own work
was based off of), namely commit fc04a730b7, fixed up the conflicts,
built it and tested it.

As I expected (because I was sure Gal had tested his work), the yellow
triangle was *not* there. So what was the difference? Well, with gradual
elimination of stuff, and parallel tweaking of Gal's work and mine, I
managed to meet in the middle, and ascertained the following facts:

- If you have no _CRS for VMGENID at all, that's fine.

- If you have a _CRS with nothing in it, that's fine too.

- If you have a _CRS with just a fixed memory32 descriptor in it, that's
  cool.

- If you have an IO descriptor in the _CRS (regardless of a present or
  absent memory descriptor "neighbor"), then the vmgenid driver in
  Windows chokes. In other words, exposing a truthful _CRS was "too
  honest" for Windows' VMGENID driver; we must not admit that we use IO
  ports in the ADDR method.

(Side note: this is not mentioned in the VMGENID spec...)

Anyhow, problem solved, right? Not so fast. When I removed the _CRS, the
error quoted above ("Conflicting Address Range") went away, but another
one popped up: "POWER FAILURE" (yellow triangle again).

(Side note again: I reproduced the same POWER FAILURE error with a
*SeaBIOS* Windows Server 2012 R2 guest as well. So it was not specific
to OVMF.)

I continued the tweaking and gradual isolation (I even tried to bump the
DSDT revision to 2, but that was useless too). Ultimately I was forced
to conclude that it was the DataTableRegion() operator that caused
Windows Server 2012 R2 to reject the device.

At this point let me remind you guys of my extensive, successful testing
of the code with ACPICA (which included BITS and a Fedora guest).
Knowing that Windows had its own ACPI interpreter, I started suspecting
that even *modern* Windows lacked support for DataTableRegion(). But, I
wanted more proof than "it works with ACPICA".

It turns out that you can debug ACPI with WinDbg. If the debuggee is a
debug checked build of Windows, or you install the debug checked
ACPI.SYS separately on a non-debug checked build of Windows, then
ACPI.SYS itself will include the AML debug agent, and you will be able
to talk to it from WinDbg.

I happened to have a debug checked build of Windows 8 lying around, and
it also reproduced the POWER FAILURE error, so it was ideal for the
debuggee role.

(1) In the debugger virtual machine (windows server 2012 r2), I
    installed WinDbg.

    https://msdn.microsoft.com/en-us/library/windows/hardware/ff551063

    "If you want to download only Debugging Tools for Windows, install
    the Windows SDK, and, during the installation, select the Debugging
    Tools for Windows box and clear all the other boxes."

    I also configured the symbol servers (to download the debug symbols
    from). I don't have the exact URLs that explain this, but it's easy
    to google and do.

(2) In the debuggee, I enabled kernel debugging, on the COM2 serial
    port. Note that the following command must be run from "cmd.exe" (as
    administrator); PowerShell will *not* work:

    bcdedit /dbgsettings serial debugport:2 baudrate:38400

    https://msdn.microsoft.com/en-us/library/windows/hardware/ff542187%28v=vs.85%29.aspx

    I chose 38400 as baud rate, because in the past I tried to use
    another debugger (the UDK debugger) through the virtual serial ports
    of QEMU, and the serial port timings were so distant from those of a
    physical serial port that the UDK debugger could never complete the
    handshake. So I intended to decrease "jitter" by choosing a
    relatively low baud rate.

    Further, I chose COM2 because all my guests are managed by libvirt,
    and libvirt kinda-sorta insists on associating COM1 with the
    "console" of the virtual machine, for historical reasons. It can be
    overridden from the command line with "virsh console --device", but
    I just wanted it to be separate, hence COM2. (Hat tip to Dan
    Berrange for explaining it to me earlier; I'm sure I paraphrased him
    incorrectly now. Sorry.)

(3) So, in the domain XML of the debuggee, I added

    <serial type='tcp'>
      <source mode='connect' host='127.0.0.1' service='12345'/>
      <protocol type='raw'/>
      <target port='1'/>
    </serial>

    (Hat tip again to DanPB for recommending TCP; it avoids any SELinux
    issues that pop up with Unix domain sockets.)

    The debuggee VM is the one that connects, because the debugger
    connection must me made early on during the boot process, and it is
    the debuggee that you might have to restart frequently. So it is the
    one that should initiate connections, and the long-running debugger
    VM should be there listening all the time.

(4) In the domain XML of the debugger, I added

    <serial type='tcp'>
      <source mode='bind' host='127.0.0.1' service='12345'/>
      <protocol type='raw'/>
      <target port='1'/>
    </serial>

That much about the configuration.

Now this is the interesting part. WinDbg has two modes of operation
while debugging a kernel:

https://msdn.microsoft.com/en-us/library/windows/hardware/ff558869%28v=vs.85%29.aspx

- "KD" mode:
  - you can call the AMLI (= AML debugger) *extensions* with the !amli
    prefix,
  - you can't call any AMLI *commands*,
  - you can call all KD commands,

- "AMLI" mode:
  - you can call the AMLI extensions *without* the !amli prefix,
  - you can call the AMLI commands without any prefixes too,
  - you cannot call KD commands; first you have to exit back to KD with
    the "q" command.

So, in the debugger VM, I started WinDbg, selected File | Kernel Debug,
specified COM2 and 38400, and clicked OK.

Then I started the debuggee VM. Here's the debug session transcript:

> ************* Symbol Path validation summary **************
> Response                         Time (ms)     Location
> Deferred                                       SRV*c:\symbols*http://msdl.microsoft.com/download/symbols
> Waiting to reconnect...
> Connected to Windows 8 9200 x64 target at (Sun Sep 13 11:10:57.162 2015 (UTC + 2:00)), ptr64 TRUE
> Kernel Debugger connection established.
>
> ************* Symbol Path validation summary **************
> Response                         Time (ms)     Location
> Deferred                                       SRV*c:\symbols*http://msdl.microsoft.com/download/symbols
> Symbol search path is: SRV*c:\symbols*http://msdl.microsoft.com/download/symbols
> Executable search path is:
> Windows 8 Kernel Version 9200 MP (1 procs) Checked x64
> Built by: 9200.16384.amd64chk.win8_rtm.120725-1247
> Machine Name:
> Kernel base = 0xfffff800`76a02000 PsLoadedModuleList = 0xfffff800`77145ac0
> System Uptime: 0 days 0:00:00.041 (checked kernels begin at 49 days)
> nt!DebugService2+0x5:
> fffff800`76c34565 cc              int     3

This is when the connection between debugger and debuggee is
established, and the debug agent breaks into WinDbg as soon as possible.

At this point I tried to set the "verboseon", "traceon", and "errbrkon"
AMLI Debugger options (see
<https://msdn.microsoft.com/en-us/library/windows/hardware/ff538158%28v=vs.85%29.aspx>),
but the AMLI debug agent (part of ACPI.SYS on the debuggee) was not
available yet:

> kd> !amli set verboseon
> AMLI_DBGERR: failed to to get the address of ACPI!gDebugger 0
> AMLI_DBGERR: failed to to get the address of ACPI!gdwfAMLIInit 0

So I just let the debuggee continue, with the "g" command:

> kd> g

but right after entering that command, I clicked Debug | Break too.

Namely, if you allow the debuggee to continue from this point
undisturbed, it will *not* break into the debugger when the "error" in
the ADDR control method is encountered. It will simply boot to the full
GUI instead.

However, by clicking Debug | Break immediately, I *queued* a break for
the earliest next possibility. Which is not "right after"; the debuggee
actually runs for tens of seconds uninterrupted after the "g" command
above, before the break occurs. This is it (note "first chance"):

> Break instruction exception - code 80000003 (first chance)
> *******************************************************************************
> *                                                                             *
> *   You are seeing this message because you pressed either                    *
> *       CTRL+C (if you run console kernel debugger) or,                       *
> *       CTRL+BREAK (if you run GUI kernel debugger),                          *
> *   on your debugger machine's keyboard.                                      *
> *                                                                             *
> *                   THIS IS NOT A BUG OR A SYSTEM CRASH                       *
> *                                                                             *
> * If you did not intend to break into the debugger, press the "g" key, then   *
> * press the "Enter" key now.  This message might immediately reappear.  If it *
> * does, press "g" and "Enter" again.                                          *
> *                                                                             *
> *******************************************************************************
> nt!DbgBreakPointWithStatus:
> fffff800`76c34510 cc              int     3

Cool. So now I *can* set those AMLI debugger options:

> kd> !amli set verboseon
>
> kd> !amli set traceon
>
> kd> !amli set errbrkon

And then I allow the debuggee to continue.

> kd> g

It will print a bunch of debug messages that are not relevant now (side
giggle: notice all those virtio-win messages?)

> KDTARGET: Refreshing KD connection
> [ParaNdis_DebugInitialize] Crash callback registered
> [DriverEntry]=>
> Sep 26 2013 09:53:55built 6.30
> [ParaNdis6_AddDevice]=>
> [ParaNdis6_StartDevice]=>
> [ParaNdis6_Initialize]=>
> [ParaNdis_InitializeContext]=>
> Found IO ports at 0000C0C0(32)
> Found Interrupt vector -8, level 65536, affinity 0, flags 7
> Found Interrupt vector -9, level 65536, affinity 0, flags 7
> Found Interrupt vector -10, level 65536, affinity 0, flags 7
> [ParaNdis_InitializeContext] Message interrupt assigned
> [ParaNdis_ResetVirtIONetDevice] Done
> VirtIO Host Feature VIRTIO_NET_F_CSUM
> VirtIO Host Feature VIRTIO_NET_F_GUEST_CSUM
> VirtIO Host Feature VIRTIO_NET_F_MAC
> VirtIO Host Feature VIRTIO_NET_F_GSO
> VirtIO Host Feature VIRTIO_NET_F_GUEST_TSO4
> VirtIO Host Feature VIRTIO_NET_F_GUEST_TSO6
> VirtIO Host Feature VIRTIO_NET_F_GUEST_ECN
> VirtIO Host Feature VIRTIO_NET_F_GUEST_UFO
> VirtIO Host Feature VIRTIO_NET_F_HOST_TSO4
> VirtIO Host Feature VIRTIO_NET_F_HOST_TSO6
> VirtIO Host Feature VIRTIO_NET_F_HOST_ECN
> VirtIO Host Feature VIRTIO_NET_F_HOST_UFO
> VirtIO Host Feature VIRTIO_NET_F_MRG_RXBUF
> VirtIO Host Feature VIRTIO_NET_F_STATUS
> VirtIO Host Feature VIRTIO_NET_F_CTRL_VQ
> VirtIO Host Feature VIRTIO_NET_F_CTRL_RX
> VirtIO Host Feature VIRTIO_NET_F_CTRL_VLAN
> VirtIO Host Feature VIRTIO_NET_F_CTRL_RX_EXTRA
> VirtIO Host Feature VIRTIO_NET_F_CTRL_MAC_ADDR
> VirtIO Host Feature VIRTIO_F_INDIRECT
> VirtIO Host Feature VIRTIO_F_PUBLISH_INDICES
> [ParaNdis_InitializeContext] Link status on driver startup: 1
> Permanent device MAC: 52-54-00-59-1d-3e
> No valid MAC configured
> Actual MAC: 52-54-00-59-1d-3e
> [ParaNdis_InitializeContext]<=0x0
> [ParaNdis_FinishInitialization]=>
> [ParaNdis_FinishSpecificInitialization]=>
> [ParaNdis_FinishSpecificInitialization] SG recommended size 808
> [ConfigureMSIXVectors] Using MSIX interrupts (3 messages, irql 10)
> [ConfigureMSIXVectors] MSIX message0=000049A2=>FEE0300C
> [ConfigureMSIXVectors] MSIX message1=00004992=>FEE0300C
> [ConfigureMSIXVectors] MSIX message2=00004982=>FEE0300C
> [ConfigureMSIXVectors] Using message 0 for RX queue
> [ConfigureMSIXVectors] Using message 1 for TX queue
> [ConfigureMSIXVectors] Using message 2 for controls
> [ApplyOffloadConfiguration] Requested: V4:IPCS=TxRx,TCPCS=TxRx,UDPCS=TxRx V6:TCPCS=TxRx,UDPCS=TxRx
> [SetOffloadField] IN TxIPChecksum TX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT TxIPChecksum TX: new=1 (accepted)
> [SetOffloadField] IN TxTCPChecksum TX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT TxTCPChecksum TX: new=1 (accepted)
> [SetOffloadField] IN TxUDPChecksum TX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT TxUDPChecksum TX: new=1 (accepted)
> [SetOffloadField] IN RxIPChecksum RX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT RxIPChecksum RX: new=1 (accepted)
> [SetOffloadField] IN RxTCPChecksum RX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT RxTCPChecksum RX: new=1 (accepted)
> [SetOffloadField] IN RxUDPChecksum RX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT RxUDPChecksum RX: new=1 (accepted)
> [SetOffloadField] IN TxTCPv6Checksum TX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT TxTCPv6Checksum TX: new=1 (accepted)
> [SetOffloadField] IN TxUDPv6Checksum TX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT TxUDPv6Checksum TX: new=1 (accepted)
> [SetOffloadField] IN RxTCPv6Checksum RX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT RxTCPv6Checksum RX: new=1 (accepted)
> [SetOffloadField] IN RxUDPv6Checksum RX: current=-1 Supported TxRx Requested TxRx
> [SetOffloadField] OUT RxUDPv6Checksum RX: new=1 (accepted)
> [ApplyOffloadConfiguration] Result: TCPv4:TxRx,UDPv4:TxRx,IPCS:TxRx TCPv6:TxRx,UDPv6:TxRx
> [ApplyOffloadConfiguration] Result: LSO: v4 -1, v6 -1
> [ApplyOffloadConfiguration] Final: the request accepted
> [ParaNdis_FinishSpecificInitialization]<=0x0
> [ParaNdis_VirtIONetInit]=>
> VirtIODeviceQueryQueueAllocation: queue 0 requires 0x4000 for 256 entries
> VirtIODeviceQueryQueueAllocation: queue 1 requires 0x4000 for 256 entries
> VirtIODeviceQueryQueueAllocation: queue 2 requires 0x3000 for 64 entries
> Creating new virtqueue>>> size 256, pages FFFFF880032C1000
> [VirtIODevicePrepareQueue] queue phys.address 00000000:4f6a0000, pfn 4f6a0
> Creating new virtqueue>>> size 256, pages FFFFF880032C5000
> [VirtIODevicePrepareQueue] queue phys.address 00000000:4f69c000, pfn 4f69c
> Creating new virtqueue>>> size 64, pages FFFFF880032C9000
> [VirtIODevicePrepareQueue] queue phys.address 00000000:4f6bd000, pfn 4f6bd
> IntelPPM.sys: RegisterIdleStates() Failed! rc=0xc00000bb
> IntelPPM.sys: RegisterIdleStates() Failed! rc=0xc00000bb
> [PrepareTransmitBuffers] available 128 Tx descriptors, 256 hw buffers
> [PrepareReceiveBuffers] MaxReceiveBuffers 256
> Indicating Connected
> [ParaNdis_FinishInitialization]<=0x0
> [ParaNdis6_Restart]=>
> [ParaNdis_OnPnPEvent]=>
> [ParaNdis_OnPnPEvent] (NdisDevicePnPEventPowerProfileChanged)
>
> AMLI(? for help)->

Aaaand there we go. Because I set "errbrkon", "any AML error [would]
cause ACPI to break into the AMLI Debugger".

Then I called the "r" AMLI debugger extension (without the !amli prefix
because the mode was already "AMLI", not "KD"). It prints the ACPI
context
<https://msdn.microsoft.com/en-us/library/windows/hardware/ff562102%28v=vs.85%29.aspx>.

> r
>
> Context=fffffa800415d000*, Queue=0, ResList=fffffa800415d1c0
> ThreadID=fffffa8000fb5640, Flags=00000010, pbOp=fffffa8004057192:[\_SB.VMGI.ADDR+1]
> StackTop=fffffa800416ce30, UsedStackSize=464 bytes, FreeStackSize=64560 bytes
> LocalHeap=fffffa800415d168, CurrentHeap=fffffa800415d168, UsedHeapSize=152 bytes
> Object=\_SB.VMGI.ADDR, Scope=\_SB.VMGI.ADDR, ObjectOwner=fffffa800415d1e0, SyncLevel=0
> AsyncCallBack=fffff88000e5ce9c, CallBackData=fffff8a003458230, CallBackContext=fffff88001794c90
>
> MethodObject=\_SB.VMGI.ADDR
> fffffa800416ceb8: Local0=Unknown()
> fffffa800416cee0: Local1=Unknown()
> fffffa800416cf08: Local2=Unknown()
> fffffa800416cf30: Local3=Unknown()
> fffffa800416cf58: Local4=Unknown()
> fffffa800416cf80: Local5=Unknown()
> fffffa800416cfa8: Local6=Unknown()
> fffffa800416cfd0: Local7=Unknown()
> fffffa800415d080: RetObj=Unknown()
>
> Resources Owned:
>   ResType=Mutex, ResObj=fffffa80040570e0
>
> Next AML Pointer: fffffa8004057192:[\_SB.VMGI.ADDR+1]
>
> fffffa8004057192 : Index(TBLR, "UEFI", AMLI_DBGERR: UnAsmSuperName: invalid SuperName - 0x0d

Okay, so we are in \_SB.VMGI.ADDR, at byte offset one; there was an
error that has dropped us into the debugger; and the debugger
disassembles the offending AML as...

    Index(TBLR, "UEFI", ...

WHAT?! The code has no Index() operator whatsoever!

Also:

    ... AMLI_DBGERR: UnAsmSuperName: invalid SuperName - 0x0d

why does the AML disassembler report a byte stream decoding failure
*inside* the (in its own right unfathomable) Index() operator?!

Now, refer to the ACPI specification, revision 2.0 (July 27, 2000), for
seeing how the Index() ASL operator is compiled:

> 17.2.4.4 Type 2 Opcodes Encoding
>
> DefIndex  := IndexOp BuffPkgStrObj IndexValue Target
> IndexOp   := 0x88

Uh-oh. Now let's see how DataTableRegion() is compiled (note: my QEMU
code correctly generates that AML, and ACPICA agrees!):

> 17.2.4.2 Named Objects Encoding
>
> DefDataRegion := DataRegionOp NameString TermArg TermArg TermArg
> DataRegionOp  := ExOpPrefix 0x88

(On the side we have to note that there's a typo in the spec here (it
should be Ex[t]OpPrefix, not Ex[]OpPrefix), and that this typo lives on
in ACPI v6.0.)

Anyway, for completeness:

> 17.2.2 Data Objects Encoding
>
> ExtOpPrefix := 0x5b

Do you see it? DataRegionOp is encoded with 0x88, and IndexOp is *also*
encoded with 0x88. However, the former is *preceded* by 0x5b; this is
how AML interpreters are supposed to distinguish DataRegionOp from
IndexOp.

And while ACPICA does distinguish them, Windows does not.

ACPI.SYS happily strolls over the ExtOpPrefix (0x5b) that is at offset
#0 of the ADDR method's body, then misinterprets the following 0x88 as
an IndexOp. Then it believes to see a malformed DefIndex, instead of the
actual, well-formed DefDataRegion!

Thus, this is a bug in ACPI.SYS. (Remember: seeing this in Windows 8 and
Windows Server 2012 R2.)

"But", you ponder, "how do then the programmers that use Microsoft's ASL
compiler, ASL.exe, instead of Intel's iASL, employ DataTableRegion()? If
DataTableRegion() doesn't *run*, how and why can they *compile* it?"

They can't.

(Let me digress: why oh why must one download the ginormous Visual
Studio 2015 Community Edition, and then the WDK too, just to acquire a
comparatively tiny ASL compiler?

  https://msdn.microsoft.com/en-us/library/windows/hardware/dn551195%28v=vs.85%29.aspx
  https://msdn.microsoft.com/en-us/windows/hardware/dn913721.aspx

Well, thankfully there's a way around it. Comments in the edk2 source
file

  https://github.com/tianocore/edk2/blob/master/BaseTools/Conf/tools_def.template

contain this nice link:

  http://download.microsoft.com/download/2/c/1/2c16c7e0-96c1-40f5-81fc-3e4bf7b65496/microsoft_asl_compiler-v4-0-0.msi

Exactly what I needed. Note that this is an ACPI 4.0 compatible
compiler, and DataTableRegion() is from ACPI 2.0!)

So obviously I wrapped the entire AML code from "docs/specs/vmgenid.txt"
into a minimal definition block, and tried to compile it with
Microsoft's ASL.exe. (I had done the same with iASL before I posted the
RFC for "vmgenid.txt" -- that's how I had ensured that the ASL would be
*valid* and suitable for "vmgenid.txt" in the first place!)

Consistently with the ACPI.SYS failure (to both execute and disassemble
DataRegionOp), ASL.exe fails to *compile* DataTableRegion()!

> C:\Program Files (x86)\Microsoft ASL Compiler v4.0>asl.exe x.dsl
> Microsoft ACPI Source Language Assembler Version 4.0.0NT [Aug 28 2009, 18:36:36]
>
> Copyright (c) 1996,2009 Microsoft Corporation
> Compliant with the ACPI 4.0 Specification
>
> x.dsl:
>
>    49:                   Offset (54),
>                                    ^***
> x.dsl(49): error: field offset is exceeding operation region range (offset=0x36)
>
>
> C:\Program Files (x86)\Microsoft ASL Compiler v4.0>

(If you search this email for "Offset (54)", you will see where the
above comes from.)

Let me explain: when you define an OperationRegion() in the ASL source,
ASL.exe marks the size of the region. Then in referring Field()
definitions, it checks the end of each individual field against the size
of the region (to ensure, at compile time, that accessing the field at
runtime won't extend beyond the region).

However, the size of the SystemMemory operation region that is defined
with the DataTableRegion() operator is only available at runtime -- the
operator looks up the matching table linked into the RSDT/XSDT, and then
that table *becomes* the operation region. So the size of the region is
the size of the table, which can be looked up at runtime *only*.

ASL.exe apparently considers the size of any such operation region zero.

Consequently, ASL.exe considers *any* field definition for such a region
to be out of bounds -- I tried to move the ADBP field to offset zero,
just for checking the compiler, and it blows up just the same; it
reports that offset 8 (the end of ADBP, when it starts at zero) is out
of range.

Without fields, an operation region is unusable. It doesn't matter that
ASL.exe doesn't actually choke on DataTableRegion(), if you then can't
define any fields for it!

I challange you (the generic you) to find *one* physical computer that
runs Windows *and* has DataTableRegion() in its ACPI tables!

TL;DR:
- Windows cannot execute DataTableRegion(), including modern Windows
  releases,
- ASL.exe cannot (usably) compile DataTableRegion(),
- this is not documented *anywhere* on the web that Google can find, but
  (given DataTableRegion()'s obviously huge utility) it must be silent,
  "secret" knowledge for physical BIOS developers (well, not any longer
  ;)),
- our great idea that centered on DataTableRegion() is busted. :(

Laszlo
Michael S. Tsirkin Sept. 13, 2015, 12:34 p.m. UTC | #8
On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
> As the subject suggests, I have terrible news.
> 
> I'll preserve the full context here, so that it's easy to scroll back to
> the ASL for reference.
> 
> I'm also CC'ing edk2-devel, because a number of BIOS developers should
> be congregating there.

Wow, bravo! It does look like we need to go back to
the drawing board.
The only crazy thing you didn't try is to use
an XSDT instead of the DSDT.
I find it unlikely that this will help ...
Laszlo Ersek Sept. 13, 2015, 12:43 p.m. UTC | #9
So, as I wrote in the parent, this does not actually work in Windows,
because Windows doesn't support the DataTableRegion() operator; not even
modern Windows versions.

I'm nonetheless posting the series for the following purposes:

- Posterity. I think the series is worth preserving in the mailing list
  archive.

- Testing by others, if anyone is so inclined. (Should someone come back
  here later: the series applies to commit
  fc04a730b7e60f4a62d6260d4eb9c537d1d3643f.)

- I think that several patches from the series would be worth merging in
  their own right.

Anatomy of the FYI series:

- Patch 01 is known from the RFC posting; it has seen a number of
  changes. Those are all noted on the patch itself.

- Patch 02 is not related to ACPI, but it was the first one I wrote,
  while trying to attack this series, so I'm including it here anyway.

- Patches 03 to 08 add generic code for, and then introduce, the UEFI
  ACPI Data Table, described in patch 01.

- Patches 09 to 13 add helper functions for, and then generate, the VMGI
  device's AML.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gal Hammer <ghammer@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Shannon Zhao <shannon.zhao@linaro.org>

Thanks
Laszlo

Laszlo Ersek (13):
  docs: describe QEMU's VMGenID design
  hw/acpi: add i386 callbacks for injecting GPE 04 when the VMGENID
    changes
  hw/acpi: rename "AcpiBuildTables.table_data" to "main_blob"
  hw/acpi: allow RSDT entries to be relocated to various fw_cfg blobs
  hw/acpi: add more flexible acpi_add_table() and build_header()
    variants
  hw/acpi: introduce ACPI_BUILD_QEMUPARAM_FILE
  hw/acpi: introduce the AcpiQemuParamTable structure
  hw/i386: build UEFI ACPI Data Table for VMGENID in the dedicated blob
    (WIP)
  hw/acpi: expose more parameters for aml_method()
  hw/acpi: add AML generator function for DataTableRegion()
  hw/acpi: add AML generator function for AccessAs()
  hw/acpi: add AML generator function for CreateQWordField()
  hw/i386: generate AML for the VMGENID device (WIP)

 include/hw/acpi/acpi.h               |   1 +
 include/hw/acpi/acpi_dev_interface.h |   4 +
 include/hw/acpi/aml-build.h          |  23 ++-
 include/hw/acpi/ich9.h               |   1 +
 include/hw/acpi/vmgenid.h            |  72 ++++++++
 hw/acpi/aml-build.c                  | 135 ++++++++++++--
 hw/acpi/ich9.c                       |   8 +
 hw/acpi/piix4.c                      |   8 +
 hw/arm/virt-acpi-build.c             |  11 +-
 hw/i386/acpi-build.c                 | 159 ++++++++++++++++-
 hw/isa/lpc_ich9.c                    |   1 +
 docs/vmgenid.txt                     | 336 +++++++++++++++++++++++++++++++++++
 12 files changed, 738 insertions(+), 21 deletions(-)
 create mode 100644 include/hw/acpi/vmgenid.h
 create mode 100644 docs/vmgenid.txt
Laszlo Ersek Sept. 13, 2015, 12:57 p.m. UTC | #10
On 09/13/15 14:34, Michael S. Tsirkin wrote:
> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
>> As the subject suggests, I have terrible news.
>>
>> I'll preserve the full context here, so that it's easy to scroll back to
>> the ASL for reference.
>>
>> I'm also CC'ing edk2-devel, because a number of BIOS developers should
>> be congregating there.
> 
> Wow, bravo! It does look like we need to go back to
> the drawing board.

Thank you. :)

> The only crazy thing you didn't try is to use
> an XSDT instead of the DSDT.
> I find it unlikely that this will help ...
> 

Actually, I forgot to mention it, but I *did* try to use XSDT, sort of automatically. I had mentioned earlier that EFI_ACPI_TABLE_PROTOCOL automatically links stuff into both RSDT and XSDT, and I verified in this case that the UEFI ACPI Data Table *was* linked into the XSDT.

----*----

[root@ovmf-fedora acpi.6]# dmesg | grep UEFI
[    0.000000] ACPI: UEFI 0x000000003E8F1000 00003E (v01 BOCHS  QEMUPARM 00000001 BXPC 00000001)

----*----

[root@ovmf-fedora acpi.6]# cat xsdt.dsl 
/*
 * Intel ACPI Component Architecture
 * AML/ASL+ Disassembler version 20150515-64
 * Copyright (c) 2000 - 2015 Intel Corporation
 * 
 * Disassembly of xsdt.dat, Sun Sep 13 14:54:17 2015
 *
 * ACPI Data Table [XSDT]
 *
 * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue
 */

[000h 0000   4]                    Signature : "XSDT"    [Extended System Description Table]
[004h 0004   4]                 Table Length : 0000004C
[008h 0008   1]                     Revision : 01
[009h 0009   1]                     Checksum : 90
[00Ah 0010   6]                       Oem ID : "BOCHS "
[010h 0016   8]                 Oem Table ID : "BXPCFACP"
[018h 0024   4]                 Oem Revision : 00000001
[01Ch 0028   4]              Asl Compiler ID : "    "
[020h 0032   4]        Asl Compiler Revision : 01000013

[024h 0036   8]       ACPI Table Address   0 : 000000003FEF5000
[02Ch 0044   8]       ACPI Table Address   1 : 000000003FEF4000
[034h 0052   8]       ACPI Table Address   2 : 000000003FEF3000
[03Ch 0060   8]       ACPI Table Address   3 : 000000003FEF2000
[044h 0068   8]       ACPI Table Address   4 : 000000003E8F1000

Raw Table Data: Length 76 (0x4C)

  0000: 58 53 44 54 4C 00 00 00 01 90 42 4F 43 48 53 20  // XSDTL.....BOCHS 
  0010: 42 58 50 43 46 41 43 50 01 00 00 00 20 20 20 20  // BXPCFACP....    
  0020: 13 00 00 01 00 50 EF 3F 00 00 00 00 00 40 EF 3F  // .....P.?.....@.?
  0030: 00 00 00 00 00 30 EF 3F 00 00 00 00 00 20 EF 3F  // .....0.?..... .?
  0040: 00 00 00 00 00 10 8F 3E 00 00 00 00              // .......>....

----*----

See "ACPI Table Address 4", 3E8F1000.

Thanks!
Laszlo
Igor Mammedov Sept. 14, 2015, 8:24 a.m. UTC | #11
On Sun, 13 Sep 2015 15:34:51 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
> > As the subject suggests, I have terrible news.
> > 
> > I'll preserve the full context here, so that it's easy to scroll back to
> > the ASL for reference.
> > 
> > I'm also CC'ing edk2-devel, because a number of BIOS developers should
> > be congregating there.
> 
> Wow, bravo! It does look like we need to go back to
> the drawing board.
I suggest we go back to the last Gal's series
which is though not universal but a simple and
straightforward solution.
That series with comments addressed probably
is what we need in the end.

> The only crazy thing you didn't try is to use
> an XSDT instead of the DSDT.
> I find it unlikely that this will help ...
>
Laszlo Ersek Sept. 14, 2015, 10:24 a.m. UTC | #12
On 09/14/15 10:24, Igor Mammedov wrote:
> On Sun, 13 Sep 2015 15:34:51 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
>> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
>>> As the subject suggests, I have terrible news.
>>>
>>> I'll preserve the full context here, so that it's easy to scroll back to
>>> the ASL for reference.
>>>
>>> I'm also CC'ing edk2-devel, because a number of BIOS developers should
>>> be congregating there.
>>
>> Wow, bravo! It does look like we need to go back to
>> the drawing board.
> I suggest we go back to the last Gal's series
> which is though not universal but a simple and
> straightforward solution.
> That series with comments addressed probably
> is what we need in the end.

I agree (I commented the same on the RHBZ too). The only one requirement
we might not satisfy with that is that the page containing the
generation ID must always be mapped as cacheable. In practice it doesn't
seem to cause issues.

We tried to play nice, but given that (a) the vmgenid doc doesn't
mention a real requirement about the _CRS -- ie. no IO descriptors are
allowed to be in it --, (b) Windows doesn't support DataTableRegion(), I
doubt we could cover our bases 100% anyway. There can be any number of
further hidden requirements, and hidden gaps in ACPI support too, so
it's just business as usual with Windows: whatever works, works, don't
ask why.

Just my opinion of course.

Laszlo

>> The only crazy thing you didn't try is to use
>> an XSDT instead of the DSDT.
>> I find it unlikely that this will help ...
>>
>
Bill Paul Sept. 14, 2015, 4:53 p.m. UTC | #13
Of all the gin joints in all the towns in all the world, Laszlo Ersek had to 
walk into mine at 03:24:42 on Monday 14 September 2015 and say:

> On 09/14/15 10:24, Igor Mammedov wrote:
> > On Sun, 13 Sep 2015 15:34:51 +0300
> > 
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
> >>> As the subject suggests, I have terrible news.
> >>> 
> >>> I'll preserve the full context here, so that it's easy to scroll back
> >>> to the ASL for reference.
> >>> 
> >>> I'm also CC'ing edk2-devel, because a number of BIOS developers should
> >>> be congregating there.
> >> 
> >> Wow, bravo! It does look like we need to go back to
> >> the drawing board.

I read your original post on this with great interest, and I applaud your 
determination in tracking this down. Nice job. Sadly, it seems you too have 
fallen victim to the "If It Works With Windows, It Must Be Ok" syndrome.

Now, I realize that as far as this particular situation is concerned, even if 
Microsoft decided to add support for DataTableRegion() tomorrow, it wouldn't 
really help because there are too many different versions of Windows in the 
field and there's no way to retroactively patch them all. (Gee, that sounds 
familiar...)

Nevertheless, am I correct in saying that this is in fact a bug in Microsoft's 
ACPI implementation (both in their ASL compiler and in the AML parser)? Unless 
DataTableRegion() is specified to be optional in some way (I don't know if it 
is or not, but I doubt it), this sounds like an clear cut case of non-
compliance with the ACPI spec. And if that's true, isn't there any way to get 
Microsoft to fix it?

-Bill

> > I suggest we go back to the last Gal's series
> > which is though not universal but a simple and
> > straightforward solution.
> > That series with comments addressed probably
> > is what we need in the end.
> 
> I agree (I commented the same on the RHBZ too). The only one requirement
> we might not satisfy with that is that the page containing the
> generation ID must always be mapped as cacheable. In practice it doesn't
> seem to cause issues.
> 
> We tried to play nice, but given that (a) the vmgenid doc doesn't
> mention a real requirement about the _CRS -- ie. no IO descriptors are
> allowed to be in it --, (b) Windows doesn't support DataTableRegion(), I
> doubt we could cover our bases 100% anyway. There can be any number of
> further hidden requirements, and hidden gaps in ACPI support too, so
> it's just business as usual with Windows: whatever works, works, don't
> ask why.
> 
> Just my opinion of course.
> 
> Laszlo
> 
> >> The only crazy thing you didn't try is to use
> >> an XSDT instead of the DSDT.
> >> I find it unlikely that this will help ...
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
Moore, Robert Sept. 14, 2015, 5:14 p.m. UTC | #14
The Windows ACPI implementation doesn't even fully support ACPI 2.0 AFAIK, let alone other new things.

Rob Gough or Michael may know better than I do.


> -----Original Message-----
> From: Bill Paul [mailto:wpaul@windriver.com]
> Sent: Monday, September 14, 2015 9:53 AM
> To: edk2-devel@ml01.01.org
> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; Gal
> Hammer; edk2-devel-01; Moore, Robert; qemu-devel@nongnu.org; Paolo Bonzini
> Subject: Re: [edk2] [Qemu-devel] Windows does not support DataTableRegion
> at all [was: docs: describe QEMU's VMGenID design]
> 
> Of all the gin joints in all the towns in all the world, Laszlo Ersek had
> to walk into mine at 03:24:42 on Monday 14 September 2015 and say:
> 
> > On 09/14/15 10:24, Igor Mammedov wrote:
> > > On Sun, 13 Sep 2015 15:34:51 +0300
> > >
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
> > >>> As the subject suggests, I have terrible news.
> > >>>
> > >>> I'll preserve the full context here, so that it's easy to scroll
> > >>> back to the ASL for reference.
> > >>>
> > >>> I'm also CC'ing edk2-devel, because a number of BIOS developers
> > >>> should be congregating there.
> > >>
> > >> Wow, bravo! It does look like we need to go back to the drawing
> > >> board.
> 
> I read your original post on this with great interest, and I applaud your
> determination in tracking this down. Nice job. Sadly, it seems you too
> have fallen victim to the "If It Works With Windows, It Must Be Ok"
> syndrome.
> 
> Now, I realize that as far as this particular situation is concerned, even
> if Microsoft decided to add support for DataTableRegion() tomorrow, it
> wouldn't really help because there are too many different versions of
> Windows in the field and there's no way to retroactively patch them all.
> (Gee, that sounds
> familiar...)
> 
> Nevertheless, am I correct in saying that this is in fact a bug in
> Microsoft's ACPI implementation (both in their ASL compiler and in the AML
> parser)? Unless
> DataTableRegion() is specified to be optional in some way (I don't know if
> it is or not, but I doubt it), this sounds like an clear cut case of non-
> compliance with the ACPI spec. And if that's true, isn't there any way to
> get Microsoft to fix it?
> 
> -Bill
> 
> > > I suggest we go back to the last Gal's series which is though not
> > > universal but a simple and straightforward solution.
> > > That series with comments addressed probably is what we need in the
> > > end.
> >
> > I agree (I commented the same on the RHBZ too). The only one
> > requirement we might not satisfy with that is that the page containing
> > the generation ID must always be mapped as cacheable. In practice it
> > doesn't seem to cause issues.
> >
> > We tried to play nice, but given that (a) the vmgenid doc doesn't
> > mention a real requirement about the _CRS -- ie. no IO descriptors are
> > allowed to be in it --, (b) Windows doesn't support DataTableRegion(),
> > I doubt we could cover our bases 100% anyway. There can be any number
> > of further hidden requirements, and hidden gaps in ACPI support too,
> > so it's just business as usual with Windows: whatever works, works,
> > don't ask why.
> >
> > Just my opinion of course.
> >
> > Laszlo
> >
> > >> The only crazy thing you didn't try is to use an XSDT instead of
> > >> the DSDT.
> > >> I find it unlikely that this will help ...
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> 
> --
> ==========================================================================
> ===
> -Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
>                  wpaul@windriver.com | Master of Unix-Fu - Wind River
> Systems
> ==========================================================================
> ===
>    "I put a dollar in a change machine. Nothing changed." - George Carlin
> ==========================================================================
> ===
Walz, Michael C Sept. 14, 2015, 5:23 p.m. UTC | #15
Who are the Microsoft representatives in the ASWG? Shouldn't this conversation be happening with them?

-----Original Message-----
From: Moore, Robert 
Sent: September 14, 2015 10:14
To: Bill Paul; edk2-devel@ml01.01.org
Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; Gal Hammer; edk2-devel-01; qemu-devel@nongnu.org; Paolo Bonzini; Gough, Robert; Walz, Michael C
Subject: RE: [edk2] [Qemu-devel] Windows does not support DataTableRegion at all [was: docs: describe QEMU's VMGenID design]

The Windows ACPI implementation doesn't even fully support ACPI 2.0 AFAIK, let alone other new things.

Rob Gough or Michael may know better than I do.


> -----Original Message-----
> From: Bill Paul [mailto:wpaul@windriver.com]
> Sent: Monday, September 14, 2015 9:53 AM
> To: edk2-devel@ml01.01.org
> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; 
> Gal Hammer; edk2-devel-01; Moore, Robert; qemu-devel@nongnu.org; Paolo 
> Bonzini
> Subject: Re: [edk2] [Qemu-devel] Windows does not support 
> DataTableRegion at all [was: docs: describe QEMU's VMGenID design]
> 
> Of all the gin joints in all the towns in all the world, Laszlo Ersek 
> had to walk into mine at 03:24:42 on Monday 14 September 2015 and say:
> 
> > On 09/14/15 10:24, Igor Mammedov wrote:
> > > On Sun, 13 Sep 2015 15:34:51 +0300
> > >
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
> > >>> As the subject suggests, I have terrible news.
> > >>>
> > >>> I'll preserve the full context here, so that it's easy to scroll 
> > >>> back to the ASL for reference.
> > >>>
> > >>> I'm also CC'ing edk2-devel, because a number of BIOS developers 
> > >>> should be congregating there.
> > >>
> > >> Wow, bravo! It does look like we need to go back to the drawing 
> > >> board.
> 
> I read your original post on this with great interest, and I applaud 
> your determination in tracking this down. Nice job. Sadly, it seems 
> you too have fallen victim to the "If It Works With Windows, It Must Be Ok"
> syndrome.
> 
> Now, I realize that as far as this particular situation is concerned, 
> even if Microsoft decided to add support for DataTableRegion() 
> tomorrow, it wouldn't really help because there are too many different 
> versions of Windows in the field and there's no way to retroactively patch them all.
> (Gee, that sounds
> familiar...)
> 
> Nevertheless, am I correct in saying that this is in fact a bug in 
> Microsoft's ACPI implementation (both in their ASL compiler and in the 
> AML parser)? Unless
> DataTableRegion() is specified to be optional in some way (I don't 
> know if it is or not, but I doubt it), this sounds like an clear cut 
> case of non- compliance with the ACPI spec. And if that's true, isn't 
> there any way to get Microsoft to fix it?
> 
> -Bill
> 
> > > I suggest we go back to the last Gal's series which is though not 
> > > universal but a simple and straightforward solution.
> > > That series with comments addressed probably is what we need in 
> > > the end.
> >
> > I agree (I commented the same on the RHBZ too). The only one 
> > requirement we might not satisfy with that is that the page 
> > containing the generation ID must always be mapped as cacheable. In 
> > practice it doesn't seem to cause issues.
> >
> > We tried to play nice, but given that (a) the vmgenid doc doesn't 
> > mention a real requirement about the _CRS -- ie. no IO descriptors 
> > are allowed to be in it --, (b) Windows doesn't support 
> > DataTableRegion(), I doubt we could cover our bases 100% anyway. 
> > There can be any number of further hidden requirements, and hidden 
> > gaps in ACPI support too, so it's just business as usual with 
> > Windows: whatever works, works, don't ask why.
> >
> > Just my opinion of course.
> >
> > Laszlo
> >
> > >> The only crazy thing you didn't try is to use an XSDT instead of 
> > >> the DSDT.
> > >> I find it unlikely that this will help ...
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> 
> --
> ======================================================================
> ====
> ===
> -Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
>                  wpaul@windriver.com | Master of Unix-Fu - Wind River 
> Systems 
> ======================================================================
> ====
> ===
>    "I put a dollar in a change machine. Nothing changed." - George 
> Carlin 
> ======================================================================
> ====
> ===
Moore, Robert Sept. 14, 2015, 6:04 p.m. UTC | #16
I should say "some new things" are not supported, not all of them.


> -----Original Message-----
> From: Walz, Michael C
> Sent: Monday, September 14, 2015 10:24 AM
> To: Moore, Robert; Bill Paul; edk2-devel@ml01.01.org
> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; Gal
> Hammer; edk2-devel-01; qemu-devel@nongnu.org; Paolo Bonzini; Gough, Robert
> Subject: RE: [edk2] [Qemu-devel] Windows does not support DataTableRegion
> at all [was: docs: describe QEMU's VMGenID design]
> 
> Who are the Microsoft representatives in the ASWG? Shouldn't this
> conversation be happening with them?
> 
> -----Original Message-----
> From: Moore, Robert
> Sent: September 14, 2015 10:14
> To: Bill Paul; edk2-devel@ml01.01.org
> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; Gal
> Hammer; edk2-devel-01; qemu-devel@nongnu.org; Paolo Bonzini; Gough,
> Robert; Walz, Michael C
> Subject: RE: [edk2] [Qemu-devel] Windows does not support DataTableRegion
> at all [was: docs: describe QEMU's VMGenID design]
> 
> The Windows ACPI implementation doesn't even fully support ACPI 2.0 AFAIK,
> let alone other new things.
> 
> Rob Gough or Michael may know better than I do.
> 
> 
> > -----Original Message-----
> > From: Bill Paul [mailto:wpaul@windriver.com]
> > Sent: Monday, September 14, 2015 9:53 AM
> > To: edk2-devel@ml01.01.org
> > Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett;
> > Gal Hammer; edk2-devel-01; Moore, Robert; qemu-devel@nongnu.org; Paolo
> > Bonzini
> > Subject: Re: [edk2] [Qemu-devel] Windows does not support
> > DataTableRegion at all [was: docs: describe QEMU's VMGenID design]
> >
> > Of all the gin joints in all the towns in all the world, Laszlo Ersek
> > had to walk into mine at 03:24:42 on Monday 14 September 2015 and say:
> >
> > > On 09/14/15 10:24, Igor Mammedov wrote:
> > > > On Sun, 13 Sep 2015 15:34:51 +0300
> > > >
> > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > >> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
> > > >>> As the subject suggests, I have terrible news.
> > > >>>
> > > >>> I'll preserve the full context here, so that it's easy to scroll
> > > >>> back to the ASL for reference.
> > > >>>
> > > >>> I'm also CC'ing edk2-devel, because a number of BIOS developers
> > > >>> should be congregating there.
> > > >>
> > > >> Wow, bravo! It does look like we need to go back to the drawing
> > > >> board.
> >
> > I read your original post on this with great interest, and I applaud
> > your determination in tracking this down. Nice job. Sadly, it seems
> > you too have fallen victim to the "If It Works With Windows, It Must Be
> Ok"
> > syndrome.
> >
> > Now, I realize that as far as this particular situation is concerned,
> > even if Microsoft decided to add support for DataTableRegion()
> > tomorrow, it wouldn't really help because there are too many different
> > versions of Windows in the field and there's no way to retroactively
> patch them all.
> > (Gee, that sounds
> > familiar...)
> >
> > Nevertheless, am I correct in saying that this is in fact a bug in
> > Microsoft's ACPI implementation (both in their ASL compiler and in the
> > AML parser)? Unless
> > DataTableRegion() is specified to be optional in some way (I don't
> > know if it is or not, but I doubt it), this sounds like an clear cut
> > case of non- compliance with the ACPI spec. And if that's true, isn't
> > there any way to get Microsoft to fix it?
> >
> > -Bill
> >
> > > > I suggest we go back to the last Gal's series which is though not
> > > > universal but a simple and straightforward solution.
> > > > That series with comments addressed probably is what we need in
> > > > the end.
> > >
> > > I agree (I commented the same on the RHBZ too). The only one
> > > requirement we might not satisfy with that is that the page
> > > containing the generation ID must always be mapped as cacheable. In
> > > practice it doesn't seem to cause issues.
> > >
> > > We tried to play nice, but given that (a) the vmgenid doc doesn't
> > > mention a real requirement about the _CRS -- ie. no IO descriptors
> > > are allowed to be in it --, (b) Windows doesn't support
> > > DataTableRegion(), I doubt we could cover our bases 100% anyway.
> > > There can be any number of further hidden requirements, and hidden
> > > gaps in ACPI support too, so it's just business as usual with
> > > Windows: whatever works, works, don't ask why.
> > >
> > > Just my opinion of course.
> > >
> > > Laszlo
> > >
> > > >> The only crazy thing you didn't try is to use an XSDT instead of
> > > >> the DSDT.
> > > >> I find it unlikely that this will help ...
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> >
> > --
> > ======================================================================
> > ====
> > ===
> > -Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
> >                  wpaul@windriver.com | Master of Unix-Fu - Wind River
> > Systems
> > ======================================================================
> > ====
> > ===
> >    "I put a dollar in a change machine. Nothing changed." - George
> > Carlin
> > ======================================================================
> > ====
> > ===
Laszlo Ersek Sept. 14, 2015, 6:20 p.m. UTC | #17
On 09/14/15 18:53, Bill Paul wrote:
> Of all the gin joints in all the towns in all the world, Laszlo Ersek had to 
> walk into mine at 03:24:42 on Monday 14 September 2015 and say:
> 
>> On 09/14/15 10:24, Igor Mammedov wrote:
>>> On Sun, 13 Sep 2015 15:34:51 +0300
>>>
>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
>>>>> As the subject suggests, I have terrible news.
>>>>>
>>>>> I'll preserve the full context here, so that it's easy to scroll back
>>>>> to the ASL for reference.
>>>>>
>>>>> I'm also CC'ing edk2-devel, because a number of BIOS developers should
>>>>> be congregating there.
>>>>
>>>> Wow, bravo! It does look like we need to go back to
>>>> the drawing board.
> 
> I read your original post on this with great interest, and I applaud your 
> determination in tracking this down. Nice job.

Thank you!

> Sadly, it seems you too have 
> fallen victim to the "If It Works With Windows, It Must Be Ok" syndrome.

Well, I'd put it like this: we've fallen victim to a publicly
undocumented feature gap / divergence from the ACPI spec in Windows'
ACPI.SYS.

> Now, I realize that as far as this particular situation is concerned, even if 
> Microsoft decided to add support for DataTableRegion() tomorrow, it wouldn't 
> really help because there are too many different versions of Windows in the 
> field and there's no way to retroactively patch them all. (Gee, that sounds 
> familiar...)

Correct.

> Nevertheless, am I correct in saying that this is in fact a bug in Microsoft's 
> ACPI implementation (both in their ASL compiler and in the AML parser)?

Absolutely. You are absolutely right.

We implemented the VMGENID spec with some undeniable creativity, but it
broke down because the AML interpreter in Windows does not support an
ACPI 2.0 feature.

(That interpreter is supposed to be ACPI 4.0 compliant, minimally; at
least if we can judge it after the "matching" AML.exe's stated
compatibility level, which is ACPI 4.0 in the standalone download, and
5.0 if you get it as part of the WDK.)

> Unless 
> DataTableRegion() is specified to be optional in some way (I don't know if it 
> is or not, but I doubt it),

It's not, to the best of my knowledge.

> this sounds like an clear cut case of non-
> compliance with the ACPI spec.

Yes, it's precisely that.

> And if that's true, isn't there any way to get 
> Microsoft to fix it?

I don't know. Is there?

Microsoft continue to release updates (KB*) for Windows 7, Windows 8,
Windows 10, and I think rolling a fix out for those would cover our
needs quite okay.

But:
- This would force QEMU/KVM host users to upgrade their Windows guest.
  Maybe not such a big hurdle, but I reckon Windows sysadmins are
  really conservative about installing updates. Perhaps we could solve
  this issue but documentation.

- More importantly, how do I even *report* this bug? How do I convince
  Microsoft to implement a whole missing feature in their ACPI compiler
  and interpreter? Can I demonstrate business justification?

  I'm doubtful especially because DataTableRegion's usefulness is quite
  apparent to the ACPI developer in search for parametrization options.
  DataTableRegion was published as part of ACPI 2.0, on July 27, 2000
  (more than fifteen years ago). I simply cannot imagine that in all
  that time *no* physical platform's firmware developer tried to use
  DataTableRegion.

  Therefore I can't help but assume that some big BIOS developer
  company has already reported this to Microsoft, and the feature
  request has been rejected. So what chance would I have?

Thanks
Laszlo

> 
> -Bill
> 
>>> I suggest we go back to the last Gal's series
>>> which is though not universal but a simple and
>>> straightforward solution.
>>> That series with comments addressed probably
>>> is what we need in the end.
>>
>> I agree (I commented the same on the RHBZ too). The only one requirement
>> we might not satisfy with that is that the page containing the
>> generation ID must always be mapped as cacheable. In practice it doesn't
>> seem to cause issues.
>>
>> We tried to play nice, but given that (a) the vmgenid doc doesn't
>> mention a real requirement about the _CRS -- ie. no IO descriptors are
>> allowed to be in it --, (b) Windows doesn't support DataTableRegion(), I
>> doubt we could cover our bases 100% anyway. There can be any number of
>> further hidden requirements, and hidden gaps in ACPI support too, so
>> it's just business as usual with Windows: whatever works, works, don't
>> ask why.
>>
>> Just my opinion of course.
>>
>> Laszlo
>>
>>>> The only crazy thing you didn't try is to use
>>>> an XSDT instead of the DSDT.
>>>> I find it unlikely that this will help ...
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>
Laszlo Ersek Sept. 14, 2015, 6:24 p.m. UTC | #18
On 09/14/15 19:23, Walz, Michael C wrote:
> Who are the Microsoft representatives in the ASWG? Shouldn't this conversation be happening with them?

Red Hat is represented in the ASWG, as far as I know, so if you guys
think this is appropriate to bring up there (ie. a specific product's
compliance), I can try talking to my peers. :)

Thanks
Laszlo

> 
> -----Original Message-----
> From: Moore, Robert 
> Sent: September 14, 2015 10:14
> To: Bill Paul; edk2-devel@ml01.01.org
> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; Gal Hammer; edk2-devel-01; qemu-devel@nongnu.org; Paolo Bonzini; Gough, Robert; Walz, Michael C
> Subject: RE: [edk2] [Qemu-devel] Windows does not support DataTableRegion at all [was: docs: describe QEMU's VMGenID design]
> 
> The Windows ACPI implementation doesn't even fully support ACPI 2.0 AFAIK, let alone other new things.
> 
> Rob Gough or Michael may know better than I do.
> 
> 
>> -----Original Message-----
>> From: Bill Paul [mailto:wpaul@windriver.com]
>> Sent: Monday, September 14, 2015 9:53 AM
>> To: edk2-devel@ml01.01.org
>> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; 
>> Gal Hammer; edk2-devel-01; Moore, Robert; qemu-devel@nongnu.org; Paolo 
>> Bonzini
>> Subject: Re: [edk2] [Qemu-devel] Windows does not support 
>> DataTableRegion at all [was: docs: describe QEMU's VMGenID design]
>>
>> Of all the gin joints in all the towns in all the world, Laszlo Ersek 
>> had to walk into mine at 03:24:42 on Monday 14 September 2015 and say:
>>
>>> On 09/14/15 10:24, Igor Mammedov wrote:
>>>> On Sun, 13 Sep 2015 15:34:51 +0300
>>>>
>>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
>>>>>> As the subject suggests, I have terrible news.
>>>>>>
>>>>>> I'll preserve the full context here, so that it's easy to scroll 
>>>>>> back to the ASL for reference.
>>>>>>
>>>>>> I'm also CC'ing edk2-devel, because a number of BIOS developers 
>>>>>> should be congregating there.
>>>>>
>>>>> Wow, bravo! It does look like we need to go back to the drawing 
>>>>> board.
>>
>> I read your original post on this with great interest, and I applaud 
>> your determination in tracking this down. Nice job. Sadly, it seems 
>> you too have fallen victim to the "If It Works With Windows, It Must Be Ok"
>> syndrome.
>>
>> Now, I realize that as far as this particular situation is concerned, 
>> even if Microsoft decided to add support for DataTableRegion() 
>> tomorrow, it wouldn't really help because there are too many different 
>> versions of Windows in the field and there's no way to retroactively patch them all.
>> (Gee, that sounds
>> familiar...)
>>
>> Nevertheless, am I correct in saying that this is in fact a bug in 
>> Microsoft's ACPI implementation (both in their ASL compiler and in the 
>> AML parser)? Unless
>> DataTableRegion() is specified to be optional in some way (I don't 
>> know if it is or not, but I doubt it), this sounds like an clear cut 
>> case of non- compliance with the ACPI spec. And if that's true, isn't 
>> there any way to get Microsoft to fix it?
>>
>> -Bill
>>
>>>> I suggest we go back to the last Gal's series which is though not 
>>>> universal but a simple and straightforward solution.
>>>> That series with comments addressed probably is what we need in 
>>>> the end.
>>>
>>> I agree (I commented the same on the RHBZ too). The only one 
>>> requirement we might not satisfy with that is that the page 
>>> containing the generation ID must always be mapped as cacheable. In 
>>> practice it doesn't seem to cause issues.
>>>
>>> We tried to play nice, but given that (a) the vmgenid doc doesn't 
>>> mention a real requirement about the _CRS -- ie. no IO descriptors 
>>> are allowed to be in it --, (b) Windows doesn't support 
>>> DataTableRegion(), I doubt we could cover our bases 100% anyway. 
>>> There can be any number of further hidden requirements, and hidden 
>>> gaps in ACPI support too, so it's just business as usual with 
>>> Windows: whatever works, works, don't ask why.
>>>
>>> Just my opinion of course.
>>>
>>> Laszlo
>>>
>>>>> The only crazy thing you didn't try is to use an XSDT instead of 
>>>>> the DSDT.
>>>>> I find it unlikely that this will help ...
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>> --
>> ======================================================================
>> ====
>> ===
>> -Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
>>                  wpaul@windriver.com | Master of Unix-Fu - Wind River 
>> Systems 
>> ======================================================================
>> ====
>> ===
>>    "I put a dollar in a change machine. Nothing changed." - George 
>> Carlin 
>> ======================================================================
>> ====
>> ===
Bill Paul Sept. 14, 2015, 9:12 p.m. UTC | #19
Of all the gin joints in all the towns in all the world, Laszlo Ersek had to 
walk into mine at 11:20:28 on Monday 14 September 2015 and say:

> On 09/14/15 18:53, Bill Paul wrote:
> > Of all the gin joints in all the towns in all the world, Laszlo Ersek had
> > to
> > 
> > walk into mine at 03:24:42 on Monday 14 September 2015 and say:
> >> On 09/14/15 10:24, Igor Mammedov wrote:
> >>> On Sun, 13 Sep 2015 15:34:51 +0300
> >>> 
> >>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
> >>>>> As the subject suggests, I have terrible news.
> >>>>> 
> >>>>> I'll preserve the full context here, so that it's easy to scroll back
> >>>>> to the ASL for reference.
> >>>>> 
> >>>>> I'm also CC'ing edk2-devel, because a number of BIOS developers
> >>>>> should be congregating there.
> >>>> 
> >>>> Wow, bravo! It does look like we need to go back to
> >>>> the drawing board.
> > 
> > I read your original post on this with great interest, and I applaud your
> > determination in tracking this down. Nice job.
> 
> Thank you!
> 
> > Sadly, it seems you too have
> > fallen victim to the "If It Works With Windows, It Must Be Ok" syndrome.
> 
> Well, I'd put it like this: we've fallen victim to a publicly
> undocumented feature gap / divergence from the ACPI spec in Windows'
> ACPI.SYS.
> 
> > Now, I realize that as far as this particular situation is concerned,
> > even if Microsoft decided to add support for DataTableRegion() tomorrow,
> > it wouldn't really help because there are too many different versions of
> > Windows in the field and there's no way to retroactively patch them all.
> > (Gee, that sounds familiar...)
> 
> Correct.
> 
> > Nevertheless, am I correct in saying that this is in fact a bug in
> > Microsoft's ACPI implementation (both in their ASL compiler and in the
> > AML parser)?
> 
> Absolutely. You are absolutely right.
> 
> We implemented the VMGENID spec with some undeniable creativity, but it
> broke down because the AML interpreter in Windows does not support an
> ACPI 2.0 feature.
> 
> (That interpreter is supposed to be ACPI 4.0 compliant, minimally; at
> least if we can judge it after the "matching" AML.exe's stated
> compatibility level, which is ACPI 4.0 in the standalone download, and
> 5.0 if you get it as part of the WDK.)
> 
> > Unless
> > DataTableRegion() is specified to be optional in some way (I don't know
> > if it is or not, but I doubt it),
> 
> It's not, to the best of my knowledge.
> 
> > this sounds like an clear cut case of non-
> > compliance with the ACPI spec.
> 
> Yes, it's precisely that.
> 
> > And if that's true, isn't there any way to get
> > Microsoft to fix it?
> 
> I don't know. Is there?

You would think that someone at Intel would know someone at Microsoft that 
could put some wheels in motion. (All this technology and still we have 
trouble communicating. :P )
 
> Microsoft continue to release updates (KB*) for Windows 7, Windows 8,
> Windows 10, and I think rolling a fix out for those would cover our
> needs quite okay.
> 
> But:
> - This would force QEMU/KVM host users to upgrade their Windows guest.
>   Maybe not such a big hurdle, but I reckon Windows sysadmins are
>   really conservative about installing updates. Perhaps we could solve
>   this issue but documentation.

I agree with you that it's a hassle, but so is patching any other Windows bug. 
And while this particular use of DataTableRegion() affects VMs, it has bearing 
on bare metal installations too.
 
> - More importantly, how do I even *report* this bug? How do I convince
>   Microsoft to implement a whole missing feature in their ACPI compiler
>   and interpreter? Can I demonstrate business justification?
>
>   I'm doubtful especially because DataTableRegion's usefulness is quite
>   apparent to the ACPI developer in search for parametrization options.
>   DataTableRegion was published as part of ACPI 2.0, on July 27, 2000
>   (more than fifteen years ago). I simply cannot imagine that in all
>   that time *no* physical platform's firmware developer tried to use
>   DataTableRegion.
> 
>   Therefore I can't help but assume that some big BIOS developer
>   company has already reported this to Microsoft, and the feature
>   request has been rejected. So what chance would I have?

I understand what you're saying. But, there has to be some way to deal with 
these sorts of things. If everyone thinks and acts that way, then that's 
effectively letting Microsoft control the ACPI standard by "embracing and 
extending" it. (Or in this case I guess it's embracing and constricting it.)

I think a major part of the problem is that the only standard with which the 
big BIOS developers feel they must comply is the Windows Logo program. The x86 
market is still dominated by Windows users, and getting that Windows badge 
onto the machines is still top priority. If there was a UEFI Forum Logo 
program to which they had to adhere instead, things might be different. But 
even if you created one today, as long as the Windows Logo program is still 
around, I doubt the BIOS developers would bother with it, because the majority 
of the end users don't know or care if their PC actually complies with the 
UEFI or ACPI specs. Years of clever marketing have convinced them that If It 
Works With Windows, It Must Be Ok. That's what dictates how they spend their 
money, which is what dictates how much money the hardware vendors make, which 
is what dictates how much money the BIOS developers make, which is what makes 
time travel possible.

Or something like that.

Also, if you want to talk business cases, I'm assuming that somewhere 
Microsoft claims ACPI compliance. If so, then clearly that claim is untrue, 
and I'd be willing to bet this isn't the only place where their implementation 
deviates from the spec either. It's bad for business to claim compliance with 
an industry standard and then very obviously (and maybe even deliberately) not 
comply with it. (If, as you say, this has already been reported and Microsoft 
decided not to fix it, then it's no longer just a good faith mistake.) It's 
even worse to do that while also being a prominent member of the very same 
industry committee responsible for defining the standard in the first place.

In any case, bugs are bugs. You shouldn't need a justification to fix them, 
especially with iron-clad proof of their existence like you have here.

All that aside, I don't know who to report it to either. Maybe this is a good 
time to establish a way to do that, because I doubt this will be the last time 
it will be necessary.

-Bill

> Thanks
> Laszlo
> 
> > -Bill
> > 
> >>> I suggest we go back to the last Gal's series
> >>> which is though not universal but a simple and
> >>> straightforward solution.
> >>> That series with comments addressed probably
> >>> is what we need in the end.
> >> 
> >> I agree (I commented the same on the RHBZ too). The only one requirement
> >> we might not satisfy with that is that the page containing the
> >> generation ID must always be mapped as cacheable. In practice it doesn't
> >> seem to cause issues.
> >> 
> >> We tried to play nice, but given that (a) the vmgenid doc doesn't
> >> mention a real requirement about the _CRS -- ie. no IO descriptors are
> >> allowed to be in it --, (b) Windows doesn't support DataTableRegion(), I
> >> doubt we could cover our bases 100% anyway. There can be any number of
> >> further hidden requirements, and hidden gaps in ACPI support too, so
> >> it's just business as usual with Windows: whatever works, works, don't
> >> ask why.
> >> 
> >> Just my opinion of course.
> >> 
> >> Laszlo
> >> 
> >>>> The only crazy thing you didn't try is to use
> >>>> an XSDT instead of the DSDT.
> >>>> I find it unlikely that this will help ...
> >> 
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Sept. 15, 2015, 10:49 a.m. UTC | #20
On 09/14/15 23:12, Bill Paul wrote:

[snip -- see my other email too]

> Also, if you want to talk business cases, I'm assuming that somewhere 
> Microsoft claims ACPI compliance.

Correct; this page:

<https://msdn.microsoft.com/en-us/library/windows/hardware/dn551195%28v=vs.85%29.aspx>

states

    Version 5.0 of the Microsoft ACPI source language (ASL) compiler
    supports the features in the Advanced Configuration and Power
    Interface Specification, Revision 5.0 [...]

And the separately downloadable ASL.exe that I tried starts with a banner that claims ACPI 4.0 compliance:

> C:\Program Files (x86)\Microsoft ASL Compiler v4.0>asl.exe x.dsl
> Microsoft ACPI Source Language Assembler Version 4.0.0NT [Aug 28 2009, 18:36:36]
> 
> Copyright (c) 1996,2009 Microsoft Corporation
> Compliant with the ACPI 4.0 Specification
> 
> [...]

The DataTableRegion() operator is from ACPI 2.0.

In ACPI 6.0 (the most recent release), it is still there.

(And, logically, if you can compile DataTableRegion() from ASL to AML (-> DefDataRegion), then your AML interpreter should also be able to parse and execute the binary DefDataRegion encoding codified by the standard.)

> If so, then clearly that claim is untrue,

Let's say, "not precise".

I think such gaps in support for various industry standards are not uncommon, but I believe they should be publicly documented. Using Google, I couldn't find a trace of this limitation. If there had been public documentation about this (or maybe a public bug tracker, or just a tech support forum post...), it would have saved us many many hours, at minimum.

(At this point though, the best for us would be if Microsoft decided to implement DataTableRegion() in ACPI.SYS, and push it out as KBxxxxx.)
 
> and I'd be willing to bet this isn't the only place where their implementation 
> deviates from the spec either.

I assume that's likely, yes.

> It's bad for business to claim compliance with 
> an industry standard and then very obviously (and maybe even deliberately) not 
> comply with it.

The inaccurate claim about compliance is certainly confusing.

Establishing the non-compliance (from the oustide anyway) was anything but obvious. But, now that we've seen the evidence, it's quite factual.

> (If, as you say, this has already been reported and Microsoft 
> decided not to fix it, then it's no longer just a good faith mistake.)

I didn't try to state that as a fact, I just opined that in the 15 years since the release of the ACPI 2.0 revision, someone must surely have tried DataTableRegion(). Assuming that programmer worked for a big BIOS company (which I think is a safe assumption -- before virtualization has become commonplace, who else looked into *writing* ACPI tables?), it seems to follow that a bug would be reported in some way.

> It's 
> even worse to do that while also being a prominent member of the very same 
> industry committee responsible for defining the standard in the first place.

Right, it's strange.

> In any case, bugs are bugs. You shouldn't need a justification to fix them, 
> especially with iron-clad proof of their existence like you have here.

Bugfixing has costs. That's what I'm worried about a little bit. I don't know what to put in the other side of the balance. "Improving compliance" should have marketing value, minimally. Then, "helping QEMU developers implement Microsoft's VMGENID spec, thereby improving Windows guest utility on QEMU" should ultimately translate to wider Windows guest adoption.

> All that aside, I don't know who to report it to either. Maybe this is a good 
> time to establish a way to do that, because I doubt this will be the last time 
> it will be necessary.

I'm hopeful about the ASWG connection.

Thanks!
Laszlo

> -Bill
> 
>> Thanks
>> Laszlo
>>
>>> -Bill
>>>
>>>>> I suggest we go back to the last Gal's series
>>>>> which is though not universal but a simple and
>>>>> straightforward solution.
>>>>> That series with comments addressed probably
>>>>> is what we need in the end.
>>>>
>>>> I agree (I commented the same on the RHBZ too). The only one requirement
>>>> we might not satisfy with that is that the page containing the
>>>> generation ID must always be mapped as cacheable. In practice it doesn't
>>>> seem to cause issues.
>>>>
>>>> We tried to play nice, but given that (a) the vmgenid doc doesn't
>>>> mention a real requirement about the _CRS -- ie. no IO descriptors are
>>>> allowed to be in it --, (b) Windows doesn't support DataTableRegion(), I
>>>> doubt we could cover our bases 100% anyway. There can be any number of
>>>> further hidden requirements, and hidden gaps in ACPI support too, so
>>>> it's just business as usual with Windows: whatever works, works, don't
>>>> ask why.
>>>>
>>>> Just my opinion of course.
>>>>
>>>> Laszlo
>>>>
>>>>>> The only crazy thing you didn't try is to use
>>>>>> an XSDT instead of the DSDT.
>>>>>> I find it unlikely that this will help ...
>>>>
>>>> _______________________________________________
>>>> edk2-devel mailing list
>>>> edk2-devel@lists.01.org
>>>> https://lists.01.org/mailman/listinfo/edk2-devel
>
Laszlo Ersek Sept. 15, 2015, 10:49 a.m. UTC | #21
On 09/14/15 19:23, Walz, Michael C wrote:
> Who are the Microsoft representatives in the ASWG? Shouldn't this conversation be happening with them?

I asked my colleague Al Stone @RH for help; he said he'd bring this up
at the next ASWG meeting. Thank you Al, and thank you Michael for the
suggestion.

Thanks!
Laszlo

> 
> -----Original Message-----
> From: Moore, Robert 
> Sent: September 14, 2015 10:14
> To: Bill Paul; edk2-devel@ml01.01.org
> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; Gal Hammer; edk2-devel-01; qemu-devel@nongnu.org; Paolo Bonzini; Gough, Robert; Walz, Michael C
> Subject: RE: [edk2] [Qemu-devel] Windows does not support DataTableRegion at all [was: docs: describe QEMU's VMGenID design]
> 
> The Windows ACPI implementation doesn't even fully support ACPI 2.0 AFAIK, let alone other new things.
> 
> Rob Gough or Michael may know better than I do.
> 
> 
>> -----Original Message-----
>> From: Bill Paul [mailto:wpaul@windriver.com]
>> Sent: Monday, September 14, 2015 9:53 AM
>> To: edk2-devel@ml01.01.org
>> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; 
>> Gal Hammer; edk2-devel-01; Moore, Robert; qemu-devel@nongnu.org; Paolo 
>> Bonzini
>> Subject: Re: [edk2] [Qemu-devel] Windows does not support 
>> DataTableRegion at all [was: docs: describe QEMU's VMGenID design]
>>
>> Of all the gin joints in all the towns in all the world, Laszlo Ersek 
>> had to walk into mine at 03:24:42 on Monday 14 September 2015 and say:
>>
>>> On 09/14/15 10:24, Igor Mammedov wrote:
>>>> On Sun, 13 Sep 2015 15:34:51 +0300
>>>>
>>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
>>>>>> As the subject suggests, I have terrible news.
>>>>>>
>>>>>> I'll preserve the full context here, so that it's easy to scroll 
>>>>>> back to the ASL for reference.
>>>>>>
>>>>>> I'm also CC'ing edk2-devel, because a number of BIOS developers 
>>>>>> should be congregating there.
>>>>>
>>>>> Wow, bravo! It does look like we need to go back to the drawing 
>>>>> board.
>>
>> I read your original post on this with great interest, and I applaud 
>> your determination in tracking this down. Nice job. Sadly, it seems 
>> you too have fallen victim to the "If It Works With Windows, It Must Be Ok"
>> syndrome.
>>
>> Now, I realize that as far as this particular situation is concerned, 
>> even if Microsoft decided to add support for DataTableRegion() 
>> tomorrow, it wouldn't really help because there are too many different 
>> versions of Windows in the field and there's no way to retroactively patch them all.
>> (Gee, that sounds
>> familiar...)
>>
>> Nevertheless, am I correct in saying that this is in fact a bug in 
>> Microsoft's ACPI implementation (both in their ASL compiler and in the 
>> AML parser)? Unless
>> DataTableRegion() is specified to be optional in some way (I don't 
>> know if it is or not, but I doubt it), this sounds like an clear cut 
>> case of non- compliance with the ACPI spec. And if that's true, isn't 
>> there any way to get Microsoft to fix it?
>>
>> -Bill
>>
>>>> I suggest we go back to the last Gal's series which is though not 
>>>> universal but a simple and straightforward solution.
>>>> That series with comments addressed probably is what we need in 
>>>> the end.
>>>
>>> I agree (I commented the same on the RHBZ too). The only one 
>>> requirement we might not satisfy with that is that the page 
>>> containing the generation ID must always be mapped as cacheable. In 
>>> practice it doesn't seem to cause issues.
>>>
>>> We tried to play nice, but given that (a) the vmgenid doc doesn't 
>>> mention a real requirement about the _CRS -- ie. no IO descriptors 
>>> are allowed to be in it --, (b) Windows doesn't support 
>>> DataTableRegion(), I doubt we could cover our bases 100% anyway. 
>>> There can be any number of further hidden requirements, and hidden 
>>> gaps in ACPI support too, so it's just business as usual with 
>>> Windows: whatever works, works, don't ask why.
>>>
>>> Just my opinion of course.
>>>
>>> Laszlo
>>>
>>>>> The only crazy thing you didn't try is to use an XSDT instead of 
>>>>> the DSDT.
>>>>> I find it unlikely that this will help ...
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>> --
>> ======================================================================
>> ====
>> ===
>> -Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
>>                  wpaul@windriver.com | Master of Unix-Fu - Wind River 
>> Systems 
>> ======================================================================
>> ====
>> ===
>>    "I put a dollar in a change machine. Nothing changed." - George 
>> Carlin 
>> ======================================================================
>> ====
>> ===
Moore, Robert Sept. 15, 2015, 1:45 p.m. UTC | #22
I can't speak to the MS interpreter, but the iASL compiler does indeed support DataTableRegion (as well as the ACPICA interpreter). It may be worth an experiment to build an AML table with the iASL compiler that contains a DataTableRegion, and try it out on Win.

Also, newer versions of the MS ASL compiler (at least 5.0), are a bit harder to obtain. It appears to now be part of the WDK:

"The ASL compiler is distributed with the Windows Driver Kit (WDK) 8.1. Look for the Asl.exe executable file in the Tools\arm\ACPIVerify, Tools\x86\ACPIVerify, or Tools\x64\ACPIVerify directory of your installed WDK."

However, I would suggest that you use the iASL compiler, it is actively maintained and has enhanced error detection. It is available at:

https://www.acpica.org/downloads/binary-tools

Bob



> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, September 15, 2015 3:49 AM
> To: Bill Paul
> Cc: edk2-devel@ml01.01.org; Igor Mammedov; Michael S. Tsirkin; Josh
> Triplett; Gal Hammer; Moore, Robert; qemu-devel@nongnu.org; Paolo Bonzini
> Subject: Re: [edk2] [Qemu-devel] Windows does not support DataTableRegion
> at all [was: docs: describe QEMU's VMGenID design]
> 
> On 09/14/15 23:12, Bill Paul wrote:
> 
> [snip -- see my other email too]
> 
> > Also, if you want to talk business cases, I'm assuming that somewhere
> > Microsoft claims ACPI compliance.
> 
> Correct; this page:
> 
> <https://msdn.microsoft.com/en-
> us/library/windows/hardware/dn551195%28v=vs.85%29.aspx>
> 
> states
> 
>     Version 5.0 of the Microsoft ACPI source language (ASL) compiler
>     supports the features in the Advanced Configuration and Power
>     Interface Specification, Revision 5.0 [...]
> 
> And the separately downloadable ASL.exe that I tried starts with a banner
> that claims ACPI 4.0 compliance:
> 
> > C:\Program Files (x86)\Microsoft ASL Compiler v4.0>asl.exe x.dsl
> > Microsoft ACPI Source Language Assembler Version 4.0.0NT [Aug 28 2009,
> > 18:36:36]
> >
> > Copyright (c) 1996,2009 Microsoft Corporation Compliant with the ACPI
> > 4.0 Specification
> >
> > [...]
> 
> The DataTableRegion() operator is from ACPI 2.0.
> 
> In ACPI 6.0 (the most recent release), it is still there.
> 
> (And, logically, if you can compile DataTableRegion() from ASL to AML (->
> DefDataRegion), then your AML interpreter should also be able to parse and
> execute the binary DefDataRegion encoding codified by the standard.)
> 
> > If so, then clearly that claim is untrue,
> 
> Let's say, "not precise".
> 
> I think such gaps in support for various industry standards are not
> uncommon, but I believe they should be publicly documented. Using Google,
> I couldn't find a trace of this limitation. If there had been public
> documentation about this (or maybe a public bug tracker, or just a tech
> support forum post...), it would have saved us many many hours, at
> minimum.
> 
> (At this point though, the best for us would be if Microsoft decided to
> implement DataTableRegion() in ACPI.SYS, and push it out as KBxxxxx.)
> 
> > and I'd be willing to bet this isn't the only place where their
> > implementation deviates from the spec either.
> 
> I assume that's likely, yes.
> 
> > It's bad for business to claim compliance with an industry standard
> > and then very obviously (and maybe even deliberately) not comply with
> > it.
> 
> The inaccurate claim about compliance is certainly confusing.
> 
> Establishing the non-compliance (from the oustide anyway) was anything but
> obvious. But, now that we've seen the evidence, it's quite factual.
> 
> > (If, as you say, this has already been reported and Microsoft decided
> > not to fix it, then it's no longer just a good faith mistake.)
> 
> I didn't try to state that as a fact, I just opined that in the 15 years
> since the release of the ACPI 2.0 revision, someone must surely have tried
> DataTableRegion(). Assuming that programmer worked for a big BIOS company
> (which I think is a safe assumption -- before virtualization has become
> commonplace, who else looked into *writing* ACPI tables?), it seems to
> follow that a bug would be reported in some way.
> 
> > It's
> > even worse to do that while also being a prominent member of the very
> > same industry committee responsible for defining the standard in the
> first place.
> 
> Right, it's strange.
> 
> > In any case, bugs are bugs. You shouldn't need a justification to fix
> > them, especially with iron-clad proof of their existence like you have
> here.
> 
> Bugfixing has costs. That's what I'm worried about a little bit. I don't
> know what to put in the other side of the balance. "Improving compliance"
> should have marketing value, minimally. Then, "helping QEMU developers
> implement Microsoft's VMGENID spec, thereby improving Windows guest
> utility on QEMU" should ultimately translate to wider Windows guest
> adoption.
> 
> > All that aside, I don't know who to report it to either. Maybe this is
> > a good time to establish a way to do that, because I doubt this will
> > be the last time it will be necessary.
> 
> I'm hopeful about the ASWG connection.
> 
> Thanks!
> Laszlo
> 
> > -Bill
> >
> >> Thanks
> >> Laszlo
> >>
> >>> -Bill
> >>>
> >>>>> I suggest we go back to the last Gal's series which is though not
> >>>>> universal but a simple and straightforward solution.
> >>>>> That series with comments addressed probably is what we need in
> >>>>> the end.
> >>>>
> >>>> I agree (I commented the same on the RHBZ too). The only one
> >>>> requirement we might not satisfy with that is that the page
> >>>> containing the generation ID must always be mapped as cacheable. In
> >>>> practice it doesn't seem to cause issues.
> >>>>
> >>>> We tried to play nice, but given that (a) the vmgenid doc doesn't
> >>>> mention a real requirement about the _CRS -- ie. no IO descriptors
> >>>> are allowed to be in it --, (b) Windows doesn't support
> >>>> DataTableRegion(), I doubt we could cover our bases 100% anyway.
> >>>> There can be any number of further hidden requirements, and hidden
> >>>> gaps in ACPI support too, so it's just business as usual with
> >>>> Windows: whatever works, works, don't ask why.
> >>>>
> >>>> Just my opinion of course.
> >>>>
> >>>> Laszlo
> >>>>
> >>>>>> The only crazy thing you didn't try is to use an XSDT instead of
> >>>>>> the DSDT.
> >>>>>> I find it unlikely that this will help ...
> >>>>
> >>>> _______________________________________________
> >>>> edk2-devel mailing list
> >>>> edk2-devel@lists.01.org
> >>>> https://lists.01.org/mailman/listinfo/edk2-devel
> >
Laszlo Ersek Sept. 15, 2015, 2:29 p.m. UTC | #23
On 09/15/15 15:45, Moore, Robert wrote:
> I can't speak to the MS interpreter, but the iASL compiler does
> indeed support DataTableRegion

Yes, that was the very first thing I tested. That was a prerequisite for
writing the design doc (which contained ASL code).

> (as well as the ACPICA interpreter).

Right, our code worked right there.

> It may be worth an experiment to build an AML table with the iASL
> compiler that contains a DataTableRegion, and try it out on Win.

That wouldn't add much info now.

QEMU generates binary AML in C source code, to be exposed by the guest
firmware to the guest OS. Beyond the fact that ACPICA's AML interpreter
executes that AML correctly, I also decompiled the same AML with
"acpidump -b" + "iasl -d" within the guest, and verified the decompiled
ASL. It looks 100% fine, and as expected.

The issue is *really* that ACPI.SYS cannot distinguish

  <ExtOpPrefix 0x88>

which is DataRegionOp, from

  <0x88>

which is IndexOp.

In turn this bug causes ACPI.SYS to mis-interpret DefDataRegion (which
starts with DataRegionOp) as DefIndex (which starts with IndexOp).

Whether this is the consequence of a "simple" AML parsing error in
ACPI.SYS, or the complete lack of DataTableRegion support, I can't tell.

> Also, newer versions of the MS ASL compiler (at least 5.0), are a bit
> harder to obtain. It appears to now be part of the WDK:
> 
> "The ASL compiler is distributed with the Windows Driver Kit (WDK)
> 8.1. Look for the Asl.exe executable file in the
> Tools\arm\ACPIVerify, Tools\x86\ACPIVerify, or Tools\x64\ACPIVerify
> directory of your installed WDK."

Yeah, I tried that in a Windows VM, but when I saw that the WDK
installer wanted to download about 10 to 20 GB of stuff, I looked after
other possibilities. (And found the standalone 4.0 compiler.)

> However, I would suggest that you use the iASL compiler, it is
> actively maintained and has enhanced error detection. It is available
> at:
> 
> https://www.acpica.org/downloads/binary-tools

Whenever we compile ASL to AML (as opposed to dynamically generating AML
in our own C code, with Igor's AML generator API), we use iasl exclusively.

The only reason I checked the Microsoft ASL.exe compiler was to see if
that tool was *bug-consistent* with ACPI.SYS. And it was; Windows tools
can neither compile, nor execute, nor decompile (in WinDbg) DataRegionOp.

Thanks
Laszlo

> 
> Bob
> 
> 
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Tuesday, September 15, 2015 3:49 AM
>> To: Bill Paul
>> Cc: edk2-devel@ml01.01.org; Igor Mammedov; Michael S. Tsirkin; Josh
>> Triplett; Gal Hammer; Moore, Robert; qemu-devel@nongnu.org; Paolo Bonzini
>> Subject: Re: [edk2] [Qemu-devel] Windows does not support DataTableRegion
>> at all [was: docs: describe QEMU's VMGenID design]
>>
>> On 09/14/15 23:12, Bill Paul wrote:
>>
>> [snip -- see my other email too]
>>
>>> Also, if you want to talk business cases, I'm assuming that somewhere
>>> Microsoft claims ACPI compliance.
>>
>> Correct; this page:
>>
>> <https://msdn.microsoft.com/en-
>> us/library/windows/hardware/dn551195%28v=vs.85%29.aspx>
>>
>> states
>>
>>     Version 5.0 of the Microsoft ACPI source language (ASL) compiler
>>     supports the features in the Advanced Configuration and Power
>>     Interface Specification, Revision 5.0 [...]
>>
>> And the separately downloadable ASL.exe that I tried starts with a banner
>> that claims ACPI 4.0 compliance:
>>
>>> C:\Program Files (x86)\Microsoft ASL Compiler v4.0>asl.exe x.dsl
>>> Microsoft ACPI Source Language Assembler Version 4.0.0NT [Aug 28 2009,
>>> 18:36:36]
>>>
>>> Copyright (c) 1996,2009 Microsoft Corporation Compliant with the ACPI
>>> 4.0 Specification
>>>
>>> [...]
>>
>> The DataTableRegion() operator is from ACPI 2.0.
>>
>> In ACPI 6.0 (the most recent release), it is still there.
>>
>> (And, logically, if you can compile DataTableRegion() from ASL to AML (->
>> DefDataRegion), then your AML interpreter should also be able to parse and
>> execute the binary DefDataRegion encoding codified by the standard.)
>>
>>> If so, then clearly that claim is untrue,
>>
>> Let's say, "not precise".
>>
>> I think such gaps in support for various industry standards are not
>> uncommon, but I believe they should be publicly documented. Using Google,
>> I couldn't find a trace of this limitation. If there had been public
>> documentation about this (or maybe a public bug tracker, or just a tech
>> support forum post...), it would have saved us many many hours, at
>> minimum.
>>
>> (At this point though, the best for us would be if Microsoft decided to
>> implement DataTableRegion() in ACPI.SYS, and push it out as KBxxxxx.)
>>
>>> and I'd be willing to bet this isn't the only place where their
>>> implementation deviates from the spec either.
>>
>> I assume that's likely, yes.
>>
>>> It's bad for business to claim compliance with an industry standard
>>> and then very obviously (and maybe even deliberately) not comply with
>>> it.
>>
>> The inaccurate claim about compliance is certainly confusing.
>>
>> Establishing the non-compliance (from the oustide anyway) was anything but
>> obvious. But, now that we've seen the evidence, it's quite factual.
>>
>>> (If, as you say, this has already been reported and Microsoft decided
>>> not to fix it, then it's no longer just a good faith mistake.)
>>
>> I didn't try to state that as a fact, I just opined that in the 15 years
>> since the release of the ACPI 2.0 revision, someone must surely have tried
>> DataTableRegion(). Assuming that programmer worked for a big BIOS company
>> (which I think is a safe assumption -- before virtualization has become
>> commonplace, who else looked into *writing* ACPI tables?), it seems to
>> follow that a bug would be reported in some way.
>>
>>> It's
>>> even worse to do that while also being a prominent member of the very
>>> same industry committee responsible for defining the standard in the
>> first place.
>>
>> Right, it's strange.
>>
>>> In any case, bugs are bugs. You shouldn't need a justification to fix
>>> them, especially with iron-clad proof of their existence like you have
>> here.
>>
>> Bugfixing has costs. That's what I'm worried about a little bit. I don't
>> know what to put in the other side of the balance. "Improving compliance"
>> should have marketing value, minimally. Then, "helping QEMU developers
>> implement Microsoft's VMGENID spec, thereby improving Windows guest
>> utility on QEMU" should ultimately translate to wider Windows guest
>> adoption.
>>
>>> All that aside, I don't know who to report it to either. Maybe this is
>>> a good time to establish a way to do that, because I doubt this will
>>> be the last time it will be necessary.
>>
>> I'm hopeful about the ASWG connection.
>>
>> Thanks!
>> Laszlo
>>
>>> -Bill
>>>
>>>> Thanks
>>>> Laszlo
>>>>
>>>>> -Bill
>>>>>
>>>>>>> I suggest we go back to the last Gal's series which is though not
>>>>>>> universal but a simple and straightforward solution.
>>>>>>> That series with comments addressed probably is what we need in
>>>>>>> the end.
>>>>>>
>>>>>> I agree (I commented the same on the RHBZ too). The only one
>>>>>> requirement we might not satisfy with that is that the page
>>>>>> containing the generation ID must always be mapped as cacheable. In
>>>>>> practice it doesn't seem to cause issues.
>>>>>>
>>>>>> We tried to play nice, but given that (a) the vmgenid doc doesn't
>>>>>> mention a real requirement about the _CRS -- ie. no IO descriptors
>>>>>> are allowed to be in it --, (b) Windows doesn't support
>>>>>> DataTableRegion(), I doubt we could cover our bases 100% anyway.
>>>>>> There can be any number of further hidden requirements, and hidden
>>>>>> gaps in ACPI support too, so it's just business as usual with
>>>>>> Windows: whatever works, works, don't ask why.
>>>>>>
>>>>>> Just my opinion of course.
>>>>>>
>>>>>> Laszlo
>>>>>>
>>>>>>>> The only crazy thing you didn't try is to use an XSDT instead of
>>>>>>>> the DSDT.
>>>>>>>> I find it unlikely that this will help ...
>>>>>>
>>>>>> _______________________________________________
>>>>>> edk2-devel mailing list
>>>>>> edk2-devel@lists.01.org
>>>>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>>
>
diff mbox

Patch

diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
new file mode 100644
index 0000000..d4bf132
--- /dev/null
+++ b/docs/specs/vmgenid.txt
@@ -0,0 +1,343 @@ 
+Virtual Machine Generation ID Device
+====================================
+
+The Microsoft specification entitled "Virtual Machine Generation ID",
+maintained at <http://go.microsoft.com/fwlink/?LinkId=260709>, defines an ACPI
+feature that allows the guest OSPM to recognize when it has been returned "to
+an earlier point in time", eg. by restoral from snapshot, or by incoming
+migration. Quoting the spec,
+
+    The virtual machine generation ID is a feature whereby the virtual machines
+    BIOS will expose a new ID. This is a 128-bit, cryptographically random
+    integer value identifier that will be different every time the virtual
+    machine executes from a different configuration file-such as executing from
+    a recovered snapshot, or executing after restoring from backup. [...]
+
+The document you are reading now extracts the requirements set forth by the
+VMGenID spec for hypervisors that intend to provide the feature, and describes
+QEMU's implementation. The design below targets both SeaBIOS and OVMF as
+compatible guest firmwares, without any changes to either of them.
+
+Requirements
+------------
+
+These requirements are extracted from the "How to implement virtual machine
+generation ID support in a virtualization platform" section of the
+specification, dated August 1, 2012.
+
+R1a. The generation ID shall live in an 8-byte aligned buffer.
+
+R1b. The buffer holding the generation ID shall be in guest RAM, ROM, or device
+     MMIO range.
+
+R1c. The buffer holding the generation ID shall be kept separate from areas
+     used by the operating system.
+
+R1d. The buffer shall not be covered by an AddressRangeMemory or
+     AddressRangeACPI entry in the E820 or UEFI memory map.
+
+R1e. The generation ID shall not live in a page frame that could be mapped with
+     caching disabled. (In other words, if the generation ID lives in RAM, then
+     it shall only be mapped as cacheable.)
+
+R2 to R5. [These AML requirements are isolated well enough in the Microsoft
+          specification for us to simply refer to them here.]
+
+R6. The hypervisor shall expose a _HID (hardware identifier) object in the
+    VMGenId device's scope that is unique to the hypervisor vendor.
+
+Generation ID buffer design
+---------------------------
+
+QEMU places the generation ID buffer inside a separate fw_cfg blob that is
+exposed to the guest OS with the ACPI linker/loader.
+
+The structure of the blob is as follows. Offsets, sizes and numeric values are
+given in decimal; furthermore the latter are encoded in little endian.
+
+  Offs  Field               Size  Value
+  ----  ------------------  ----  ------------------------------------
+     0  System Description    36
+        Table Header
+     0    Signature            4                                "UEFI"
+     4    Length               4                                    62
+     8    Revision             1                                     1
+     9    Checksum             1                                     0
+    10    OEMID                6        ACPI_BUILD_APPNAME6 ("BOCHS ")
+    16    OEM Table ID         8                            "QEMUPARM"
+    24    OEM Revision         4                                     1
+    28    Creator ID           4          ACPI_BUILD_APPNAME4 ("BXPC")
+    32    Creator Revision     4                                     1
+
+    36  UEFI Table            18
+        Sub-Header
+    36    Identifier          16  417a5dff-bf4b-4abc-a839-6593bb41f452
+    52    DataOffset           2                                    54
+
+    54  ADDR base pointer      8                                    62
+  ....................................................................
+    62  OVMF SDT Header       36                                zeroes
+        probe suppressor
+    98  VMGenID alignment      6                                zeroes
+        padding
+   104  generation ID         16                       128-bit VMGenID
+   120  fw_cfg blob         3976                                zeroes
+        padding
+  4096  <end of blob>
+
+The fw_cfg blob is divided in two parts conceptually (separated by the dotted
+line in the diagram). The first part, up to and excluding offset 62, is a
+"UEFI" ACPI Table, governed by the UEFI specification 2.5, Appendix O. The
+second part is mainly padding, but it also contains the generation ID.
+
+The "UEFI" ACPI Table -- in the first part -- is a "normal" ACPI table whose
+generic header is defined by the ACPI specification, but for which the UEFI
+spec defines the "UEFI" signature and adds two more fixed fields, "Identifier"
+and "DataOffset".
+
+- The Identifier field carries a 128-bit GUID, and enables firmware
+  implementors to install several "UEFI" tables with different internal
+  structures, enabling OSPM to tell them apart based on the (Type-)Identifier
+  GUID field.
+
+  For the purposes of QEMU's VMGenID implementation, we generated a new GUID
+  with the "uuidgen" utility. It should be different from all other
+  "Identifier" values, present and future, but otherwise no other software need
+  be aware of the concrete GUID value we generated.
+
+- The DataOffset field is just an offset into the table where the actual
+  (Identifier-specific) data starts.
+
+  For the purposes of QEMU's VMGenID implementation, we simply set it to the
+  next (QEMU-specific) field, "ADDR base pointer".
+
+Linker/loader commands
+----------------------
+
+The name of the fw_cfg blob is "etc/acpi/qemuparam". The ALLOCATE command that
+instructs the guest firmware to download this fw_cfg blob specifies an
+alignment of 4096, and the blob will have size 4096 too.
+
+An ADD_POINTER command links the "UEFI" ACPI Table at the start of the blob
+into the RSDT.
+
+Another ADD_POINTER command relocates the "ADDR base pointer" field to the
+absolute address of the "OVMF SDT Header probe suppressor" field, within the
+same blob.
+
+After this relocation, an ADD_CHECKSUM command updates the Checksum field,
+covering the entire "UEFI" ACPI Table (which extends up to and excluding offset
+62).
+
+Blob behavior under SeaBIOS
+---------------------------
+
+(Most of the complexity in the blob is ignored when the guest firmware is
+SeaBIOS.)
+
+- SeaBIOS's ACPI linker/loader client allocates the blob in normal RAM
+  (satisfying R1b).
+
+- Because the ALLOCATE command prescribes an alignment of 4KB, and the blob's
+  size is also 4KB, the allocation covers a standalone page frame in full
+  (satisfying R1e).
+
+- The 128-bit VMGenID field is located at offset 104 within that page,
+  resulting in a guest-physical address divisible by 8 (satisfying R1a).
+
+- The blob is marked as Reserved in the E820 map (satisfying R1c and R1d).
+
+- The "UEFI" ACPI Table at the start of the blob is linked into the RSDT,
+  in-place.
+
+- The "ADDR" AML method (see later) is allowed to refer to the "UEFI" ACPI
+  Table with the DataTableRegion operator, because the table is located in
+  memory marked as AddressRangeReserved.
+
+- The "ADDR base pointer" field points at "OVMF SDT Header probe suppressor",
+  which is right after the "UEFI" ACPI Table inside the blob. At OSPM runtime,
+  the "ADDR" AML method reads the "ADDR base pointer" field, and adds 42, to
+  arrive at the address of the VMGenID field.
+
+  blob @ page offset 0              RSDT
+  +-----------------------+         +-----+
+  | "UEFI" ACPI Table <---------+   | ... |
+  | +-------------------+ |     |   | ... |
+  | | ...               | |     +---- ... |
+  | | ...               | |         +-----+
+  | | ADDR base pointer -----+
+  | +-------------------+ |  |
+  | probe suppressor <-------+
+  | VMGenID @ offset 104  |
+  | padding               |
+  +-----------------------+
+
+Blob behavior under OVMF
+------------------------
+
+The complexity in the blob is required by the two-pass nature of OVMF's ACPI
+linker/loader client, which in turn comes from the fact that OVMF has to
+dissect blobs into individual ACPI tables vs. "other things", tracking the
+ADD_POINTER commands, so that tables can be installed individually, with
+EFI_ACPI_TABLE_PROTOCOL.
+
+- OVMF's ACPI linker/loader client allocates the blob in normal RAM (satisfying
+  R1b).
+
+- Because the ALLOCATE command prescribes an alignment of 4KB, and the blob's
+  size is also 4KB, the allocation covers a standalone page frame in full
+  (satisfying R1e).
+
+- The 128-bit VMGenID field is located at offset 104 within that page,
+  resulting in a guest-physical address divisible by 8 (satisfying R1a).
+
+- OVMF's ACPI linker/loader allocates the blob in EfiACPIMemoryNVS type memory,
+  therefore it is marked as such in the UEFI memmap (satisfying R1c and R1d).
+
+- OVMF identifies the "UEFI" ACPI Table at the start of the blob in the second
+  pass, following the ADD_POINTER command that is meant to link the table into
+  the RSDT. OVMF installs a *copy* of the "UEFI" ACPI Table with
+  EFI_ACPI_TABLE_PROTOCOL (linking the copy into both RSDT and XSDT). Given the
+  "UEFI" signature of the table, EFI_ACPI_TABLE_PROTOCOL places the copy of the
+  table in EfiACPIMemoryNVS type memory.
+
+- The "ADDR" AML method (see later) is allowed to refer to the "UEFI" ACPI
+  Table with the DataTableRegion operator, because the table is located in
+  memory marked as AddressRangeNVS.
+
+- The "ADDR base pointer" field inside the installed table points at "OVMF SDT
+  Header probe suppressor" in the original blob. Because this field is filled
+  with zeros, OVMF's table identification heuristics unconditionally reports a
+  negative when it tracks the relevant ADD_POINTER command to it in the second
+  pass. Therefore the blob is marked as "hosts something else than just ACPI
+  tables", and it is preserved permanently (in the same EfiACPIMemoryNVS type
+  memory where it has been originally allocated).
+
+  At OSPM runtime, the "ADDR" AML method reads the "ADDR base pointer" field,
+  and adds 42, to arrive at the address of the VMGenID field.
+
+  blob @ page offset 0               RSDT         XSDT
+  +-----------------------------+    +-----+      +-----+
+  | "UEFI" ACPI Table (in blob) |    | ... |      | ... |
+  | +-------------------------+ |    | ... ---+   | ... ---------------+
+  | |XXXXXXXXXXXXXXXXXXXXXXXXX| |    +-----+  |   +-----+              |
+  | |XXXXXXX [unused] XXXXXXXX| |             |                        |
+  | |XXXXXXXXXXXXXXXXXXXXXXXXX| |             +------------------------+
+  | +-------------------------+ |                                      |
+  | probe suppressor <-------------+  "UEFI" ACPI Table (installed) <--+
+  | VMGenID @ offset 104        |  |  +---------------------------+
+  | padding                     |  |  | ...                       |
+  +-----------------------------+  |  | ...                       |
+                                   +--- ADDR base pointer         |
+                                      +---------------------------+
+
+ACPI device, control methods
+----------------------------
+
+Requirements R2 through R6 of the VMGenID specification are satisfied with the
+following ACPI logic, exposed by QEMU's ACPI generator in one of the SSDTs, and
+installed by both guest firmwares as such.
+
+The basic idea is that, when the appropriate guest driver calls the ADDR method
+(see R4), OSPM locates the generation ID field in the 4KB blob that lives in
+E820 Reserved (SeaBIOS) or EfiACPIMemoryNVS type (OVMF) memory. The
+guest-physical address of the field is communicated to QEMU via IO ports
+[0x512..0x519] inclusive. Then QEMU is cued through IO port 0x51A to refresh
+(and keep refreshing when appropriate) the generation ID at the passed back
+address. Finally, the method returns the address to the guest driver too, in
+the format required by R4.
+
+    Scope(\_SB) {
+        Device (VMGI) {
+            /* satisfy R2 */
+            Name (_CID, "VM_Gen_Counter")
+
+            /* satisfy R3 */
+            Name (_DDN, "VM_Gen_Counter")
+
+            /* satisfy R6 */
+            Name (_HID, "QEMU0002")
+
+            /* the device owns this IO port range */
+            Name (_CRS, ResourceTemplate () {
+                IO (Decode16, 0x512, 0x512, 1, 9)
+            })
+
+            /* Device status: present, enabled & decoding resources, should be
+             * shown in the UI, functioning properly.
+             */
+            Name (_STA, 0xF)
+
+            /* Satisfy R4.
+             *
+             * This method is serialized because it creates named objects.
+             */
+            Method (ADDR, 0, Serialized) {
+                /* The 8-byte integer field defined as ADBP below is the
+                 * "ADDR base pointer" field in the UEFI ACPI Table.
+                 *
+                 * The DataTableRegion() operator locates that ACPI table by
+                 * scanning the RSDT/XSDT using the (SignatureString,
+                 * OemIDString, OemTableIDString) triplet as key.
+                 *
+                 * Windows XP would normally crash on the DataTableRegion()
+                 * operator, but it never calls the ADDR method, hence it never
+                 * reaches or evaluates DataTableRegion().
+                 */
+                DataTableRegion (TBLR, "UEFI", "BOCHS", "QEMUPARM")
+                Field (TBLR, AnyAcc, NoLock, Preserve) {
+                  Offset (54),
+                  ADBP, 64
+                }
+
+                /* This is the IO port range exposed in the _CRS above.
+                 *
+                 * The first two 4-byte ports are used to communicate the
+                 * 64-bit guest-physical address of the actual (relocated)
+                 * 128-bit generation ID field to QEMU, in little endian
+                 * encoding, so that QEMU can rewrite that field in guest RAM.
+                 *
+                 * A write to last 1-byte port signals that the address has
+                 * been written fully, and QEMU is free to dereference it.
+                 */
+                OperationRegion (VMGR, SystemIO, 0x512, 9)
+                Field (VMGR, DWordAcc, NoLock, Preserve) {
+                    PTLO, 32,
+                    PTHI, 32,
+                    AccessAs (ByteAcc),
+                    DONE, 8
+                }
+
+                /* The ADBP field points to the "OVMF SDT Header probe
+                 * suppressor" area in the blob, at offset 62. In order to
+                 * arrive at the generation ID field at offset 104, we must add
+                 * 42 dynamically.
+                 *
+                 * The RESU buffer below will contain the result of the
+                 * addition. The ADFU field exposes it as an 8-byte integer
+                 * (for storing the sum), while the ADLO and ADHI fields enable
+                 * us to access the result in two separate 4-byte integers.
+                 * This exact integer width is especially important for
+                 * composing the package object that the ADDR method must
+                 * return.
+                 */
+                Name (RESU, Buffer (8) {})
+                CreateQWordField (RESU, 0, ADFU)
+                CreateDWordField (RESU, 0, ADLO)
+                CreateDWordField (RESU, 4, ADHI)
+
+                Add (ADBP, 42, ADFU)
+                Store (ADLO, PTLO)
+                Store (ADHI, PTHI)
+                Store (0, DONE)
+                Return (Package (2) { ADLO, ADHI })
+            }
+        }
+    }
+
+    /* satisfy R5 */
+    Scope (\_GPE) {
+        Method (_E04) {
+            Notify (\_SB.VMGI, 0x80)
+        }
+    }