On Thu, Dec 14, 2023 at 3:22 PM Matthew Carlis <mattc@xxxxxxxxxxxxxxx> wrote: > > Hi Bjorn. > > Thank you for the quick response, it looks correct that this is first in v6.2. My thinking is that the kernel should use DPC on a switch port if it would use it on a root port when dpc-native is not set. I would be happy to post a formal patch for this. Maybe using host->native_aer is the correct way to ensure that the kernel in this system will be using AER, maybe not on this device but it will on some device. Then, we can proceed to use DPC on the device. > > I will submit something in the next few days here. I think your message got rejected from the mailing lists because they only accept plain-text email: http://vger.kernel.org/majordomo-info.html More importantly, I forgot that there is a native_dpc flag, too, so it's not clear that testing native_aer is the right thing here. If native_aer *is* the right thing, it would certainly need a comment because it would look like a typo. I haven't investigated enough to know what the right answer is. Bjorn > On Thu, Dec 14, 2023 at 12:29 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: >> >> On Wed, Dec 13, 2023 at 11:37:17PM -0700, Matthew W Carlis wrote: >> > Hello Any Interested >> > >> > Recently found that this patch had the affect of requiring us to set >> > pcie_ports_dpc_native in order to use the kernel DPC driver with PCIe switch >> > downstream ports. The kernel check for the DPC capability in portdrv.c has; >> > if pci_aer_available() and (dpc-native or using AER port service driver on >> > the device). I wonder if we couldn't do away with the requirement of the >> > AER service being used on the port if pci_aer_available() & host->native_aer >> > don't lie. I'm still trying to decide exactly what the condition ought to >> > look like, but it might draw from the AER service check above it. For example: >> > >> > if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && >> > - pci_aer_available() && >> > - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) >> > + dev->aer_cap && pci_aer_available() && >> > + (pcie_ports_dpc_native || host->native_aer)) >> > services |= PCIE_PORT_SERVICE_DPC; >> >> This sounds like it might be a regression report for d8d2b65a940b >> ("PCI/portdrv: Allow AER service only for Root Ports & RCECs"), which >> appeared in v6.2. Is that true? >> >> If d8d2b65a940b requires you to use the "pcie_ports=dpc-native" kernel >> parameter when you didn't need it before, that sounds like a >> regression. >> >> Looking at the code, that "services & PCIE_PORT_SERVICE_AER" >> definitely looks like a problem. We added that with >> https://git.kernel.org/linus/4e5fad429bd1 ("PCI/DPC: Do not enable DPC >> if AER control is not allowed by the BIOS"), but I think your >> suggestion of checking host->native_aer is better. >> >> Do you want to post a formal patch for it? >> >> Bjorn
