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

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

Hi,

On 12/8/25 3:25 AM, Darshit Shah wrote:
> According to [1] 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 [1] 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 in [1] meant to imply. If the firmware
> hands the OS control of AER via _OSC on the host bridge upstream of the
> device, then the OS should be allowed to assume control of DPC on that 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.
>
> [1]: PCI Express Base Specification Revision 5.0 Version 1.0, Sec. 6.2.10
>
> Signed-off-by: Darshit Shah <darnshah@amazon.de>
> ---

Looks fine to me.

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

> Changes in patch v2:
>    * Don't touch the PCIE_PORT_SERVICE_AER attachment
>    * Stop relying on PCIE_PORT_SERVICE_AER for enabling DPC
>    * Instead test that OS has control of AER at parent host bridge
>
>   drivers/pci/pcie/portdrv.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index 38a41ccf79b9..8db2fa140ae2 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)))
> +	    (host->native_aer || pcie_ports_dpc_native))
>   		services |= PCIE_PORT_SERVICE_DPC;
>   
>   	/* Enable bandwidth control if more than one speed is supported. */

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer