diff mbox

PCI: aspm: deal with missing root ports in link state handling

Message ID 20171002140840.7767-1-ard.biesheuvel@linaro.org (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Ard Biesheuvel Oct. 2, 2017, 2:08 p.m. UTC
Even though it is unconventional, some PCIe host implementations omit
the root ports entirely, and simply consist of a host bridge (which
is not modeled as a device in the PCI hierarchy) and a link.

When the downstream device is an endpoint, our current code does not
seem to mind this unusual configuration. However, when PCIe switches
are involved, the ASPM code assumes that any downstream switch port
has a parent, and blindly derefences the bus->parent->self field of
the pci_dev struct to chain the downstream link state to the link
state of the root port. Given that the root port is missing, the link
is not modeled at all, and nor is the link state, and attempting to
access it results in a NULL pointer dereference and a crash.

So let's avoid this by allowing the link state chain to terminate at
the downstream port if no root port exists.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/pci/pcie/aspm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Bjorn Helgaas Oct. 4, 2017, 10:27 p.m. UTC | #1
On Mon, Oct 02, 2017 at 03:08:40PM +0100, Ard Biesheuvel wrote:
> Even though it is unconventional, some PCIe host implementations omit
> the root ports entirely, and simply consist of a host bridge (which
> is not modeled as a device in the PCI hierarchy) and a link.
> 
> When the downstream device is an endpoint, our current code does not
> seem to mind this unusual configuration. However, when PCIe switches
> are involved, the ASPM code assumes that any downstream switch port
> has a parent, and blindly derefences the bus->parent->self field of
> the pci_dev struct to chain the downstream link state to the link
> state of the root port. Given that the root port is missing, the link
> is not modeled at all, and nor is the link state, and attempting to
> access it results in a NULL pointer dereference and a crash.
> 
> So let's avoid this by allowing the link state chain to terminate at
> the downstream port if no root port exists.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Applied to pci/aspm for v4.15, thanks!

> ---
>  drivers/pci/pcie/aspm.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 1dfa10cc566b..0bea8498b5a5 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -802,10 +802,14 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
>  
>  	/*
>  	 * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
> -	 * hierarchies.
> +	 * hierarchies.  Note that some PCIe host implementations omit
> +	 * the root ports entirely, in which case a downstream port on
> +	 * a switch may become the root of the link state chain for all
> +	 * its subordinate endpoints.
>  	 */
>  	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
> -	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE) {
> +	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE ||
> +	    !pdev->bus->parent->self) {
>  		link->root = link;
>  	} else {
>  		struct pcie_link_state *parent;
> -- 
> 2.11.0
>
diff mbox

Patch

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 1dfa10cc566b..0bea8498b5a5 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -802,10 +802,14 @@  static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 
 	/*
 	 * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
-	 * hierarchies.
+	 * hierarchies.  Note that some PCIe host implementations omit
+	 * the root ports entirely, in which case a downstream port on
+	 * a switch may become the root of the link state chain for all
+	 * its subordinate endpoints.
 	 */
 	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
-	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE) {
+	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE ||
+	    !pdev->bus->parent->self) {
 		link->root = link;
 	} else {
 		struct pcie_link_state *parent;