PCI: iproc: Work around Stingray CRS defects
authorOza Pawandeep <oza.oza@broadcom.com>
Mon, 28 Aug 2017 21:43:30 +0000 (16:43 -0500)
committerBjorn Helgaas <bhelgaas@google.com>
Mon, 28 Aug 2017 21:43:30 +0000 (16:43 -0500)
Configuration Request Retry Status ("CRS") completions are a required part
of PCIe.  A PCIe device may respond to config a request with a CRS
completion to indicate that it needs more time to initialize.  A Root Port
that receives a CRS completion may automatically retry the request, or it
may treat the request as a failed transaction.  For a failed read, it will
likely synthesize all 1's data, i.e., 0xffffffff, to complete the read to
the CPU.

CRS Software Visibility ("CRS SV") is an optional feature.  Per PCIe r3.1,
sec 2.3.2, if supported and enabled, a Root Port that receives a CRS
completion for a config read of the Vendor ID will synthesize 0x0001 data
(an invalid Vendor ID) instead of retrying or failing the transaction.  The
0x0001 data makes the CRS completion visible to software, so it can perform
other tasks while waiting for the device.

The iProc "Stingray" PCIe controller does not support CRS completions
correctly.  From the Stingray PCIe Controller spec:

  4.7.3.3. Retry Status On Configuration Cycle

  Endpoints are allowed to generate retry status on configuration cycles.
  In this case, the RC needs to re-issue the request. The IP does not
  handle this because the number of configuration cycles needed will
  probably be less than the total number of non-posted operations needed.

  When a retry status is received on the User RX interface for a
  configuration request that was sent on the User TX interface, it will be
  indicated with a completion with the CMPL_STATUS field set to 2=CRS, and
  the user will have to find the address and data values and send a new
  transaction on the User TX interface.  When the internal configuration
  space returns a retry status during a configuration cycle (user_cscfg =
  1) on the Command/Status interface, the pcie_cscrs will assert with the
  pcie_csack signal to indicate the CRS status.

  When the CRS Software Visibility Enable register in the Root Control
  register is enabled, the IP will return the data value to 0x0001 for the
  Vendor ID value and 0xffff  (all 1’s) for the rest of the data in the
  request for reads of offset 0 that return with CRS status.  This is true
  for both the User RX Interface and for the Command/Status interface.
  When CRS Software Visibility is enabled, the CMPL_STATUS field of the
  completion on the User RX Interface will not be 2=CRS and the pcie_cscrs
  signal will not assert on the Command/Status interface.

The Stingray hardware never reissues configuration requests when it
receives CRS completions.  Contrary to what sec 4.7.3.3 above says, when it
receives a CRS completion, it synthesizes 0xffff0001 data regardless of the
address of the read or the value of the CRS SV enable bit.

This is broken in two ways:

  1) When CRS SV is disabled, the Root Port should never synthesize the
  0x0001 value.  If it receives a CRS completion, it should fail the
  transaction and synthesize all 1's data.

  2) When CRS SV is enabled, the Root Port should only synthesize 0x0001
  data if it receives a CRS completion for a read of the Vendor ID.  If it
  receives a CRS completion for any other read, it should fail the
  transaction and synthesize all 1's data.

This breaks pci_flr_wait(), which reads the Command register and expects to
see all 1's data if the read fails because of CRS completions.  On
Stingray, it sees the incorrect 0xffff0001 data instead.

It also breaks config registers that contain the 0xffff0001 value.  If we
read such a register, software can't distinguish a CRS completion from the
actual value read from the device.

On Stingray, if we read 0xffff0001 data, assume this indicates a CRS
completion and retry the read for 500ms.  If we time out, return all 1's
(0xffffffff) data.  Note that this corrupts registers that happen to
contain 0xffff0001.

Stingray advertises CRS SV support in its Root Capabilities register, and
the CRS SV enable bit is writable (even though the hardware ignores it).
Mask out PCI_EXP_RTCAP_CRSVIS so software doesn't try to use CRS SV.

Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>
[bhelgaas: changelog, add probe-time warning about corruption, don't
advertise CRS SV support, remove duplicate pci_generic_config_read32(),
fix alignment based on patch from Arnd Bergmann <arnd@arndb.de>]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
drivers/pci/host/pcie-iproc.c

