| Message ID | 20241215154659.151158-2-Ariel.Otilibili-Anieli@eurecom.fr (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/libacpi: clear ASL warning about PCI0 | expand |
On 15.12.2024 16:40, 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) > ``` > > * generally _HID devices are enumerated and have their drivers loaded by ACPI > * this is from "ASL 2.0 Introduction and Overview" (page 4). > * removing _ADR, the warning is cleared out. Okay, that's the positive aspect. Yet what about the potential fallout thereof? Can you confirm that there's no risk of regressions with older guest OSes, for example? Jan
On Monday, December 16, 2024 10:53 CET, Jan Beulich <jbeulich@suse.com> wrote: > On 15.12.2024 16:40, 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) > > ``` > > > > * generally _HID devices are enumerated and have their drivers loaded by ACPI > > * this is from "ASL 2.0 Introduction and Overview" (page 4). > > * removing _ADR, the warning is cleared out. > > Okay, that's the positive aspect. Yet what about the potential fallout thereof? > Can you confirm that there's no risk of regressions with older guest OSes, for > example? Hi Jan, OSes that were released after ACPI 2.0 should work [1]; including WinXP: The 2.0 specs says either _HID or _ADR should be included [2], not both (Section 6.1, page 146). I chose WinXP because, on another patch, it came up in the discussion [3]. Ariel [1] https://uefi.org/acpi/specs [2] https://uefi.org/sites/default/files/resources/ACPI_2.pdf [3] https://lore.kernel.org/all/ed248424-00a2-44ab-a7cf-3f2197d589a1@citrix.com/ > > Jan
On 16.12.2024 11:36, Ariel Otilibili-Anieli wrote: > On Monday, December 16, 2024 10:53 CET, Jan Beulich <jbeulich@suse.com> wrote: > >> On 15.12.2024 16:40, 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) >>> ``` >>> >>> * generally _HID devices are enumerated and have their drivers loaded by ACPI >>> * this is from "ASL 2.0 Introduction and Overview" (page 4). >>> * removing _ADR, the warning is cleared out. >> >> Okay, that's the positive aspect. Yet what about the potential fallout thereof? >> Can you confirm that there's no risk of regressions with older guest OSes, for >> example? > > OSes that were released after ACPI 2.0 should work [1]; including WinXP: > The 2.0 specs says either _HID or _ADR should be included [2], not both (Section 6.1, page 146). We must be looking at two different variants of the spec then. My copy says "device object must contain either an _HID object or an _ADR object, but can contain both." Also still in 2.0c. I agree that in e.g. 6.5 the wording has changed. I also agree that the use of "either" doesn't help clarity. > I chose WinXP because, on another patch, it came up in the discussion [3]. And indeed appropriately so. Jan
On Monday, December 16, 2024 12:01 CET, Jan Beulich <jbeulich@suse.com> wrote: > On 16.12.2024 11:36, Ariel Otilibili-Anieli wrote: > > On Monday, December 16, 2024 10:53 CET, Jan Beulich <jbeulich@suse.com> wrote: > > > >> On 15.12.2024 16:40, 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) > >>> ``` > >>> > >>> * generally _HID devices are enumerated and have their drivers loaded by ACPI > >>> * this is from "ASL 2.0 Introduction and Overview" (page 4). > >>> * removing _ADR, the warning is cleared out. > >> > >> Okay, that's the positive aspect. Yet what about the potential fallout thereof? > >> Can you confirm that there's no risk of regressions with older guest OSes, for > >> example? > > > > OSes that were released after ACPI 2.0 should work [1]; including WinXP: > > The 2.0 specs says either _HID or _ADR should be included [2], not both (Section 6.1, page 146). > > We must be looking at two different variants of the spec then. My copy says > "device object must contain either an _HID object or an _ADR object, but can > contain both." Also still in 2.0c. I agree that in e.g. 6.5 the wording has > changed. I also agree that the use of "either" doesn't help clarity. I looked up 2.0 (July 2000); indeed, it said "can contain both". My bad. > > > I chose WinXP because, on another patch, it came up in the discussion [3]. The change should work down to WinXP: the name _HID is kept. ``` $ git grep -B2 -A2 -n PNP0A03 tools/libacpi/dsdt.asl-40- Device (PCI0) tools/libacpi/dsdt.asl-41- { tools/libacpi/dsdt.asl:42: Name (_HID, EisaId ("PNP0A03")) tools/libacpi/dsdt.asl-43- Name (_UID, 0x00) tools/libacpi/dsdt.asl-44- Name (_ADR, 0x00) ``` Its EISA ID is "PNP0A03"; the namespace is reserved for Microsoft. Microsoft identifies "PNP0A03" as PCI devices [1]. ``` $ curl -k -s https://download.microsoft.com/download/1/6/1/161ba512-40e2-4cc9-843a-923143f3456c/devids.txt | grep PNP0A03 PNP0A03 PCI Bus ``` Linux displays PCI devices with _HID and no _HID [2]. [1] https://download.microsoft.com/download/1/6/1/161ba512-40e2-4cc9-843a-923143f3456c/devids.txt [2] https://www.infradead.org/~mchehab/kernel_docs/firmware-guide/acpi/namespace.html > > And indeed appropriately so. > > Jan
On 16.12.2024 12:31, Ariel Otilibili-Anieli wrote: > On Monday, December 16, 2024 12:01 CET, Jan Beulich <jbeulich@suse.com> wrote: > >> On 16.12.2024 11:36, Ariel Otilibili-Anieli wrote: >>> On Monday, December 16, 2024 10:53 CET, Jan Beulich <jbeulich@suse.com> wrote: >>> >>>> On 15.12.2024 16:40, 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) >>>>> ``` >>>>> >>>>> * generally _HID devices are enumerated and have their drivers loaded by ACPI >>>>> * this is from "ASL 2.0 Introduction and Overview" (page 4). >>>>> * removing _ADR, the warning is cleared out. >>>> >>>> Okay, that's the positive aspect. Yet what about the potential fallout thereof? >>>> Can you confirm that there's no risk of regressions with older guest OSes, for >>>> example? >>> >>> OSes that were released after ACPI 2.0 should work [1]; including WinXP: >>> The 2.0 specs says either _HID or _ADR should be included [2], not both (Section 6.1, page 146). >> >> We must be looking at two different variants of the spec then. My copy says >> "device object must contain either an _HID object or an _ADR object, but can >> contain both." Also still in 2.0c. I agree that in e.g. 6.5 the wording has >> changed. I also agree that the use of "either" doesn't help clarity. > > I looked up 2.0 (July 2000); indeed, it said "can contain both". My bad. >> >>> I chose WinXP because, on another patch, it came up in the discussion [3]. > > The change should work down to WinXP: the name _HID is kept. > > ``` > $ git grep -B2 -A2 -n PNP0A03 > tools/libacpi/dsdt.asl-40- Device (PCI0) > tools/libacpi/dsdt.asl-41- { > tools/libacpi/dsdt.asl:42: Name (_HID, EisaId ("PNP0A03")) > tools/libacpi/dsdt.asl-43- Name (_UID, 0x00) > tools/libacpi/dsdt.asl-44- Name (_ADR, 0x00) > ``` > > Its EISA ID is "PNP0A03"; the namespace is reserved for Microsoft. Microsoft identifies "PNP0A03" as PCI devices [1]. You again say "should" without explaining what you derive this from. Is it written down somewhere that no OS we (remotely) care about ever evaluated _ADR when _HID was there? As before, along side mentioning the benefits of the change, I'd like to also see a discussion of risks. Jan
On Monday, December 16, 2024 12:38 CET, Jan Beulich <jbeulich@suse.com> wrote: > On 16.12.2024 12:31, Ariel Otilibili-Anieli wrote: > > On Monday, December 16, 2024 12:01 CET, Jan Beulich <jbeulich@suse.com> wrote: > > > >> On 16.12.2024 11:36, Ariel Otilibili-Anieli wrote: > >>> On Monday, December 16, 2024 10:53 CET, Jan Beulich <jbeulich@suse.com> wrote: > >>> > >>>> On 15.12.2024 16:40, 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) > >>>>> ``` > >>>>> > >>>>> * generally _HID devices are enumerated and have their drivers loaded by ACPI > >>>>> * this is from "ASL 2.0 Introduction and Overview" (page 4). > >>>>> * removing _ADR, the warning is cleared out. > >>>> > >>>> Okay, that's the positive aspect. Yet what about the potential fallout thereof? > >>>> Can you confirm that there's no risk of regressions with older guest OSes, for > >>>> example? > >>> > >>> OSes that were released after ACPI 2.0 should work [1]; including WinXP: > >>> The 2.0 specs says either _HID or _ADR should be included [2], not both (Section 6.1, page 146). > >> > >> We must be looking at two different variants of the spec then. My copy says > >> "device object must contain either an _HID object or an _ADR object, but can > >> contain both." Also still in 2.0c. I agree that in e.g. 6.5 the wording has > >> changed. I also agree that the use of "either" doesn't help clarity. > > > > I looked up 2.0 (July 2000); indeed, it said "can contain both". My bad. > >> > >>> I chose WinXP because, on another patch, it came up in the discussion [3]. > > > > The change should work down to WinXP: the name _HID is kept. > > > > ``` > > $ git grep -B2 -A2 -n PNP0A03 > > tools/libacpi/dsdt.asl-40- Device (PCI0) > > tools/libacpi/dsdt.asl-41- { > > tools/libacpi/dsdt.asl:42: Name (_HID, EisaId ("PNP0A03")) > > tools/libacpi/dsdt.asl-43- Name (_UID, 0x00) > > tools/libacpi/dsdt.asl-44- Name (_ADR, 0x00) > > ``` > > > > Its EISA ID is "PNP0A03"; the namespace is reserved for Microsoft. Microsoft identifies "PNP0A03" as PCI devices [1]. > > You again say "should" without explaining what you derive this from. Is it > written down somewhere that no OS we (remotely) care about ever evaluated > _ADR when _HID was there? As before, along side mentioning the benefits of > the change, I'd like to also see a discussion of risks. > I derive this knowledge only from the APCI specs. Indeed, I've not researched how every OS interprets _HID and _ADR. From your answer, I see you would like to be sure the change will introduce no regression. I do understand you point of view; keeping the backward compatibility. The benefit is that the warning will be cleared. We agree on that. The risk is that, if ever any OS relies on _ADR, and cannot read _HID; it would break. After thinking about it, the other way is less risky: this _HID name is only recognized by Windows. Every OS should (I did say it again, should) understand _ADR. If you think the change makes sense, I can remove _HID instead of _ADR. Otherwise, I think we should end the discussion. Ariel > Jan
On 16.12.2024 13:19, Ariel Otilibili-Anieli wrote: > On Monday, December 16, 2024 12:38 CET, Jan Beulich <jbeulich@suse.com> wrote: > >> On 16.12.2024 12:31, Ariel Otilibili-Anieli wrote: >>> On Monday, December 16, 2024 12:01 CET, Jan Beulich <jbeulich@suse.com> wrote: >>> >>>> On 16.12.2024 11:36, Ariel Otilibili-Anieli wrote: >>>>> On Monday, December 16, 2024 10:53 CET, Jan Beulich <jbeulich@suse.com> wrote: >>>>> >>>>>> On 15.12.2024 16:40, 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) >>>>>>> ``` >>>>>>> >>>>>>> * generally _HID devices are enumerated and have their drivers loaded by ACPI >>>>>>> * this is from "ASL 2.0 Introduction and Overview" (page 4). >>>>>>> * removing _ADR, the warning is cleared out. >>>>>> >>>>>> Okay, that's the positive aspect. Yet what about the potential fallout thereof? >>>>>> Can you confirm that there's no risk of regressions with older guest OSes, for >>>>>> example? >>>>> >>>>> OSes that were released after ACPI 2.0 should work [1]; including WinXP: >>>>> The 2.0 specs says either _HID or _ADR should be included [2], not both (Section 6.1, page 146). >>>> >>>> We must be looking at two different variants of the spec then. My copy says >>>> "device object must contain either an _HID object or an _ADR object, but can >>>> contain both." Also still in 2.0c. I agree that in e.g. 6.5 the wording has >>>> changed. I also agree that the use of "either" doesn't help clarity. >>> >>> I looked up 2.0 (July 2000); indeed, it said "can contain both". My bad. >>>> >>>>> I chose WinXP because, on another patch, it came up in the discussion [3]. >>> >>> The change should work down to WinXP: the name _HID is kept. >>> >>> ``` >>> $ git grep -B2 -A2 -n PNP0A03 >>> tools/libacpi/dsdt.asl-40- Device (PCI0) >>> tools/libacpi/dsdt.asl-41- { >>> tools/libacpi/dsdt.asl:42: Name (_HID, EisaId ("PNP0A03")) >>> tools/libacpi/dsdt.asl-43- Name (_UID, 0x00) >>> tools/libacpi/dsdt.asl-44- Name (_ADR, 0x00) >>> ``` >>> >>> Its EISA ID is "PNP0A03"; the namespace is reserved for Microsoft. Microsoft identifies "PNP0A03" as PCI devices [1]. >> >> You again say "should" without explaining what you derive this from. Is it >> written down somewhere that no OS we (remotely) care about ever evaluated >> _ADR when _HID was there? As before, along side mentioning the benefits of >> the change, I'd like to also see a discussion of risks. >> > > I derive this knowledge only from the APCI specs. Indeed, I've not researched how every OS interprets _HID and _ADR. > > From your answer, I see you would like to be sure the change will introduce no regression. I do understand you point of view; keeping the backward compatibility. > > The benefit is that the warning will be cleared. We agree on that. > > The risk is that, if ever any OS relies on _ADR, and cannot read _HID; it would break. > > After thinking about it, the other way is less risky: this _HID name is only recognized by Windows. Every OS should (I did say it again, should) understand _ADR. > > If you think the change makes sense, I can remove _HID instead of _ADR. But that would remove relevant information, the the PNP ID serves a purpose. > Otherwise, I think we should end the discussion. Well, you may decide to withdraw / abandon the patch, or you may decide to re-submit with an extended description, clarifying why the removal is expected to be safe. Even if - obviously - you can't inspect e.g. Windows sources. Jan
On Monday, December 16, 2024 13:39 CET, Jan Beulich <jbeulich@suse.com> wrote: > On 16.12.2024 13:19, Ariel Otilibili-Anieli wrote: > > On Monday, December 16, 2024 12:38 CET, Jan Beulich <jbeulich@suse.com> wrote: > > > >> On 16.12.2024 12:31, Ariel Otilibili-Anieli wrote: > >>> On Monday, December 16, 2024 12:01 CET, Jan Beulich <jbeulich@suse.com> wrote: > >>> > >>>> On 16.12.2024 11:36, Ariel Otilibili-Anieli wrote: > >>>>> On Monday, December 16, 2024 10:53 CET, Jan Beulich <jbeulich@suse.com> wrote: > >>>>> > >>>>>> On 15.12.2024 16:40, 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) > >>>>>>> ``` > >>>>>>> > >>>>>>> * generally _HID devices are enumerated and have their drivers loaded by ACPI > >>>>>>> * this is from "ASL 2.0 Introduction and Overview" (page 4). > >>>>>>> * removing _ADR, the warning is cleared out. > >>>>>> > >>>>>> Okay, that's the positive aspect. Yet what about the potential fallout thereof? > >>>>>> Can you confirm that there's no risk of regressions with older guest OSes, for > >>>>>> example? > >>>>> > >>>>> OSes that were released after ACPI 2.0 should work [1]; including WinXP: > >>>>> The 2.0 specs says either _HID or _ADR should be included [2], not both (Section 6.1, page 146). > >>>> > >>>> We must be looking at two different variants of the spec then. My copy says > >>>> "device object must contain either an _HID object or an _ADR object, but can > >>>> contain both." Also still in 2.0c. I agree that in e.g. 6.5 the wording has > >>>> changed. I also agree that the use of "either" doesn't help clarity. > >>> > >>> I looked up 2.0 (July 2000); indeed, it said "can contain both". My bad. > >>>> > >>>>> I chose WinXP because, on another patch, it came up in the discussion [3]. > >>> > >>> The change should work down to WinXP: the name _HID is kept. > >>> > >>> ``` > >>> $ git grep -B2 -A2 -n PNP0A03 > >>> tools/libacpi/dsdt.asl-40- Device (PCI0) > >>> tools/libacpi/dsdt.asl-41- { > >>> tools/libacpi/dsdt.asl:42: Name (_HID, EisaId ("PNP0A03")) > >>> tools/libacpi/dsdt.asl-43- Name (_UID, 0x00) > >>> tools/libacpi/dsdt.asl-44- Name (_ADR, 0x00) > >>> ``` > >>> > >>> Its EISA ID is "PNP0A03"; the namespace is reserved for Microsoft. Microsoft identifies "PNP0A03" as PCI devices [1]. > >> > >> You again say "should" without explaining what you derive this from. Is it > >> written down somewhere that no OS we (remotely) care about ever evaluated > >> _ADR when _HID was there? As before, along side mentioning the benefits of > >> the change, I'd like to also see a discussion of risks. > >> > > > > I derive this knowledge only from the APCI specs. Indeed, I've not researched how every OS interprets _HID and _ADR. > > > > From your answer, I see you would like to be sure the change will introduce no regression. I do understand you point of view; keeping the backward compatibility. > > > > The benefit is that the warning will be cleared. We agree on that. > > > > The risk is that, if ever any OS relies on _ADR, and cannot read _HID; it would break. > > > > After thinking about it, the other way is less risky: this _HID name is only recognized by Windows. Every OS should (I did say it again, should) understand _ADR. > > > > If you think the change makes sense, I can remove _HID instead of _ADR. > > But that would remove relevant information, the the PNP ID serves a purpose. You are right, I dumped the DSDT tables of my Linux, and the PCI object does use it: ``` # cat /sys/firmware/acpi/tables/DSDT > dsdt.dat # iasl -e dsdt.dat -d dsdt.dsl # grep PNP0A03 -B3 dsdt.dsl Device (PC00) { Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID ``` > > > Otherwise, I think we should end the discussion. > > Well, you may decide to withdraw / abandon the patch, or you may decide to > re-submit with an extended description, clarifying why the removal is > expected to be safe. Even if - obviously - you can't inspect e.g. Windows > sources. Thanks for the feedback, Jan; I am pushing another patch series. Documenting all the findings. I'll keep you posted, Ariel > > Jan
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 */
* 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) ``` * generally _HID devices are enumerated and have their drivers loaded by ACPI * this is from "ASL 2.0 Introduction and Overview" (page 4). * removing _ADR, the warning is cleared out. 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(-)