On Tue, Feb 13, 2018 at 5:03 AM, Alex Hung <alex.hung@xxxxxxxxxxxxx> wrote:
> On Mon, Feb 12, 2018 at 12:51 AM, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>> On Sat, Feb 10, 2018 at 4:34 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>>> On Sat, Feb 10, 2018 at 09:08:42AM +0100, Rafael J. Wysocki wrote:
>>>> On Sat, Feb 10, 2018 at 2:05 AM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>>>> > On Fri, Feb 09, 2018 at 02:56:43PM -0800, 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" does not need to
>>>> >> be in kernel messages all the time but only when debug is enabled.
>>>> >
>>>> > I agree and would even go further: _PRS is optional and I don't think
>>>> > there's a reason to log anything at all if it's absent. A log message
>>>> > like "failed to evaluate _PRS" makes people think something is wrong
>>>> > when in fact nothing is wrong.
>>>>
>>>> _PRS is required if the link object is pointed to by a _PRT entry.
>>>
>>> The closest thing I see with a quick look is ACPI 6.0, sec 6.2.13:
>>>
>>> These objects [PCI Interrupt Link Devices] have _PRS, _CRS, _SRS,
>>> and _DIS control methods to allocate the interrupt.
>>>
>>> I don't read that as a requirement for _PRS in particular; I read it
>>> as saying that the interrupt link devices use the normal device
>>> configuration method, i.e., _CRS is required and _PRS and _SRS are
>>> optional and needed for configurable devices but not for static ones.
>>
>> Well, that's slightly out of context. :-)
>>
>> First of all, Section 6.2.13 says this: "Typically, the interrupt
>> input that a given PCI interrupt is on is configurable. [...] In this
>> model, each interrupt is represented in the ACPI namespace as a PCI
>> Interrupt Link Device." Then, it says the above.
>>
>> Now, if _PRS is not present, the IRQ represented by the link object
>> cannot be configured, so in fact the underlying assumption doesn't
>> apply to it, strictly speaking. It follows from the last paragraph in
>> Section 6.2.13 that _PRT entries representing such IRQs should not
>> point to interrupt link device objects (there should be 0 in their
>> Source fields).
>>
>> Hence, if a _PRT entry points to an interrupt link device object in
>> the namespace, _PRS should be present under this object or at least it
>> is reasonable to expect that it will be present in there.
>>
>>> But this is just a drive-by comment and I'm really fine with whatever
>>> you decide to do.
>>
>> OK
>>
>> So I think that (a) the message should be printed using
>> acpi_handle_debug(), except that I would make it slightly more
>> neutral, something like "_PRS not present or invalid", and (b) 0
>> should be returned instead of -ENODEV when _PRS evaluation doesn't
>> succeed.
>
> Looks like we will do the following.
>
> if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS"));
> - return -ENODEV;
> + acpi_handle_debug(link->device->handle, "_PRS not
> present or invalid");
> + return 0;
Yes, but this affects the caller.
Please make sure that everything will work correctly in there after this change.
