Hi, > From: Daniel Drake [mailto:drake@xxxxxxxxxxxx] > Subject: Re: [PATCH] ACPI / EC: handle ECDT EC and DSDT EC simultaneously > > Hi Lv, > > Thanks for the detailed response. In trying to decode the tricky code > flow and seeing all this first_ec / boot_ec / DSDT EC / ECDT EC stuff, > I seem to have made the wrong interpretation about how this is > designed. > > On Sun, Apr 23, 2017 at 10:43 PM, Zheng, Lv <lv.zheng@xxxxxxxxx> wrote: > > The entire problem looks to me is: > > When GPE setting differs between ECDT and DSDT, which one should be > > trusted by OS? > > This case suggests that Windows uses the ECDT setting, right? I checked the provided acpi tables. Indeed, the ECDT EC is correct. [000h 0000 4] Signature : "ECDT" [Embedded Controller Boot Resources Table] [004h 0004 4] Table Length : 000000C1 [008h 0008 1] Revision : 01 [009h 0009 1] Checksum : EC [00Ah 0010 6] Oem ID : "_ASUS_" [010h 0016 8] Oem Table ID : "Notebook" [018h 0024 4] Oem Revision : 01072009 [01Ch 0028 4] Asl Compiler ID : "AMI." [020h 0032 4] Asl Compiler Revision : 00000005 [024h 0036 12] Command/Status Register : [Generic Address Structure] [024h 0036 1] Space ID : 01 [SystemIO] [025h 0037 1] Bit Width : 08 [026h 0038 1] Bit Offset : 00 [027h 0039 1] Encoded Access Width : 00 [Undefined/Legacy] [028h 0040 8] Address : 0000000000000066 [030h 0048 12] Data Register : [Generic Address Structure] [030h 0048 1] Space ID : 01 [SystemIO] [031h 0049 1] Bit Width : 08 [032h 0050 1] Bit Offset : 00 [033h 0051 1] Encoded Access Width : 00 [Undefined/Legacy] [034h 0052 8] Address : 0000000000000062 [03Ch 0060 4] UID : 00000000 [040h 0064 1] GPE Number : 23 [041h 0065 19] Namepath : "\_SB.PCI0.LPCB.EC0" While the DSDT EC is in fact invalid as it returns 0 from its _STA: Scope (_SB.PCI0.LPCB) { Device (H_EC) { Name (_HID, EisaId ("PNP0C09") /* Embedded Controller Device */) // _HID: Hardware ID Name (_UID, One) // _UID: Unique ID Method (_STA, 0, NotSerialized) // _STA: Status { ^^^GFX0.CLID = 0x03 Return (Zero) } ... } } It doesn't contain _CRS, so it's likely that it will fail in acpi_ec_dsdt_probe() due to failure of walk _CRS. Are you sure the same problem can be seen no this platform? If so, possibly you only need to add some sanity checks in acpi_ec_dsdt_probe(). After acpi_get_devices() call, check if its IO addresses are invalid. Or check its _STA (present && return valid value) after that. It might not be possible to do such _STA execution. So IMO, the acpi_ec_dsdt_probe() is not a good choice. The history is: 1. In ACPI spec 1.0, there is no ECDT, BIOS developers have to provide wrappers in EC read/write functions in order to be able to access EC firmware before OS loads the EC driver. Before EC._REG is invoked, access systemio opregion instead of directly accessing EC opregions. In ACPI spec 2.0, ECDT is provided to eliminate the Complication of this needs. 2. During early Linux EC support, ECDT code is provided for the spec purpose. But several un-root-caused kernel bugs made the developers to make the wrong choice, starting to always override ECDT EC using DSDT EC. The culprit commits are c6cb0e87 and a5032bfd. The culprit in fact finally also played as one of those reasons preventing the AML interpreter from being correctly implemented as Windows compliant. 3. However, after switching to the old behavior, and the DSDT boot EC mechanism is entirely abandoned. But users Reported that they can see EC opregion accessed in some _STA/_INI, as Linux executes these control methods very Early (earlier before probe EC driver) and there is no ECDT on those platforms, we can see errors complaining no opregion handler. The bug is kernel Bugzilla 119261. TBH, these errors are not such critical IMO, the BIOS obviously violates facts mentioned in history 1 and we obviously do not have knowledge of the execution order of these control methods on Windows and we obviously do not know if such error can also be seen by Winodws and do not know whether such error can be functional failure and reached BIOS developers' attention. But we finally have no choice but restoring the boot DSDT EC hack back as we do not know how Linux can execute all control methods in the exact order as Windows... Then now, it actually makes the problem more complicated. as it might not be possible to execute _CRS/_GPE/_STA at that time to sanitize the DSDT EC. Probably we could just abandon it again? Anyway we can firstly just add 1 line IO address and _GPE existence sanity check in acpi_ec_dsdt_probe() to fix your issue. > > > The current code chose to always trust DSDT GPE settings as in theory it > > doesn't make sense to trust the ECDT GPE setting in most of the cases, > > ECDT GPE is not meant to be used during runtime. So why don't we just add > > a quirk to favor GPE setting from ECDT rather than the GPE setting from > > DSDT for these platforms? > > Do you mean a DMI quirk? We have found those to be quite impractical > in the past, it is too hard to get 100% coverage of all machines > affected by the bug, and usually we would only be able to quirk it > late in the process (after the machine has already shipped to users). > In a recent case we had an Asus DMI quirks list that grew slowly to > over 30 entries over a year, before we found a generic solution. > > In this case we have asked Asus BIOS engineers to not repeat this > issue on future products, but even if they follow our advice there, we > can expect a decent number of models affected by this issue. > > And we just found a 2nd model with the same issue. Here is the > "acpidump -b" output: > https://www.dropbox.com/s/d3w2xrmrz1oklnw/x580vd_acpi.tgz?dl=0 > > To see the windows device tree using Microsoft Windows Internals, are > you referring to the paperback book? Which of the 2 parts would we > have to purchase? Does it come with a binary on CD or would we have to > figure out how to compile the tool? Sorry I confused it with sysinternal tools https://technet.microsoft.com/en-us/sysinternals/bb842062.aspx However it should be OSR online DDK developer support utility: http://www.osronline.com/article.cfm?id=97 Thanks Lv ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
