Re: [PATCH 1/2] PCI/DPC: Clear Interrupt Status in dpc_reset_link()

[Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>]

Hi All,

Please ignore my previous emails. For some reason, when I was typing the reply, it seems to have sent it.

On 2/12/26 8:22 PM, Sathyanarayanan Kuppuswamy wrote:
>
> Hi Keith,
>
> On 2/12/26 5:22 PM, Keith Busch wrote:
>> On Thu, Feb 12, 2026 at 02:51:46PM -0800, Kuppuswamy Sathyanarayanan wrote:
>>> On 2/12/2026 2:12 PM, Keith Busch wrote:
>>>
>>> As per EDR flow, firmware waits for _OST reply from OS to complete the
>>> current interrupt handling. After receiving _OST, firmware decides whether
>>> recovery should continue or if the link should be disabled. When/how firmware
>>> handles subsequent DPC events depends on firmware's implementation.
>> You at least agree the OS controls the "Trigger Status". That controls
>> whether the link stays contained or not, but now you're saying the
>> firmware gets to yank the link after the OS returns _OST success?
>> There's no flow in the spec suggesting any such behavior.
>
> I was referring to the flow chart and notes on page 85, in PCI firmware spec,
> v3.3, which mention firmware can choose to disable the link in certain
> cases (notes 3,5).
>
>>>> Sure, there's no explicit language in any spec I can find that the OS
>>>> must write 1 to bit 3 of the status register, but it doesn't say
>>>> firmware owns that bit either. The firmware handed control of the status
>>>> to the OS in this path. It would not make sense to return to firmware in
>>>> a state that makes it impossible to report new errors during that
>>>> transition window.
>>> The spec explicitly lists what the OS can write during the EDR window. For
>>> few registers it gives full contro; For DPC status, it explicitly states
>>> which bit it can clear (DPC trigger status).
>> What harm do you think will come if we return the dpc status register to
>> firmware in a state that indicates the OS handled it to completion?
>>
>>>> Besides, is firmware first even triggering off an interrupt? Pretty sure
>>>> it's triggering off the ERR_COR message, no? Why would it need to own
>>>> the Interrupt Status bit when it's not even relying on it?
>>> If it not triggered by interrupt, then interrupt status bit does not
>>> matter (even for OS handler).
>> Emperically speaking, yes, it does matter. Otherwise why would this
>> patch exist?
>
> What's the actual symptom being observed? Is the Interrupt Status bit
> being left set causing a functional problem, or is this preventive cleanup?
>
>> Or the other way, if you think it doesn't matter, then why oppose
>> improving Linux hardware interop?
>
> I'm not opposing the change. I want to make sure we align with the
> spec and not break any firmware expectations.
>
> If firmware is the final decision maker of the recovery flow (as shown in
> the flow chart), then does this change really help early re-handling of DPC
> events?
>
> Another point I am concerned, If there currently exits a firmware
> implementation, which  assumes firmware own the decision on when to
> re-handle DPC events. we
>  not want handle DPC again before
> completing the current event and makes a decision on whether to recover
> , then this change might break it.
>
> If you think it is better for OS to clear it, then I think we should attempt to
> clarify that part in spec as a follow up to make sure we set clear expectation
> to the firmware.
>
> -- 
> Sathyanarayanan Kuppuswamy
> Linux Kernel Developer

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer