Malcolm Crossley wrote:
The PCI Express port driver is registering the AER service for any device
reporting AER capability. The AER driver uses registers which are only
available on PCI Express device root port devices which advertise AER
capabilities.
This patch adds a check that the PCI Express device is a root port before
enabling the AER service driver on that port.
Signed-off-by: Malcolm Crossley <malcolm.crossley2@xxxxxxxxxxx>
---
A bug was seen on boards using a PLX 8518 switch device which advertises AER
on each of it's transparent bridges. The AER driver was loaded for each bridge
and this driver tried to access the AER source ID register whenever an interrupt
occured on the shared PCI INTX lines. The source ID register does not exist
on non root port PCIE device's which advertise AER and trying to access this
register causes a unsupported request error on the bridge. Thus, when the next
interrupt occurs, another error is found and the non existent source ID
register is accessed again, and so it goes on.
The result is a spammed dmesg with unsupported request PCI express errors on
the bridge device that the AER driver is loaded against.
drivers/pci/pcie/portdrv_core.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 52f84fc..2f0cfd5 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -225,7 +225,9 @@ static int get_port_device_capability(struct pci_dev *dev)
int services = 0, pos;
u16 reg16;
u32 reg32;
+ struct pcie_port_data *port_data;
+ port_data = pci_get_drvdata(dev);
pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
pci_read_config_word(dev, pos + PCIE_CAPABILITIES_REG, ®16);
/* Hot-Plug Capable */
@@ -236,7 +238,8 @@ static int get_port_device_capability(struct pci_dev *dev)
services |= PCIE_PORT_SERVICE_HP;
}
/* AER capable */
- if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
+ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR) &&
+ port_data->port_type == PCIE_RC_PORT)
services |= PCIE_PORT_SERVICE_AER;
/* VC support */
if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VC))
Hi,
I'm sorry for the delayed comment.
I think some of current AER driver's code seems to have an assumption
that struct pcie_device are allocated for switch upstream/downstream
ports. And it also seems to assume that any other AER driver would be
registered to switch upstream/downstream ports. See find_aer_service_iter()
in aerdrv_core.c, for example.
How about changing .port_type of struct pcie_port_service_driver for
AER driver like below?
Thanks,
Kenji Kaneshige
---
drivers/pci/pcie/aer/aerdrv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: 20091005/drivers/pci/pcie/aer/aerdrv.c
===================================================================
--- 20091005.orig/drivers/pci/pcie/aer/aerdrv.c
+++ 20091005/drivers/pci/pcie/aer/aerdrv.c
@@ -52,7 +52,7 @@ static struct pci_error_handlers aer_err
static struct pcie_port_service_driver aerdriver = {
.name = "aer",
- .port_type = PCIE_ANY_PORT,
+ .port_type = PCIE_RC_PORT,
.service = PCIE_PORT_SERVICE_AER,
.probe = aer_probe,
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
