From: Nick Desaulniers <ndesaulniers(a)google.com>
Sent: Friday, November 13, 2020 2:09 PM
To: Moore, Robert <robert.moore(a)intel.com>
Cc: Kaneda, Erik <erik.kaneda(a)intel.com>; Wysocki, Rafael J <rafael.j.wysocki(a)intel.com>; Gustavo A . R . Silva <gustavoars(a)kernel.org>; clang-built-linux(a)googlegroups.com; Len Brown <lenb(a)kernel.org>; linux-acpi(a)vger.kernel.org; devel(a)acpica.org; linux-kernel(a)vger.kernel.org
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Fri, Nov 13, 2020 at 1:45 PM Moore, Robert <robert.moore(a)intel.com> wrote:
> BTW, if you can make a pull request for the patch up on github, that would help.
Great, thanks. I'll look at/merge the request next week.
I can do it this way:
In the global header actypes.h:
In the gcc-specific header (acgcc.h):
#define ACPI_FALLTHROUGH __attribute__((__fallthrough__))
This would not be #defined in the MSVC-specific header (acmsvc.h) -- thus using the default (null) in actypes.h (The per-environment headers are always included first).
(We do all macros in upper case, prefixed with "ACPI_")
If you can update your patch to use ACPI_FALLTHROUGH, I can do the rest (above).
From: Joe Perches <joe(a)perches.com>
Sent: Friday, November 13, 2020 8:30 AM
To: Miguel Ojeda <miguel.ojeda.sandonis(a)gmail.com>; Nick Desaulniers <ndesaulniers(a)google.com>
Cc: Moore, Robert <robert.moore(a)intel.com>; Kaneda, Erik <erik.kaneda(a)intel.com>; Wysocki, Rafael J <rafael.j.wysocki(a)intel.com>; Gustavo A . R . Silva <gustavoars(a)kernel.org>; clang-built-linux(a)googlegroups.com; Len Brown <lenb(a)kernel.org>; linux-acpi(a)vger.kernel.org; devel(a)acpica.org; linux-kernel(a)vger.kernel.org
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Fri, 2020-11-13 at 09:14 +0100, Miguel Ojeda wrote:
> On Fri, Nov 13, 2020 at 1:09 AM Nick Desaulniers
> <ndesaulniers(a)google.com> wrote:
> > Thank you for the explicit diagnostics observed. Something fishy is
> > going on though, https://godbolt.org/z/Gbxbxa is how I expect MSVC
> > to handle include/linux/compiler_attributes.h.
> > The C preprocessor should make it such that MSVC never sees
> > `__attribute__` or `__fallthrough__`; that it does begs the question.
> > That would seem to imply that `#if __has_attribute(__fallthrough__)`
> > somehow evaluates to true on MSVC, but my godbolt link shows it does
> > not.
> > Could the upstream ACPICA project be #define'ing something that
> > could be altering this? (Or not #define'ing something?)
> > Worst case, we could do as Joe Perches suggested and disable
> > -Wfallthrough for drivers/acpi/acpica/.
> I agree, something is fishy. MSVC has several flags for conformance
> and extensions support, including two full C preprocessors in newer
> versions; which means we might be missing something, but I don't see
> how the code in compiler_attributes.h could be confusing MSVC even in
> older non-conforming versions.
I believe this has nothing to do with linux and only to do with compiling acpica for other environments like Windows.
The ACPI Component Architecture (ACPICA) project provides an operating system (OS)-independent reference implementation of the Advanced Configuration and Power Interface Specification (ACPI).
It can be easily adapted to execute under any host OS.
13 November 2020. Summary of changes for version 20201113:
This release is available at https://acpica.org/downloads
1) ACPICA kernel-resident subsystem:
Interpreter: fixed a memory leak by using use existing buffer in _HID repair. There was a memory leak that occurred when a _CID object is defined as a package containing string objects. When _CID is checked for any possible repairs, it calls a helper function to repair _HID (because _CID basically contains multiple _HID entries). The _HID repair function assumes that string objects are standalone objects that are not contained inside of any packages. The _HID repair function replaced the string object with a brand new object and attempted to delete the old object by decrementing the reference count of the old object. Strings inside of packages have a reference count of 2 so the _HID repair function leaves this object in a dangling state and causes a memory leak. Instead of allocating a brand new object and removing the old object, use the existing object when repairing the _HID object.
Added function trace macros to improve namespace debugging. The namespace repair mechanism does not have function tracing macros. Add several trace macros to improve debuggability.
Handle "orphan" _REG methods for GPIO OpRegions. Before this change AcpiEvExecuteRegMethods() had special handling to handle "orphan" (no matching OpRegion declared) _REG methods for EC nodes. On Intel Cherry Trail devices there are 2 possible ACPI OpRegions for accessing GPIOs. The standard GeneralPurposeIo OpRegion and the Cherry Trail - specific UserDefined 0x9X OpRegions. Having 2 different types of OpRegions leads to potential issues with checks for OpRegion availability, or in other words checks if _REG has been called for the OpRegion which the ACPI code wants to use. Except for the "orphan" EC handling, ACPICA core does not call _REG on an ACPI node which does not define an OpRegion matching the type being registered; and the reference design DSDT, from which most Cherry Trail DSDTs are derived, does not define GeneralPurposeIo, nor UserDefined(0x93) OpRegions for the GPO2 (UID 3) device, because no pins were assigned ACPI controlled functions in the reference design. Together this leads to the perfect storm, at least on the Cherry Trail based Medion Akayo E1239T. This design does use a GPO2 pin from its ACPI code and has added the Cherry Trail specific UserDefined(0x93) opregion to its GPO2 ACPI node to access this pin. But it uses a "has _REG been called" availability check for the standard GeneralPurposeIo OpRegion. This clearly is a bug in the DSDT, but this does work under Windows. This issue leads to the intel vbtn driver reporting the device always being in tablet-mode at boot, even if it is in laptop mode. Which in turn causes userspace to ignore touchpad events. So in other words, this issue causes the touchpad to not work at boot. This change fixes this by extending the "orphan" _REG method handling to also apply to GPIO address-space handlers.
2) iASL Compiler/Disassembler and ACPICA tools:
iASL: Added more info to namespace dump file (-ln option). In a separate section of the dump file (after the main namespace dump), emit the full pathname for each namespace node, its type, and the ASL filename and line number where it is declared.
AcpiHelp: Added an option to display/decode iASL exceptions. Option is: -x [Hex Value] where "Hex Value" is the iASL exception code. If Hex Value is omitted, all iASL exceptions are displayed.
iASL: Use StringLiteral instead of StringData for some ASL macros. The use of the stringData rule allows for some "string" oriented opcodes (Such as ToString, ToHexString, etc.) None of which make sense with the macros in question. This change modifies the StringData part of the rule for these macros to a simple string literal - thus disallowing the use of ToString, ToHexString, etc.
The following ASL operators (macros) are affected:
Note: The MS compiler requires the use of string literals for these operators also.
iASL: Added a remark for an unknown UUID: ASL_MSG_UUID_NOT_FOUND. Search the list of "known" UUIDs for the input to the ToUUID macro.
Added 5 new UUIDs to the known UUID table. All related to NVDIMM and the NFIT table.