index 61d9be6..9a006cb 100644 (file)
@@ -68,6 +68,9 @@
 #define APB_ERR_EN_SHIFT             0
 #define APB_ERR_EN                   BIT(APB_ERR_EN_SHIFT)
 
+#define CFG_RETRY_STATUS             0xffff0001
+#define CFG_RETRY_STATUS_TIMEOUT_US  500000 /* 500 milliseconds */
+
 /* derive the enum index of the outbound/inbound mapping registers */
 #define MAP_REG(base_reg, index)      ((base_reg) + (index) * 2)
 
@@ -473,6 +476,77 @@ static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,
        return (pcie->base + offset);
 }
 
+static unsigned int iproc_pcie_cfg_retry(void __iomem *cfg_data_p)
+{
+       int timeout = CFG_RETRY_STATUS_TIMEOUT_US;
+       unsigned int data;
+
+       /*
+        * As per PCIe spec r3.1, sec 2.3.2, CRS Software Visibility only
+        * affects config reads of the Vendor ID.  For config writes or any
+        * other config reads, the Root may automatically reissue the
+        * configuration request again as a new request.
+        *
+        * For config reads, this hardware returns CFG_RETRY_STATUS data
+        * when it receives a CRS completion, regardless of the address of
+        * the read or the CRS Software Visibility Enable bit.  As a
+        * partial workaround for this, we retry in software any read that
+        * returns CFG_RETRY_STATUS.
+        *
+        * Note that a non-Vendor ID config register may have a value of
+        * CFG_RETRY_STATUS.  If we read that, we can't distinguish it from
+        * a CRS completion, so we will incorrectly retry the read and
+        * eventually return the wrong data (0xffffffff).
+        */
+       data = readl(cfg_data_p);
+       while (data == CFG_RETRY_STATUS && timeout--) {
+               udelay(1);
+               data = readl(cfg_data_p);
+       }
+
+       if (data == CFG_RETRY_STATUS)
+               data = 0xffffffff;
+
+       return data;
+}
+
+static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
+                                   int where, int size, u32 *val)
+{
+       struct iproc_pcie *pcie = iproc_data(bus);
+       unsigned int slot = PCI_SLOT(devfn);
+       unsigned int fn = PCI_FUNC(devfn);
+       unsigned int busno = bus->number;
+       void __iomem *cfg_data_p;
+       unsigned int data;
+       int ret;
+
+       /* root complex access */
+       if (busno == 0) {
+               ret = pci_generic_config_read32(bus, devfn, where, size, val);
+               if (ret != PCIBIOS_SUCCESSFUL)
+                       return ret;
+
+               /* Don't advertise CRS SV support */
+               if ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCTL)
+                       *val &= ~(PCI_EXP_RTCAP_CRSVIS << 16);
+               return PCIBIOS_SUCCESSFUL;
+       }
+
+       cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);
+
+       if (!cfg_data_p)
+               return PCIBIOS_DEVICE_NOT_FOUND;
+
+       data = iproc_pcie_cfg_retry(cfg_data_p);
+
+       *val = data;
+       if (size <= 2)
+               *val = (data >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+       return PCIBIOS_SUCCESSFUL;
+}
+
 /**
  * Note access to the configuration registers are protected at the higher layer
  * by 'pci_lock' in drivers/pci/access.c
@@ -567,9 +641,13 @@ static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
                                    int where, int size, u32 *val)
 {
        int ret;
+       struct iproc_pcie *pcie = iproc_data(bus);
 
        iproc_pcie_apb_err_disable(bus, true);
-       ret = pci_generic_config_read32(bus, devfn, where, size, val);
+       if (pcie->type == IPROC_PCIE_PAXB_V2)
+               ret = iproc_pcie_config_read(bus, devfn, where, size, val);
+       else
+               ret = pci_generic_config_read32(bus, devfn, where, size, val);
        iproc_pcie_apb_err_disable(bus, false);
 
        return ret;
@@ -1236,6 +1314,8 @@ static int iproc_pcie_rev_init(struct iproc_pcie *pcie)
                pcie->ib.nr_regions = ARRAY_SIZE(paxb_v2_ib_map);
                pcie->ib_map = paxb_v2_ib_map;
                pcie->need_msi_steer = true;
+               dev_warn(dev, "reads of config registers that contain %#x return incorrect data\n",
+                        CFG_RETRY_STATUS);
                break;
        case IPROC_PCIE_PAXC:
                regs = iproc_pcie_reg_paxc;