PCI: brcmstb: Prevent config space access when link is down
authorJim Quinlan <jim2101024@gmail.com>
Mon, 25 Jul 2022 15:12:52 +0000 (11:12 -0400)
committerBjorn Helgaas <bhelgaas@google.com>
Wed, 27 Jul 2022 16:52:54 +0000 (11:52 -0500)
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: Jim Quinlan <jim2101024@gmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
drivers/pci/controller/pcie-brcmstb.c

index bd88a0a..52a53c4 100644 (file)
@@ -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);
 }