| Message ID | 1517377939-9817-1-git-send-email-alex.hung@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: > In recent Intel hardware the IRQs become non-configurable after BIOS > initializes them in PEI phase and _PRS objects are no longer included in > ASL. > > This is the same as "static (non-configurable) devices do not > specify a _PRS object" in ACPI spec. As a result, error messages > saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not > needed. That's questionable at best. The errors basically indicate that _PRT entries corresponding to these IRQs are messed up (because they should contain the value of 0 instead of a NamePath in the Source column), so we are now going to paper over bugs in ACPI tables as someone in the firmware land cannot be bothered with putting correct values into them. :-/ > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > drivers/acpi/pci_link.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 85ad679390e3..f98215971f30 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -172,10 +172,8 @@ static int acpi_pci_link_get_possible(struct acpi_pci_link *link) > > status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, > acpi_pci_link_check_possible, link); > - if (ACPI_FAILURE(status)) { > - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); > + if (ACPI_FAILURE(status)) > return -ENODEV; > - } > > ACPI_DEBUG_PRINT((ACPI_DB_INFO, > "Found %d possible IRQs\n", > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: >> In recent Intel hardware the IRQs become non-configurable after BIOS >> initializes them in PEI phase and _PRS objects are no longer included in >> ASL. >> >> This is the same as "static (non-configurable) devices do not >> specify a _PRS object" in ACPI spec. As a result, error messages >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not >> needed. > > That's questionable at best. > > The errors basically indicate that _PRT entries corresponding to these > IRQs are messed up (because they should contain the value of 0 instead of > a NamePath in the Source column), so we are now going to paper over bugs > in ACPI tables as someone in the firmware land cannot be bothered with > putting correct values into them. :-/ Rafael, Thanks for quick reply and sharing the information It seems static (non-configurable) devices on ACPI are discussed in both _PRS and _PRT as below: 6.2.12 _PRS (Possible Resource Settings) "... Static (non-configurable) devices do not specify a _PRS object... " 6.2.13 _PRT (PCI Routing Table) "In the second model, the PCI interrupts are hardwired to specific interrupt inputs on the interrupt controller and are not configurable. In this case, the Source field in _PRT does not reference a device, but instead contains the value zero, and the Source Index field contains the global system interrupt to which the PCI interrupt is hardwired." My interpretation is the both are true from ACPI's perspective, and both should be implemented by system firmware. On this particular system I am debugging remotely, it does the _PRS part but not _PRT, and I will follow up with firmware engineers. On the other hand, it may not be unreasonable to remove AE_NOT_FOUND as defined in 6.2.12 in ACPI spec. I also did a code trace and it seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of zero in Source field in _PRT. Finally, this seems to be a confusing topic in ACPI spec (to the firmware engineers and I at least). If we can figure this out, we may want to clarify the wording with ASWG. > >> Signed-off-by: Alex Hung <alex.hung@canonical.com> >> --- >> drivers/acpi/pci_link.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c >> index 85ad679390e3..f98215971f30 100644 >> --- a/drivers/acpi/pci_link.c >> +++ b/drivers/acpi/pci_link.c >> @@ -172,10 +172,8 @@ static int acpi_pci_link_get_possible(struct acpi_pci_link *link) >> >> status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, >> acpi_pci_link_check_possible, link); >> - if (ACPI_FAILURE(status)) { >> - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); >> + if (ACPI_FAILURE(status)) >> return -ENODEV; >> - } >> >> ACPI_DEBUG_PRINT((ACPI_DB_INFO, >> "Found %d possible IRQs\n", >> > >
On Wed, Jan 31, 2018 at 8:39 PM, Alex Hung <alex.hung@canonical.com> wrote: > On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: >>> In recent Intel hardware the IRQs become non-configurable after BIOS >>> initializes them in PEI phase and _PRS objects are no longer included in >>> ASL. >>> >>> This is the same as "static (non-configurable) devices do not >>> specify a _PRS object" in ACPI spec. As a result, error messages >>> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not >>> needed. >> >> That's questionable at best. >> >> The errors basically indicate that _PRT entries corresponding to these >> IRQs are messed up (because they should contain the value of 0 instead of >> a NamePath in the Source column), so we are now going to paper over bugs >> in ACPI tables as someone in the firmware land cannot be bothered with >> putting correct values into them. :-/ > > Rafael, > > Thanks for quick reply and sharing the information > > It seems static (non-configurable) devices on ACPI are discussed in > both _PRS and _PRT as below: > > 6.2.12 _PRS (Possible Resource Settings) > "... Static (non-configurable) devices do not specify a _PRS object... " > > 6.2.13 _PRT (PCI Routing Table) > "In the second model, the PCI interrupts are hardwired to specific > interrupt inputs on the interrupt controller and are not configurable. > In this case, the Source field in _PRT does not reference a device, > but instead contains the value zero, and the Source Index field > contains the global system interrupt to which the PCI interrupt is > hardwired." > > My interpretation is the both are true from ACPI's perspective, and > both should be implemented by system firmware. On this particular > system I am debugging remotely, it does the _PRS part but not _PRT, > and I will follow up with firmware engineers. I made a mistake saying firmware's _PRT part is not done. In the DSDT, the _PRT is as below: Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table { If (PICM) { Return (AR00) /* \_SB_.AR00 */ } Return (PK00) /* \_SB_.PK00 */ } When I used acpiexec to load *.dat from acpidump, it returns _SB.PK00; however, actual system should return _SB.AR00 which is defined as below: Name (AR00, Package (0x2E) { Package (0x04) { 0x0004FFFF, Zero, Zero, 0x10 }, Package (0x04) { 0x0005FFFF, Zero, Zero, 0x10 }, Package (0x04) { 0x001EFFFF, Zero, Zero, 0x14 }, Package (0x04) { 0x001EFFFF, One, Zero, 0x15 }, ... // skips more The third elements, Source, are zero and it meets ACPI's requirement. > > On the other hand, it may not be unreasonable to remove AE_NOT_FOUND > as defined in 6.2.12 in ACPI spec. I also did a code trace and it > seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of > zero in Source field in _PRT. > > Finally, this seems to be a confusing topic in ACPI spec (to the > firmware engineers and I at least). If we can figure this out, we may > want to clarify the wording with ASWG. > >> >>> Signed-off-by: Alex Hung <alex.hung@canonical.com> >>> --- >>> drivers/acpi/pci_link.c | 4 +--- >>> 1 file changed, 1 insertion(+), 3 deletions(-) >>> >>> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c >>> index 85ad679390e3..f98215971f30 100644 >>> --- a/drivers/acpi/pci_link.c >>> +++ b/drivers/acpi/pci_link.c >>> @@ -172,10 +172,8 @@ static int acpi_pci_link_get_possible(struct acpi_pci_link *link) >>> >>> status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, >>> acpi_pci_link_check_possible, link); >>> - if (ACPI_FAILURE(status)) { >>> - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); >>> + if (ACPI_FAILURE(status)) >>> return -ENODEV; >>> - } >>> >>> ACPI_DEBUG_PRINT((ACPI_DB_INFO, >>> "Found %d possible IRQs\n", >>> >> >> > > > > -- > Cheers, > Alex Hung
On Thu, Feb 1, 2018 at 5:39 AM, Alex Hung <alex.hung@canonical.com> wrote: > On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: >>> In recent Intel hardware the IRQs become non-configurable after BIOS >>> initializes them in PEI phase and _PRS objects are no longer included in >>> ASL. >>> >>> This is the same as "static (non-configurable) devices do not >>> specify a _PRS object" in ACPI spec. As a result, error messages >>> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not >>> needed. >> >> That's questionable at best. >> >> The errors basically indicate that _PRT entries corresponding to these >> IRQs are messed up (because they should contain the value of 0 instead of >> a NamePath in the Source column), so we are now going to paper over bugs >> in ACPI tables as someone in the firmware land cannot be bothered with >> putting correct values into them. :-/ > > Rafael, > > Thanks for quick reply and sharing the information > > It seems static (non-configurable) devices on ACPI are discussed in > both _PRS and _PRT as below: > > 6.2.12 _PRS (Possible Resource Settings) > "... Static (non-configurable) devices do not specify a _PRS object... " > > 6.2.13 _PRT (PCI Routing Table) > "In the second model, the PCI interrupts are hardwired to specific > interrupt inputs on the interrupt controller and are not configurable. > In this case, the Source field in _PRT does not reference a device, > but instead contains the value zero, and the Source Index field > contains the global system interrupt to which the PCI interrupt is > hardwired." > > My interpretation is the both are true from ACPI's perspective, and > both should be implemented by system firmware. On this particular > system I am debugging remotely, it does the _PRS part but not _PRT, > and I will follow up with firmware engineers. OK > On the other hand, it may not be unreasonable to remove AE_NOT_FOUND > as defined in 6.2.12 in ACPI spec. I also did a code trace and it > seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of > zero in Source field in _PRT. I'm not sure what you mean here. Do you mean that the code would mishandle 0 in the Source field of _PRT? -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 31, 2018 at 11:30 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Thu, Feb 1, 2018 at 5:39 AM, Alex Hung <alex.hung@canonical.com> wrote: >> On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >>> On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: >>>> In recent Intel hardware the IRQs become non-configurable after BIOS >>>> initializes them in PEI phase and _PRS objects are no longer included in >>>> ASL. >>>> >>>> This is the same as "static (non-configurable) devices do not >>>> specify a _PRS object" in ACPI spec. As a result, error messages >>>> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not >>>> needed. >>> >>> That's questionable at best. >>> >>> The errors basically indicate that _PRT entries corresponding to these >>> IRQs are messed up (because they should contain the value of 0 instead of >>> a NamePath in the Source column), so we are now going to paper over bugs >>> in ACPI tables as someone in the firmware land cannot be bothered with >>> putting correct values into them. :-/ >> >> Rafael, >> >> Thanks for quick reply and sharing the information >> >> It seems static (non-configurable) devices on ACPI are discussed in >> both _PRS and _PRT as below: >> >> 6.2.12 _PRS (Possible Resource Settings) >> "... Static (non-configurable) devices do not specify a _PRS object... " >> >> 6.2.13 _PRT (PCI Routing Table) >> "In the second model, the PCI interrupts are hardwired to specific >> interrupt inputs on the interrupt controller and are not configurable. >> In this case, the Source field in _PRT does not reference a device, >> but instead contains the value zero, and the Source Index field >> contains the global system interrupt to which the PCI interrupt is >> hardwired." >> >> My interpretation is the both are true from ACPI's perspective, and >> both should be implemented by system firmware. On this particular >> system I am debugging remotely, it does the _PRS part but not _PRT, >> and I will follow up with firmware engineers. > > OK > >> On the other hand, it may not be unreasonable to remove AE_NOT_FOUND >> as defined in 6.2.12 in ACPI spec. I also did a code trace and it >> seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of >> zero in Source field in _PRT. > > I'm not sure what you mean here. > > Do you mean that the code would mishandle 0 in the Source field of _PRT? I meant the AE_NOT_FOUND messages still pop up when SOURCE = 0.
On Thursday, February 1, 2018 9:07:59 AM CET Alex Hung wrote: > On Wed, Jan 31, 2018 at 11:30 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Thu, Feb 1, 2018 at 5:39 AM, Alex Hung <alex.hung@canonical.com> wrote: > >> On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > >>> On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: > >>>> In recent Intel hardware the IRQs become non-configurable after BIOS > >>>> initializes them in PEI phase and _PRS objects are no longer included in > >>>> ASL. > >>>> > >>>> This is the same as "static (non-configurable) devices do not > >>>> specify a _PRS object" in ACPI spec. As a result, error messages > >>>> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not > >>>> needed. > >>> > >>> That's questionable at best. > >>> > >>> The errors basically indicate that _PRT entries corresponding to these > >>> IRQs are messed up (because they should contain the value of 0 instead of > >>> a NamePath in the Source column), so we are now going to paper over bugs > >>> in ACPI tables as someone in the firmware land cannot be bothered with > >>> putting correct values into them. :-/ > >> > >> Rafael, > >> > >> Thanks for quick reply and sharing the information > >> > >> It seems static (non-configurable) devices on ACPI are discussed in > >> both _PRS and _PRT as below: > >> > >> 6.2.12 _PRS (Possible Resource Settings) > >> "... Static (non-configurable) devices do not specify a _PRS object... " > >> > >> 6.2.13 _PRT (PCI Routing Table) > >> "In the second model, the PCI interrupts are hardwired to specific > >> interrupt inputs on the interrupt controller and are not configurable. > >> In this case, the Source field in _PRT does not reference a device, > >> but instead contains the value zero, and the Source Index field > >> contains the global system interrupt to which the PCI interrupt is > >> hardwired." > >> > >> My interpretation is the both are true from ACPI's perspective, and > >> both should be implemented by system firmware. On this particular > >> system I am debugging remotely, it does the _PRS part but not _PRT, > >> and I will follow up with firmware engineers. > > > > OK > > > >> On the other hand, it may not be unreasonable to remove AE_NOT_FOUND > >> as defined in 6.2.12 in ACPI spec. I also did a code trace and it > >> seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of > >> zero in Source field in _PRT. > > > > I'm not sure what you mean here. > > > > Do you mean that the code would mishandle 0 in the Source field of _PRT? > > I meant the AE_NOT_FOUND messages still pop up when SOURCE = 0. OK, so why does the firmware define the link objects in the namespace then? > Do you have other comments about this patch or concerns that I can work > with firmware engineers? It looks to me that there are some PCI interrupt link objects in the namespace without _PRS and which aren't pointed to by any _PRT entries. If so, what are they useful for then? We sure may ignore such things, but the patch makes us ignore cases that are outright invalid too AFAICS. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 2, 2018 at 3:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Thursday, February 1, 2018 9:07:59 AM CET Alex Hung wrote: >> On Wed, Jan 31, 2018 at 11:30 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: >> > On Thu, Feb 1, 2018 at 5:39 AM, Alex Hung <alex.hung@canonical.com> wrote: >> >> On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> >>> On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: >> >>>> In recent Intel hardware the IRQs become non-configurable after BIOS >> >>>> initializes them in PEI phase and _PRS objects are no longer included in >> >>>> ASL. >> >>>> >> >>>> This is the same as "static (non-configurable) devices do not >> >>>> specify a _PRS object" in ACPI spec. As a result, error messages >> >>>> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not >> >>>> needed. >> >>> >> >>> That's questionable at best. >> >>> >> >>> The errors basically indicate that _PRT entries corresponding to these >> >>> IRQs are messed up (because they should contain the value of 0 instead of >> >>> a NamePath in the Source column), so we are now going to paper over bugs >> >>> in ACPI tables as someone in the firmware land cannot be bothered with >> >>> putting correct values into them. :-/ >> >> >> >> Rafael, >> >> >> >> Thanks for quick reply and sharing the information >> >> >> >> It seems static (non-configurable) devices on ACPI are discussed in >> >> both _PRS and _PRT as below: >> >> >> >> 6.2.12 _PRS (Possible Resource Settings) >> >> "... Static (non-configurable) devices do not specify a _PRS object... " >> >> >> >> 6.2.13 _PRT (PCI Routing Table) >> >> "In the second model, the PCI interrupts are hardwired to specific >> >> interrupt inputs on the interrupt controller and are not configurable. >> >> In this case, the Source field in _PRT does not reference a device, >> >> but instead contains the value zero, and the Source Index field >> >> contains the global system interrupt to which the PCI interrupt is >> >> hardwired." >> >> >> >> My interpretation is the both are true from ACPI's perspective, and >> >> both should be implemented by system firmware. On this particular >> >> system I am debugging remotely, it does the _PRS part but not _PRT, >> >> and I will follow up with firmware engineers. >> > >> > OK >> > >> >> On the other hand, it may not be unreasonable to remove AE_NOT_FOUND >> >> as defined in 6.2.12 in ACPI spec. I also did a code trace and it >> >> seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of >> >> zero in Source field in _PRT. >> > >> > I'm not sure what you mean here. >> > >> > Do you mean that the code would mishandle 0 in the Source field of _PRT? >> >> I meant the AE_NOT_FOUND messages still pop up when SOURCE = 0. > > OK, so why does the firmware define the link objects in the namespace then? > >> Do you have other comments about this patch or concerns that I can work >> with firmware engineers? > > It looks to me that there are some PCI interrupt link objects in the > namespace without _PRS and which aren't pointed to by any _PRT entries. > > If so, what are they useful for then? The LNKA~LNKD used in Name(PK00) are used when PIC mode is used, ex. _PIC(0). Disassembled ASL code is as below: Method (_PIC, 1, NotSerialized) // _PIC: Interrupt Model { GPIC = Arg0 PICM = Arg0 } Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table { If (PICM) { Return (AR00) /* \_SB_.AR00 */ } Return (PK00) /* \_SB_.PK00 */ } When the default APIC mode is used, Name(AR00) is reported, as below: Name (AR00, Package (0x2E) { Package (0x04) { 0x0004FFFF, Zero, Zero, 0x10 }, Package (0x04) { 0x0005FFFF, Zero, Zero, 0x10 }, // more are skipped.. } > > We sure may ignore such things, but the patch makes us ignore cases that > are outright invalid too AFAICS. How about skipping when status == AE_NOT_FOUND, like - if (ACPI_FAILURE(status)) { + if (status != AE_NOT_FOUND && ACPI_FAILURE(status)) > > Thanks, > Rafael > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, February 6, 2018 6:59:34 AM CET Alex Hung wrote: > On Fri, Feb 2, 2018 at 3:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Thursday, February 1, 2018 9:07:59 AM CET Alex Hung wrote: > >> On Wed, Jan 31, 2018 at 11:30 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: > >> > On Thu, Feb 1, 2018 at 5:39 AM, Alex Hung <alex.hung@canonical.com> wrote: > >> >> On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > >> >>> On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: > >> >>>> In recent Intel hardware the IRQs become non-configurable after BIOS > >> >>>> initializes them in PEI phase and _PRS objects are no longer included in > >> >>>> ASL. > >> >>>> > >> >>>> This is the same as "static (non-configurable) devices do not > >> >>>> specify a _PRS object" in ACPI spec. As a result, error messages > >> >>>> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not > >> >>>> needed. > >> >>> > >> >>> That's questionable at best. > >> >>> > >> >>> The errors basically indicate that _PRT entries corresponding to these > >> >>> IRQs are messed up (because they should contain the value of 0 instead of > >> >>> a NamePath in the Source column), so we are now going to paper over bugs > >> >>> in ACPI tables as someone in the firmware land cannot be bothered with > >> >>> putting correct values into them. :-/ > >> >> > >> >> Rafael, > >> >> > >> >> Thanks for quick reply and sharing the information > >> >> > >> >> It seems static (non-configurable) devices on ACPI are discussed in > >> >> both _PRS and _PRT as below: > >> >> > >> >> 6.2.12 _PRS (Possible Resource Settings) > >> >> "... Static (non-configurable) devices do not specify a _PRS object... " > >> >> > >> >> 6.2.13 _PRT (PCI Routing Table) > >> >> "In the second model, the PCI interrupts are hardwired to specific > >> >> interrupt inputs on the interrupt controller and are not configurable. > >> >> In this case, the Source field in _PRT does not reference a device, > >> >> but instead contains the value zero, and the Source Index field > >> >> contains the global system interrupt to which the PCI interrupt is > >> >> hardwired." > >> >> > >> >> My interpretation is the both are true from ACPI's perspective, and > >> >> both should be implemented by system firmware. On this particular > >> >> system I am debugging remotely, it does the _PRS part but not _PRT, > >> >> and I will follow up with firmware engineers. > >> > > >> > OK > >> > > >> >> On the other hand, it may not be unreasonable to remove AE_NOT_FOUND > >> >> as defined in 6.2.12 in ACPI spec. I also did a code trace and it > >> >> seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of > >> >> zero in Source field in _PRT. > >> > > >> > I'm not sure what you mean here. > >> > > >> > Do you mean that the code would mishandle 0 in the Source field of _PRT? > >> > >> I meant the AE_NOT_FOUND messages still pop up when SOURCE = 0. > > > > OK, so why does the firmware define the link objects in the namespace then? > > > >> Do you have other comments about this patch or concerns that I can work > >> with firmware engineers? > > > > It looks to me that there are some PCI interrupt link objects in the > > namespace without _PRS and which aren't pointed to by any _PRT entries. > > > > If so, what are they useful for then? > > The LNKA~LNKD used in Name(PK00) are used when PIC mode is used, ex. > _PIC(0). I see. OK, in that case I'd just change the log level of the message to "debug", and use acpi_handle_debug() for printing it for that matter. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 8, 2018 at 1:55 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Tuesday, February 6, 2018 6:59:34 AM CET Alex Hung wrote: >> On Fri, Feb 2, 2018 at 3:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> > On Thursday, February 1, 2018 9:07:59 AM CET Alex Hung wrote: >> >> On Wed, Jan 31, 2018 at 11:30 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: >> >> > On Thu, Feb 1, 2018 at 5:39 AM, Alex Hung <alex.hung@canonical.com> wrote: >> >> >> On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> >> >>> On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: >> >> >>>> In recent Intel hardware the IRQs become non-configurable after BIOS >> >> >>>> initializes them in PEI phase and _PRS objects are no longer included in >> >> >>>> ASL. >> >> >>>> >> >> >>>> This is the same as "static (non-configurable) devices do not >> >> >>>> specify a _PRS object" in ACPI spec. As a result, error messages >> >> >>>> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not >> >> >>>> needed. >> >> >>> >> >> >>> That's questionable at best. >> >> >>> >> >> >>> The errors basically indicate that _PRT entries corresponding to these >> >> >>> IRQs are messed up (because they should contain the value of 0 instead of >> >> >>> a NamePath in the Source column), so we are now going to paper over bugs >> >> >>> in ACPI tables as someone in the firmware land cannot be bothered with >> >> >>> putting correct values into them. :-/ >> >> >> >> >> >> Rafael, >> >> >> >> >> >> Thanks for quick reply and sharing the information >> >> >> >> >> >> It seems static (non-configurable) devices on ACPI are discussed in >> >> >> both _PRS and _PRT as below: >> >> >> >> >> >> 6.2.12 _PRS (Possible Resource Settings) >> >> >> "... Static (non-configurable) devices do not specify a _PRS object... " >> >> >> >> >> >> 6.2.13 _PRT (PCI Routing Table) >> >> >> "In the second model, the PCI interrupts are hardwired to specific >> >> >> interrupt inputs on the interrupt controller and are not configurable. >> >> >> In this case, the Source field in _PRT does not reference a device, >> >> >> but instead contains the value zero, and the Source Index field >> >> >> contains the global system interrupt to which the PCI interrupt is >> >> >> hardwired." >> >> >> >> >> >> My interpretation is the both are true from ACPI's perspective, and >> >> >> both should be implemented by system firmware. On this particular >> >> >> system I am debugging remotely, it does the _PRS part but not _PRT, >> >> >> and I will follow up with firmware engineers. >> >> > >> >> > OK >> >> > >> >> >> On the other hand, it may not be unreasonable to remove AE_NOT_FOUND >> >> >> as defined in 6.2.12 in ACPI spec. I also did a code trace and it >> >> >> seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of >> >> >> zero in Source field in _PRT. >> >> > >> >> > I'm not sure what you mean here. >> >> > >> >> > Do you mean that the code would mishandle 0 in the Source field of _PRT? >> >> >> >> I meant the AE_NOT_FOUND messages still pop up when SOURCE = 0. >> > >> > OK, so why does the firmware define the link objects in the namespace then? >> > >> >> Do you have other comments about this patch or concerns that I can work >> >> with firmware engineers? >> > >> > It looks to me that there are some PCI interrupt link objects in the >> > namespace without _PRS and which aren't pointed to by any _PRT entries. >> > >> > If so, what are they useful for then? >> >> The LNKA~LNKD used in Name(PK00) are used when PIC mode is used, ex. >> _PIC(0). > > I see. > > OK, in that case I'd just change the log level of the message to "debug", > and use acpi_handle_debug() for printing it for that matter. > Hi Rafael, Do you mean something like the below change? if (ACPI_FAILURE(status)) { - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); + acpi_handle_debug(link->device->handle, "failed to evaluate _PRS"); return -ENODEV; } If so, I will submit a V2 accordingly. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 9, 2018 at 10:02 AM, Alex Hung <alex.hung@canonical.com> wrote: > On Thu, Feb 8, 2018 at 1:55 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> On Tuesday, February 6, 2018 6:59:34 AM CET Alex Hung wrote: >>> On Fri, Feb 2, 2018 at 3:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >>> > On Thursday, February 1, 2018 9:07:59 AM CET Alex Hung wrote: >>> >> On Wed, Jan 31, 2018 at 11:30 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: >>> >> > On Thu, Feb 1, 2018 at 5:39 AM, Alex Hung <alex.hung@canonical.com> wrote: >>> >> >> On Wed, Jan 31, 2018 at 1:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >>> >> >>> On Wednesday, January 31, 2018 6:52:19 AM CET Alex Hung wrote: >>> >> >>>> In recent Intel hardware the IRQs become non-configurable after BIOS >>> >> >>>> initializes them in PEI phase and _PRS objects are no longer included in >>> >> >>>> ASL. >>> >> >>>> >>> >> >>>> This is the same as "static (non-configurable) devices do not >>> >> >>>> specify a _PRS object" in ACPI spec. As a result, error messages >>> >> >>>> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not >>> >> >>>> needed. >>> >> >>> >>> >> >>> That's questionable at best. >>> >> >>> >>> >> >>> The errors basically indicate that _PRT entries corresponding to these >>> >> >>> IRQs are messed up (because they should contain the value of 0 instead of >>> >> >>> a NamePath in the Source column), so we are now going to paper over bugs >>> >> >>> in ACPI tables as someone in the firmware land cannot be bothered with >>> >> >>> putting correct values into them. :-/ >>> >> >> >>> >> >> Rafael, >>> >> >> >>> >> >> Thanks for quick reply and sharing the information >>> >> >> >>> >> >> It seems static (non-configurable) devices on ACPI are discussed in >>> >> >> both _PRS and _PRT as below: >>> >> >> >>> >> >> 6.2.12 _PRS (Possible Resource Settings) >>> >> >> "... Static (non-configurable) devices do not specify a _PRS object... " >>> >> >> >>> >> >> 6.2.13 _PRT (PCI Routing Table) >>> >> >> "In the second model, the PCI interrupts are hardwired to specific >>> >> >> interrupt inputs on the interrupt controller and are not configurable. >>> >> >> In this case, the Source field in _PRT does not reference a device, >>> >> >> but instead contains the value zero, and the Source Index field >>> >> >> contains the global system interrupt to which the PCI interrupt is >>> >> >> hardwired." >>> >> >> >>> >> >> My interpretation is the both are true from ACPI's perspective, and >>> >> >> both should be implemented by system firmware. On this particular >>> >> >> system I am debugging remotely, it does the _PRS part but not _PRT, >>> >> >> and I will follow up with firmware engineers. >>> >> > >>> >> > OK >>> >> > >>> >> >> On the other hand, it may not be unreasonable to remove AE_NOT_FOUND >>> >> >> as defined in 6.2.12 in ACPI spec. I also did a code trace and it >>> >> >> seems that the AE_NOT_FOUND in _PRS cannot be removed by a value of >>> >> >> zero in Source field in _PRT. >>> >> > >>> >> > I'm not sure what you mean here. >>> >> > >>> >> > Do you mean that the code would mishandle 0 in the Source field of _PRT? >>> >> >>> >> I meant the AE_NOT_FOUND messages still pop up when SOURCE = 0. >>> > >>> > OK, so why does the firmware define the link objects in the namespace then? >>> > >>> >> Do you have other comments about this patch or concerns that I can work >>> >> with firmware engineers? >>> > >>> > It looks to me that there are some PCI interrupt link objects in the >>> > namespace without _PRS and which aren't pointed to by any _PRT entries. >>> > >>> > If so, what are they useful for then? >>> >>> The LNKA~LNKD used in Name(PK00) are used when PIC mode is used, ex. >>> _PIC(0). >> >> I see. >> >> OK, in that case I'd just change the log level of the message to "debug", >> and use acpi_handle_debug() for printing it for that matter. >> > > Hi Rafael, > > Do you mean something like the below change? > > if (ACPI_FAILURE(status)) { > - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); > + acpi_handle_debug(link->device->handle, "failed to > evaluate _PRS"); > return -ENODEV; > } > > If so, I will submit a V2 accordingly. Yes, please. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 85ad679390e3..f98215971f30 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -172,10 +172,8 @@ static int acpi_pci_link_get_possible(struct acpi_pci_link *link) status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, acpi_pci_link_check_possible, link); - if (ACPI_FAILURE(status)) { - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); + if (ACPI_FAILURE(status)) return -ENODEV; - } ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d possible IRQs\n",
In recent Intel hardware the IRQs become non-configurable after BIOS initializes them in PEI phase and _PRS objects are no longer included in ASL. This is the same as "static (non-configurable) devices do not specify a _PRS object" in ACPI spec. As a result, error messages saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" are not needed. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- drivers/acpi/pci_link.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)