Re: [PATCH v7 2/5] PCI/DPC: Run recovery on device that detected the error

Shuai Xue
2026-02-06T08:42:30Z

On 2/2/26 10:02 PM, Lukas Wunner wrote:
> On Sat, Jan 24, 2026 at 03:45:54PM +0800, Shuai Xue wrote:
>> +++ b/drivers/pci/pcie/dpc.c
>> @@ -372,10 +388,11 @@ static irqreturn_t dpc_handler(int irq, void *context)
>> return IRQ_HANDLED;
>> }
>>
>> - dpc_process_error(err_port);
>> + err_dev = dpc_process_error(err_port);
>>
>> /* We configure DPC so it only triggers on ERR_FATAL */
>> - pcie_do_recovery(err_port, pci_channel_io_frozen, dpc_reset_link);
>> + pcie_do_recovery(err_dev, pci_channel_io_frozen, dpc_reset_link);
>> + pci_dev_put(err_dev);
>>
>> return IRQ_HANDLED;
>> }
>
> You're assuming that the parent of the Requester is always identical
> to the containing Downstream Port. But that's not necessarily the case:
>
> E.g., imagine a DPC-capable Root Port with a PCIe switch below
> whose Downstream Ports are not DPC-capable. Let's say an Endpoint
> beneath the PCIe switch sends ERR_FATAL upstream. AFAICS, your patch
> will cause pcie_do_recovery() to invoke dpc_reset_link() with the
> Downstream Port of the PCIe switch as argument. That function will
> then happily use pdev->dpc_cap even though it's 0.

I see. Goot point.

>
> A possible solution may be to amend dpc_reset_link() to walk upwards,
> starting at pdev, and search for a pci_dev whose pdev->dpc_cap is
> non-zero. That should be the containing DPC-capable port.

See my reply in your later patch.

>
> I don't really like the err_dev and err_port terminology invented here.
> We generally use spec terms to make it easy to connect the code to the
> spec. The spec uses the term "Downstream Port" to denote the containing
> Downstream Port, so a name such as "dport" may be better than "err_port".
>
>
> Similarly, the spec uses "Requester" or "Source" for the reporting agent,
> so "req", "requester", "src" or "source" may all be better names than
> "err_dev".

Sure, I wll use the common "dport" and "req" term from spec to make it more readable.

>
> Thanks,
>
> Lukas

Thanks for valuable comments.
Shuai