On 1/10/2024 8:41 AM, Kuppuswamy Sathyanarayanan wrote: > > > 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? > Hi Bjorn, Since requesting DPC control needs both EDR and DPC support in kernel, I am wondering whether we need two different configs for it. IMO, it makes sense to merge them under one config. I don't see EDR working without DPC support. Since DPC control also need support for EDR, I think we don't need to differentiate them. What you think? file: drivers/acpi/pci_root.c + /* + * Per the Downstream Port Containment Related Enhancements ECN to + * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5, + * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC + * and EDR. + */ + if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR)) + control |= OSC_PCI_EXPRESS_DPC_CONTROL; >> 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
