On Wed, Dec 07, 2022 at 10:41:05AM +0200, Mika Westerberg wrote: > Only Root Ports and Event Collectors use MSI for AER. PCIe Switch ports > or endpoints on the other hand only send messages (that get collected by > the former). For this reason do not require PCIe switch ports and > endpoints to use interrupt if they support AER. > > This allows portdrv to attach PCIe switch ports of Intel DG1 and DG2 > discrete graphics cards. These do not declare MSI or legacy interrupts. > > Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> Thanks for the additional info! This seems like something we should definitely do. I'm wondering whether we should check for this in get_port_device_capability(). It already has similar checks for device type for other services. This would skip pci_set_master() for these non-RP, non-RCEC devices, which is probably harmless, since I assume we only need that to make sure MSI works. It would also prevent allocation of the AER service for non-RP, non-RCEC devices. That's also probably harmless, since aer_probe() ignores those devices anyway. What do you think of something like this? (This is based on my pci/portdrv branch which squashed everything into portdrv.c: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/log/?h=pci/portdrv) diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c index a6c4225505d5..8b16e96ec15c 100644 --- a/drivers/pci/pcie/portdrv.c +++ b/drivers/pci/pcie/portdrv.c @@ -232,7 +232,9 @@ static int get_port_device_capability(struct pci_dev *dev) } #ifdef CONFIG_PCIEAER - if (dev->aer_cap && pci_aer_available() && + if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT || + pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) && + dev->aer_cap && pci_aer_available() && (pcie_ports_native || host->native_aer)) services |= PCIE_PORT_SERVICE_AER; #endif
