Re: [PATCH 1/1] PCI/portdrv: Allow DPC if the OS controls AER natively.

Kuppuswamy Sathyanarayanan
2024-01-08T19:53:58Z

On 1/8/2024 11:46 AM, Matthew W Carlis wrote:
> Hello again, sorry for the delayed response.. I have been on PTO. The above patch
> doesn't fix the problem in our systems as host->native_dpc is not set due to
> not using or having support for Error Disconnect Recovery (EDR). I wonder if

The only condition we check before requesting DPC control is whether OS
supports both EDR and DPC. As long as you have enabled related configs
(IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR)), this will be true.
Enabling these configs does not mean you are using it. Any reason why you don't
enable it in your case?

> host->native_dpc is a misleading name a way... Misleading in the sense that
> setting host->native_aer implies firmware intends the OS to control AER, whereas
> host->native_dpc being set appears to have an additional requirement on the
> use/support of EDR in addition to DPC. When I was working on the patch as
> submitted I had been thinking about all of these fields & my thinking was
> as follows.. The kernel requires host->native_aer in order to control AER, but
> it could control DPC whether host->native_dpc is set or unset. Therefore, if
> the kernel will control AER it should also control DPC on any capable devices.
> Of course there is also the requirement of having built with CONFIG_PCIE_AER
> & CONFIG_PCIE_DPC. Please advise if my understanding of all this is incorrect..
>
> Thanks,
> -Matt
>
> I included an update to the patch submitted in chain which should remove the
> build error that occured when CONFIG_PCIE_AER was not set. Including it
> in case my understanding of EDR/DPC/etc is correct.
>
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index 14a4b89a3b83..2fc006f12988 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -257,12 +257,18 @@ static int get_port_device_capability(struct pci_dev *dev)
> }
>
> /*
> + * _OSC AER Control is required by the OS & requires OS to control AER,
> + * but _OSC DPC Control isn't required by the OS to control DPC; however
> + * it does require the OS to control DPC. _OSC DPC Control also requres
> + * _OSC EDR Control (Error Disconnect Recovery) (PCI Firmware - DPC ECN rev3.2)
> + * PCI_Express_Base 6.1, 6.2.11 Determination of DPC Control recommends
> + * platform fw or OS always link control of DPC to AER.
> + *
> * With dpc-native, allow Linux to use DPC even if it doesn't have
> * permission to use AER.
> */
> if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> - pci_aer_available() &&
> - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
> + pci_aer_available() && (pcie_ports_dpc_native || host->native_aer))
> services |= PCIE_PORT_SERVICE_DPC;
>
> if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM ||
>

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer