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

[Bjorn Helgaas]

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@xxxxxxxxxxxxxxx
> Signed-off-by: Darshit Shah <darnshah@xxxxxxxxx>
> Reviewed-by: Lukas Wunner <lukas@xxxxxxxxx>
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>

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?


commit 97ca178c899d ("PCI/DPC: Allow DPC on all Downstream Ports when OS controls AER")
Author: Darshit Shah <darnshah@xxxxxxxxx>
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@xxxxxxxxx>
    [bhelgaas: commit log, reorder || operands to simplify patch]
    Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
    Reviewed-by: Lukas Wunner <lukas@xxxxxxxxx>
    Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
    Cc: stable@xxxxxxxxxxxxxxx
    Link: https://patch.msgid.link/20251211164257.81655-1-darnshah@xxxxxxxxx


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. */