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

["Ariel Otilibili-Anieli" <Ariel.Otilibili-Anieli@xxxxxxxxxx>]

On Tuesday, December 17, 2024 10:29 CET, Jan Beulich <jbeulich@xxxxxxxx> wrote:

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

I can do that, Jan; I'll research from where did the wording changed.
> 
> 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.

Let me look into acpi_build_tables(). Thanks for the hints.
> 
> 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.

There are flags to remove warnings:

```
$ iasl -h | grep -i warn
Errors, Warnings, and Remarks:
  -va                 Disable all errors/warnings/remarks
  -ve                 Report only errors (ignore warnings and remarks)
  -vi                 Less verbose errors and warnings for use with IDEs
  -vw <messageid>     Ignore specific error, warning or remark
  -vx <messageid>     Expect a specific warning, remark, or error
  -w <1|2|3>          Set warning reporting level
  -we                 Report warnings as errors
  -ww <messageid>     Report specific warning or remark as error

$ iasl -v 

Intel ACPI Component Architecture
ASL+ Optimizing Compiler/Disassembler version 20240927
Copyright (c) 2000 - 2023 Intel Corporation
```

I am keeping you posted; I am compiling with warnings disabled.
Ariel
> 
> Jan