Re: [Qemu-devel] [edk2] Windows does not support DataTableRegion at all [was: docs: describe QEMU's VMGenID design]

[Laszlo Ersek <lersek@redhat.com>]

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