Commit c8fc9339 authored May 21, 2015 by Yijing Wang Committed by Bjorn Helgaas May 26, 2015

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>

parent d0751b98

drivers/pci/pcie/aspm.c

+10 −6

Original line number	Diff line number	Diff line
		@@ -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;

Admin message