diff mbox series

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

Message ID 20241216235241.217642-2-Ariel.Otilibili-Anieli@eurecom.fr (mailing list archive)
State New
Headers show
Series tools/libacpi: clear ASL warning about PCI0 | expand

Commit Message

Ariel Otilibili Dec. 16, 2024, 11:50 p.m. UTC
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:

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]:

```
$ cat /sys/firmware/acpi/tables/DSDT > dsdt.dat
$ iasl -v

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

$ iasl -d dsdt.dat 2>/dev/null

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

$ grep PNP0A03 -B3 dsdt.dsl
        Device (PCI0)
        {
            Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
            Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
```

[1] https://uefi.org/sites/default/files/resources/ACPI_2.pdf
[2] https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html?highlight=_hid#device-identification-objects
[3] https://www.infradead.org/~mchehab/kernel_docs/firmware-guide/acpi/namespace.html

Link: https://www.intel.com/content/www/us/en/developer/topic-technology/open/acpica/documentation.html
Fixes: a5da231f56268702ba9d9e0c4f1ad7156446e77b
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Anthony PERARD <anthony.perard@vates.tech>
Signed-off-by: Ariel Otilibili <Ariel.Otilibili-Anieli@eurecom.fr>
---
 tools/libacpi/dsdt.asl | 1 -
 1 file changed, 1 deletion(-)

Comments

Jan Beulich Dec. 17, 2024, 9:29 a.m. UTC | #1
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
Ariel Otilibili Dec. 17, 2024, 1:24 p.m. UTC | #2
On Tuesday, December 17, 2024 10:29 CET, Jan Beulich <jbeulich@suse.com> 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
Jan Beulich Dec. 17, 2024, 1:58 p.m. UTC | #3
On 17.12.2024 14:24, Ariel Otilibili-Anieli wrote:
> On Tuesday, December 17, 2024 10:29 CET, Jan Beulich <jbeulich@suse.com> 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.

Well, if you want to double check what I've done, feel free. As per above
I did already identify when the wording changed.

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

Disabling warnings altogether is unlikely to be what we want. And the help
output above also doesn't suggest there's a control to suppress specifically
what we may want to suppress. Even suppressing warnings by <messageid> is
likely going to be too broad.

Jan
diff mbox series

Patch

diff --git a/tools/libacpi/dsdt.asl b/tools/libacpi/dsdt.asl
index 32b42f85ae..9d50578e98 100644
--- a/tools/libacpi/dsdt.asl
+++ b/tools/libacpi/dsdt.asl
@@ -41,7 +41,6 @@  DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM", 0)
        {
            Name (_HID, EisaId ("PNP0A03"))
            Name (_UID, 0x00)
-           Name (_ADR, 0x00)
            Name (_BBN, 0x00)
 
            /* Make cirrues VGA S3 suspend/resume work in Windows XP/2003 */