PCI/ASPM: Use dev->has_secondary_link to find downstream links

We allocate pcie_link_state for the component at the upstream end of a
Link.  Previously we did this by allocating pcie_link_state for Root Ports
and Downstream Ports.  This works fine for the typical topology:

  00:1c.0 Root Port       [bridge to bus 02]
  02:00.0 Upstream Port   [bridge to bus 03]
  03:00.0 Downstream Port [bridge to bus 04]
  04:00.0 Endpoint or Switch Port

However, it is possible to have a Root Port connected to a Downstream Port
instead of an Upstream Port, as in Robert White's ATCA system:

  00:1c.0 Root Port       [bridge to bus 02]
  02:00.0 Downstream Port [bridge to bus 03]
  03:01.0 Downstream Port [bridge to bus 04]
  04:00.0 Endpoint or Switch Port

In this topology, we wrongly allocated pcie_link_state for the 02:00.0
Downstream Port, which is actually the *downstream* end of a link.  This
led to the following NULL pointer dereference when we tried to connect this
link into the tree of links starting at the 00:1c.0 Root Port:

  BUG: unable to handle kernel NULL pointer dereference at 0000000000000088
  IP: [<ffffffff81550324>] pcie_aspm_init_link_state+0x744/0x850
  Hardware name: Kontron B3001/B3001, BIOS 4.6.3 08/07/2012
  Call Trace:
   [<ffffffff8153b865>] pci_scan_slot+0xd5/0x120
   [<ffffffff8153ca1d>] pci_scan_child_bus+0x2d/0xd0
   ...

Instead of relying on the component type to identify the upstream end of a
link, use the "dev->has_secondary_link" field.

This means it's now possible for an Upstream Port to have a link on its
secondary side, so alloc_pcie_link_state() needs to connect links
originating from both Upstream and Downstream Ports into the tree.

[bhelgaas: changelog, add comment]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=94361
Link: http://lkml.kernel.org/r/54EB81B2.4050904@pobox.com
Reported-by: Robert White <rwhite@pobox.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 4e1af7637501bb8ea1c0c534d39195cdb859615c..a84f87247d2fa776c83ae7e4296ea5fa90c7ca00 100644 (file)
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -525,7 +525,7 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
        INIT_LIST_HEAD(&link->children);
        INIT_LIST_HEAD(&link->link);
        link->pdev = pdev;
-       if (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM) {
+       if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT) {
                struct pcie_link_state *parent;
                parent = pdev->bus->parent->self->link_state;
                if (!parent) {
@@ -559,10 +559,15 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
        if (!aspm_support_enabled)
                return;
 
-       if (!pci_is_pcie(pdev) || pdev->link_state)
+       if (pdev->link_state)
                return;
-       if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
-           pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM)
+
+       /*
+        * We allocate pcie_link_state for the component on the upstream
+        * end of a Link, so there's nothing to do unless this device has a
+        * Link on its secondary side.
+        */
+       if (!pdev->has_secondary_link)
                return;
 
        /* VIA has a strange chipset, root port is under a bridge */
@@ -715,8 +720,7 @@ static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
        if (!pci_is_pcie(pdev))
                return;
 
-       if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
-           pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM)
+       if (pdev->has_secondary_link)
                parent = pdev;
        if (!parent || !parent->link_state)
                return;