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

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

On 2/12/2026 11:18 AM, Danielle Costantino wrote:
> In the native DPC interrupt path, dpc_irq() clears
> PCI_EXP_DPC_STATUS_INTERRUPT before scheduling the threaded handler
> that eventually calls dpc_reset_link().  However, in the firmware-first
> EDR path, dpc_irq() is never invoked -- firmware owns the DPC interrupt
> and notifies the OS via an ACPI EDR notification.  dpc_reset_link() is
> then called directly from edr_handle_event() via pcie_do_recovery().
> 
> Because dpc_reset_link() only clears PCI_EXP_DPC_STATUS_TRIGGER, the
> Interrupt Status bit (bit 3) is left set permanently after every EDR
> event.
> 
> Clear PCI_EXP_DPC_STATUS_INTERRUPT alongside PCI_EXP_DPC_STATUS_TRIGGER
> in dpc_reset_link().  Both bits are RW1C in the DPC Status register per
> PCIe r6.1, sec 7.9.14.5, so writing them together is safe.  The native
> path is unaffected because dpc_irq() has already cleared the Interrupt
> Status bit before dpc_reset_link() runs.
> 

In native path (dpc_irq()), OS owns the interrupt delivery and handling.
So it clears it in dpc_irq() handler.

In case of EDR, firmware owns the interrupt handling. It just uses ACPI
method to request OS help with recovery. Since interrupt handling is
owned by firmware, I think firmware should clear the interrupt status.

> Fixes: aea47413e7ce ("PCI/DPC: Expose dpc_process_error(), dpc_reset_link() for use by EDR")
> Signed-off-by: Danielle Costantino <dcostantino@meta.com>
> ---
>  drivers/pci/pcie/dpc.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index fc18349614d7..9baa2345e33e 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -171,8 +171,16 @@ pci_ers_result_t dpc_reset_link(struct pci_dev *pdev)
>  		goto out;
>  	}
>  
> +	/*
> +	 * Clear both DPC Trigger Status and DPC Interrupt Status.  In the
> +	 * native DPC path, dpc_irq() already clears Interrupt Status before
> +	 * the threaded handler runs.  But in the EDR (firmware-first) path,
> +	 * dpc_irq() is never called, so Interrupt Status must be cleared
> +	 * here to prevent it from remaining stale indefinitely.
> +	 */
>  	pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
> -			      PCI_EXP_DPC_STATUS_TRIGGER);
> +			      PCI_EXP_DPC_STATUS_TRIGGER |
> +			      PCI_EXP_DPC_STATUS_INTERRUPT);
>  
>  	if (pci_bridge_wait_for_secondary_bus(pdev, "DPC")) {
>  		clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags);

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer