Re: [PATCH v3] drivers/pci: Decouple DPC from AER service

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Jul 10, 2026 at 12:00:57PM -0500, Bjorn Helgaas wrote:
> On Thu, Dec 11, 2025 at 04:42:53PM +0000, Darshit Shah wrote:
> > According to PCIe r7.0, sec. 6.2.11, "Implementation Note: Determination
> > of DPC Control", it is recommended that the Operating System link the
> > enablement of Downstream Port Containment (DPC) to the enablement of
> > Advanced Error Reporting (AER).
> > 
> > However, AER is advertised only on Root Port (RP) or Root Complex Event
> > Collector (RCEC) devices. On the other hand, DPC may be advertised on
> > any PCIe device in the hierarchy. In fact, since the main usecase of DPC
> > is for the switch upstream of an endpoint device to trigger a signal to
> > the host-bridge, it is imperative that it be supported on non-RP,
> > non-RCEC devices.
> > 
> > Previously portdrv has interpreted the spec to mean that the AER service
> > must be available on the same device in order for DPC to be available.
> > This is not what the implementation note meant to imply. If the firmware
> > hands Linux control of AER via _OSC on the host bridge upstream of the
> > device, then Linux should be allowed to assume control of DPC on the device.
> > 
> > The comment above this check alludes to this, by saying:
> >   > With dpc-native, allow Linux to use DPC even if it doesn't have
> >   > permission to use AER.
> > 
> > However, permission to use AER is negotiated at the host bridge, not
> > per-device. So we should not link DPC to enabling AER at the device.
> > Instead, DPC should be enabled if the OS has control of AER for the
> > host bridge that is upstream of the device in question, or if dpc-native
> > was set on the command line.
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Darshit Shah <darnshah@amazon.de>
> > Reviewed-by: Lukas Wunner <lukas@wunner.de>
> > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Sorry for the delay in picking this up.  Thanks for doing this; I
> think it's the right thing.  I think the commit log is not completely
> accurate in terms on which devices advertise AER and DPC, so I propose
> the following.  I also reordered the || operands to make the patch
> easier to read.  Your thoughts?

Path below applied to pci/dpc for v7.3, thanks, Darshit!

I'm actually a little surprised that this wasn't an issue earlier.  I
guess DPC hasn't really been exercised on switches.

> commit 97ca178c899d ("PCI/DPC: Allow DPC on all Downstream Ports when OS controls AER")
> Author: Darshit Shah <darnshah@amazon.de>
> Date:   Thu Dec 11 16:42:53 2025 +0000
> 
>     PCI/DPC: Allow DPC on all Downstream Ports when OS controls AER
>     
>     PCIe r7.0, sec 6.2.11, "Implementation Note: Determination of DPC Control",
>     recommends that "... operating systems always link control of DPC to the
>     control of Advanced Error Reporting."
>     
>     Any PCIe device may advertise AER, but only Root Ports and Root Complex
>     Event Collectors can generate AER interrupts, so the AER driver only binds
>     to RPs and RCECs.
>     
>     Any Root Port or Switch Downstream Port may advertise Downstream Port
>     Containment (DPC), but previously the DPC driver was limited to devices the
>     AER driver could bind to, i.e., only RPs that advertised AER.
>     
>     Since any Port with DPC can generate DPC interrupts, allow the DPC driver
>     to bind to such a Port as long as the OS controls AER, regardless of
>     whether the AER driver binds to it.
>     
>     Signed-off-by: Darshit Shah <darnshah@amazon.de>
>     [bhelgaas: commit log, reorder || operands to simplify patch]
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>     Reviewed-by: Lukas Wunner <lukas@wunner.de>
>     Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>     Cc: stable@vger.kernel.org
>     Link: https://patch.msgid.link/20251211164257.81655-1-darnshah@amazon.de
> 
> 
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index 2d6aa488fe7b..65f502602dee 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -264,7 +264,7 @@ static int get_port_device_capability(struct pci_dev *dev)
>  	 */
>  	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
>  	    pci_aer_available() &&
> -	    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
> +	    (pcie_ports_dpc_native || host->native_aer))
>  		services |= PCIE_PORT_SERVICE_DPC;
>  
>  	/* Enable bandwidth control if more than one speed is supported. */