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:ler...@redhat.com] >> Sent: Tuesday, September 15, 2015 3:49 AM >> To: Bill Paul >> Cc: edk2-de...@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-de...@lists.01.org >>>>>> https://lists.01.org/mailman/listinfo/edk2-devel >>> >