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

Kuppuswamy Sathyanarayanan
2024-01-10T16:41:27Z

On 1/8/2024 4:15 PM, Matthew W Carlis wrote:
> A small part is probably historical; we've been using DPC on PCIe switches
> since before there was any EDR support in the kernel. It looks like there
> was a PCIe DPC ECN as early as Feb 2012, but this EDR/DPC fw ECN didn't come in
> till Jan 2019 & kernel support for ECN was even later. Its not immediately
> clear I would want to use EDR in my newer architecures & then there are also
> the older architecures still requiring support. When I submitted this patch I
> came at it with the approach of trying to keep the old behavior & still support
> the newer EDR behavior. Bjorns patch from Dec 28 2023 would seem to change

Just advertising the support for EDR in OS should not change any functional
behavior. EDR will be used only if your firmware take DPC control and sends
EDR notification. Since your kernel has EDR source support, why not enable
the relevant config? or did I not understand the issue correctly?

> the behavior for both root ports & switch ports, requiring them to set
> _OSC Control Field bit 7 (DPC) and _OSC Support Field bit 7 (EDR) or a kernel
> command line value. I think no matter what, we want to ensure that PCIe Root
> Ports and PCIe switches arrive at the same policy here.
>
> Should we consider CONFIG_PCIEAER or CONFIG_PCIEDPC as any amount of directive
> for the OS to use AER/DPC? In addition we have kernel command line arguments

No, I don't think we should use CONFIG options in actual support check.

> for pcieports=(compat/native/dpc-native) and pci=noaer. There are perhaps some
> others I'm not aware of. Then, there are the PCIe capabilities of the devies
> & bios settings for AER FW/OS controls, etc. I'm not sure if it strikes me as the
> right thing to now require users to specify additional fields to use DPC when
> they had been using it happily before.
>
> Perhaps the condition should be:
>> 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 && !IS_ENABLED(CONFIG_PCIE_EDR))))
>
> i.e: Use DPC if we set the command line argument or use DPC if we are are using
> EDR's _OSC DPC field, or use DPC if we have AER & there isn't EDR support?
>

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer