Re: [PATCH v2 1/1] tools/libacpi: clear ASL warning about PCI0

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.12.2024 00:50, Ariel Otilibili wrote:
> iasl complains _HID and _ADR cannot be used at the same time:
> 
> ```
> /usr/bin/iasl -vs -p tools/firmware/hvmloader/dsdt_anycpu.tmp -tc 
> tools/firmware/hvmloader/dsdt_anycpu.asl 2>&1 | grep -B10 HID
> 
> tools/firmware/hvmloader/dsdt_anycpu.asl     40:        Device (PCI0)
> Warning  3073 -                                    Multiple types ^  (Device 
> object requires either a _HID or _ADR, but not both)
> ```
> 
> Per ACPI 2.0 (Jul. 27, 2000; Section 6.1, page 146), the configuration was 
> legit:
> 
> "A device object must contain either an _HID object or an _ADR object,
> but can contain both." [1]
> 
> But, per ACPI 6.5 (Aug. 2022), this is no more legit:
> 
> "A device object must contain either an _HID object or an _ADR object,
> but must not contain both." [2]
> 
> Generally _HID devices are enumerated and have their drivers loaded
> by ACPI ("ASL 2.0 Introduction and Overview", page 4).
> 
> Removing _ADR, the warning is cleared out.
> 
> The change should be compatible down to OSes released after ACPI 2.0,
> including Windows XP:

So my earlier hint apparently wasn't clear enough. I really would have
expected you to determine in what version the wording changed. Even 5.1
still has the old wording, and that's more than 10 years newer than 2.0.
And then in 6.0 the wording first changed to "but should not contain
both."

With this I'm afraid considering just WinXP is insufficient. May I also
point you at a Win2K related comment in acpi_build_tables(), seemingly
suggesting that that still was a "ACPI 1.0 operating system"? Further
in that function you'll find that apparently, besides the 1.0 special
case, we only support ACPI revisions 4 and 5. Therefore the spec change
in v6 would become relevant only once we actually supported (and
surfaced to guests) v6. At that point I'd further be of the opinion that
unless it can be proven that _ADR is unused by any OS we (ever) care(d)
about, we'd need to further split the set of DSDTs we may make use of.
One (pair) for up to 5.x with _ADR present, and another (pair) for 6.0
and newer with _ADR absent.

I'm further afraid that ...

> 1. The _HID kept in the DSDT files is the EISA ID "PNP0A03",
> Microsoft recognizes it as PCI bus:
> 
> ```
> $ curl -k -s 
> https://download.microsoft.com/download/1/6/1/161ba512-40e2-4cc9-843a-923143f3456c/devids.txt
>  | grep PNP0A
> 
> PNP0A00         ISA Bus
> PNP0A01         EISA Bus
> PNP0A02         MCA Bus
> PNP0A03         PCI Bus
> PNP0A04         VESA/VL Bus
> PNP0A05         Generic ACPI Bus
> PNP0A06         Generic ACPI Extended-IO Bus (EIO bus)
> ```
> 
> 2. Linux 6.12 uses also _HID for identifying PCI devices [3]:

... this fact alone means very little here. The more important question is
whether there are / were OSes which use(d) _ADR for any purpose even when
_HID is there. With just looking at the surface of just Linux, I find e.g.
a library-like function acpi_get_local_u64_address(), all users of which
would need auditing. Plus, once done, we'd then still only know the state
of things in one specific Linux version.

Bottom line: I wonder whether iasl has an option to suppress that warning.
Sadly I can't find a new enough iasl anywhere on the systems I have easy
access to, so I can't check myself. If there was no way to suppress this
warning, I'd wonder whether this wasn't a shortcoming of the tool, as the
warning is clearly inappropriate when dealing with tables for pre-v6
configurations.

Jan