PCI: Workaround wrong flags completions for IDT switch

The IDT switch incorrectly flags an ACS source violation on a read config
request to an end point device on the completion (IDT 89H32H8G3-YC,
errata #36) even though the PCI Express spec states that completions are
never affected by ACS source violation (PCI Spec 3.1, Section 6.12.1.1).

The suggested workaround by IDT is to issue a configuration write to the
downstream device before issuing the first config read. This allows the
downstream device to capture its bus number, thus avoiding the ACS
violation on the completion.

The patch does the following -

1. Disable ACS source violation if enabled
2. Wait for config space access to become available by reading vendor id
3. Do a config write to the end point (errata workaround)
4. Enable ACS source validation (if it was enabled to begin with)

-v2: move workaround to pci_bus_read_dev_vendor_id() from
pci_bus_check_dev()
      and move enable_acs_sv to drivers/pci/pci.c -- by Yinghai
-v3: add bus->self check for root bus and virtual bus for sriov vfs.
-v4: only do workaround for IDT switches
-v5: tweak pci_std_enable_acs_sv to deal with unimplemented SV and
clarify return value

Signed-off-by: James Puthukattukaran <james.puthukattukaran@oracle.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
--

  drivers/pci/pci.c   | 37 +++++++++++++++++++++++++++++++++++++
  drivers/pci/pci.h   |  1 +
  drivers/pci/probe.c | 38 ++++++++++++++++++++++++++++++++++++--
  3 files changed, 74 insertions(+), 2 deletions(-)

Orabug: 26243152

Link: https://patchwork.kernel.org/patch/9828571/
Fixed wrapped lines in the original patch to fit the lines in 80 columns.
Changed variable types from integer to bool to keep consistent with function
return type.

Signed-off-by: Shan Hai <shan.hai@oracle.com>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index eb7479929cee81267b4bd5fee6eb7b41fbcee5a2..0c90860d805ef39a2686435c9ffbff047f91fd8c 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2336,6 +2336,42 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
        return (ctrl & acs_flags) == acs_flags;
 }
 
+/**
+ *  pci_std_toggle_acs_sv - toggle ACS source validation feature
+ *  @dev - pcie switch/RP
+ *  @enable - enable (true) or disable (false) source validation
+ *
+ *  Returns : false on failure (if SV capability is not implemented)
+ *           previous acs_sv state (true or false)
+ */
+bool pci_std_toggle_acs_sv(struct pci_dev *dev, bool enable)
+{
+       int pos;
+       u16 cap;
+       u16 ctrl;
+       bool retval;
+
+       pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
+       if (!pos)
+               return false;
+
+       pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
+
+       if (!(cap & PCI_ACS_SV))
+               return false;
+
+       pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
+
+       retval = !!(ctrl & cap & PCI_ACS_SV);
+       if (enable)
+               ctrl |= (cap & PCI_ACS_SV);
+       else
+               ctrl &= ~(cap & PCI_ACS_SV);
+
+       pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
+
+       return retval;
+}
 /**
  * pci_acs_enabled - test ACS against required flags for a given device
  * @pdev: device to test

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0573df2e866e78fac24fc6b53cb23da5eb897ff5..1a0f6f93c46a1ae86770450f1d86c0a116436933 100644 (file)
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -307,6 +307,7 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 }
 
 void pci_enable_acs(struct pci_dev *dev);
+bool pci_std_toggle_acs_sv(struct pci_dev *dev, bool enable);
 
 struct pci_dev_reset_methods {
        u16 vendor;

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 7d9ece4a07a04eef599ae7dcc350f65e1b3a3925..890361d1fd53a83f8a11df4b108bde600697e931 100644 (file)
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1485,8 +1485,8 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
 }
 EXPORT_SYMBOL(pci_alloc_dev);
 
-bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
-                               int crs_timeout)
+static bool __pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn,
+                                        u32 *l, int crs_timeout)
 {
        int delay = 1;
 
@@ -1523,7 +1523,42 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
 
        return true;
 }
-EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
+
+bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
+                               int crs_timeout)
+{
+       bool found;
+       bool enable = false;
+       bool idt_workaround = (bus->self &&
+                               (bus->self->vendor == PCI_VENDOR_ID_IDT));
+       /*
+        * Some IDT switches flag an ACS violation for config reads
+        * even though the PCI spec allows for it (PCIe 3.1, 6.1.12.1)
+        * It flags it because the bus number is not properly set in the
+        * completion. The workaround is to do a dummy write to properly
+        * latch number once the device is ready for config operations
+        */
+
+       if (idt_workaround)
+               enable = pci_std_toggle_acs_sv(bus->self, false);
+
+       found = __pci_bus_read_dev_vendor_id(bus, devfn, l, crs_timeout);
+
+       /*
+        * The fact that we can read the vendor id indicates that the device
+        * is ready for config operations. Do the write as part of the errata
+        * workaround.
+        */
+       if (idt_workaround) {
+               if (found)
+                       pci_bus_write_config_word(bus, devfn, PCI_VENDOR_ID, 0);
+               if (enable)
+                       pci_std_toggle_acs_sv(bus->self, enable);
+       }
+
+       return found;
+}
+ EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
 
 /*
  * Read the config data for a PCI device, sanity-check it