Commit 5fb8f262 authored by Jim Quinlan's avatar Jim Quinlan Committed by Bjorn Helgaas
Browse files

PCI: brcmstb: Prevent config space access when link is down 

When the link is down, config accesses to downstream devices cause CPU
aborts.  Allow config accesses only when the link is up.

As the following scenario shows, this check is racy and cannot completely
avoid CPU aborts, but it makes them less likely:

  pci_generic_config_read
    addr = brcm_pcie_map_conf            # bus->ops->map_bus()
      brcm_pcie_link_up                  # returns "true"; link is up
    <link goes down>
    *val = readb(addr)                   # link is now down
    <CPU abort>

Note that config space accesses to the Root Port are not affected by link
status.

[bhelgaas: commit log, use PCIE_ECAM_REG() instead of magic 0xfff masks;
note that pci_generic_config_read32() masks low two bits already]
Link: https://lore.kernel.org/r/20220725151258.42574-4-jim2101024@gmail.com


Signed-off-by: default avatarJim Quinlan <jim2101024@gmail.com>
Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
Tested-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
parent 0ac2ff42

drivers/pci/controller/pcie-brcmstb.c

+14 −6
Original line number Diff line number Diff line
@@ -683,14 +683,18 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn, 
	void __iomem *base = pcie->base;

	int idx;



	/* Accesses to the RC go right to the RC registers if slot==0 */

	/* Accesses to the RC go right to the RC registers if !devfn */

	if (pci_is_root_bus(bus))

		return PCI_SLOT(devfn) ? NULL : base + where;

		return devfn ? NULL : base + PCIE_ECAM_REG(where);



	/* An access to our HW w/o link-up will cause a CPU Abort */

	if (!brcm_pcie_link_up(pcie))

		return NULL;



	/* For devices, write to the config space index register */

	idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);

	writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);

	return base + PCIE_EXT_CFG_DATA + where;

	return base + PCIE_EXT_CFG_DATA + PCIE_ECAM_REG(where);

}



static void __iomem *brcm_pcie_map_conf32(struct pci_bus *bus, unsigned int devfn,
@@ -700,12 +704,16 @@ static void __iomem *brcm_pcie_map_conf32(struct pci_bus *bus, unsigned int devf 
	void __iomem *base = pcie->base;

	int idx;



	/* Accesses to the RC go right to the RC registers if slot==0 */

	/* Accesses to the RC go right to the RC registers if !devfn */

	if (pci_is_root_bus(bus))

		return PCI_SLOT(devfn) ? NULL : base + (where & ~0x3);

		return devfn ? NULL : base + PCIE_ECAM_REG(where);



	/* An access to our HW w/o link-up will cause a CPU Abort */

	if (!brcm_pcie_link_up(pcie))

		return NULL;



	/* For devices, write to the config space index register */

	idx = PCIE_ECAM_OFFSET(bus->number, devfn, (where & ~3));

	idx = PCIE_ECAM_OFFSET(bus->number, devfn, where);

	writel(idx, base + IDX_ADDR(pcie));

	return base + DATA_ADDR(pcie);

}