[PATCH V2 4/4] PCI: Clear stale 14-Bit Tag Requester Enable when a link leaves Flit Mode

[Vidya Sagar]

Sanitizing DEV3_CTL on restore covers the devices that go through
pci_dev_restore(), but those are not the only devices whose 14-Bit Tag
Requester Enable can go stale, and for the case that hurts most they are
not even the right device.

For a reset mediated by a bridge, i.e. a Secondary Bus Reset, a DPC
trigger and release, an AER bus reset, a slot reset or a bridge
D3cold->D0 resume, the bridge itself is neither reset nor saved and
restored.  The bridge is however the requester for the config read the
core issues to the device below as soon as the link is back up.  If the
link returns in Non-Flit Mode while the bridge still has 14-Bit Tag
Requester Enable set, that very first config read fails with a Completion
Timeout and an Unexpected Completion, because the device does answer but
with a completion the bridge can no longer match.  Devices below the
bridge that are reached only by paths which never call pci_dev_restore(),
such as a DPC release or a hotplug link status update, have the same
problem.

Add pci_bridge_refresh_14bit_tag().  It re-reads the bridge's own
LNKSTA2.Flit_Mode, clears 14-Bit Tag Requester Enable in the bridge's
DEV3_CTL if Flit Mode is gone, refreshes bus->flit_mode and then walks the
subordinate bus and does the same for every device that advertises 14-Bit
Tag Requester support.  Fixing up the bridge only requires access to the
bridge's own config space on the primary bus, so it works even while the
link below is still unusable.  Hardware is touched only when the enable is
set and Flit Mode is no longer active, so the helper is idempotent and
does nothing on the common path.

Call it from the two choke points that together cover every
kernel-visible link mode change:

  - pci_bridge_wait_for_secondary_bus(), right after the link is known to
    be up and before pci_dev_wait() issues the first config read
    downstream.  This covers SBR via pci_bridge_secondary_bus_reset(),
    DPC release via dpc_reset_link(), AER bus reset via
    pci_bus_error_reset(), slot reset via pciehp_reset_slot() and
    D3cold->D0 resume via pci_pm_bridge_power_up_actions().

  - __pcie_update_link_speed(), where bus->flit_mode is authoritatively
    updated whenever the kernel observes a link change: initial
    enumeration, pcie_retrain_link() for an ASPM common clock
    configuration or a target speed change, the bwctrl IRQ for autonomous
    hardware speed changes and the pciehp link status check.  None of
    these go through pci_bridge_wait_for_secondary_bus().

Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
---
V2:
* New patch as a result of splitting the V1 monolithic patch

 drivers/pci/pci.c   | 131 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/pci/pci.h   |   2 +
 drivers/pci/probe.c |  11 ++++
 3 files changed, 142 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 3bbeed1ba20c..b53ca5f47f07 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1750,12 +1750,22 @@ static void pci_restore_dev3_state(struct pci_dev *dev)
 	 * coherent enable state.  Every other bit of DEV3_CTL (DMWr
 	 * Requester Enable, DMWr Egress Blocking, L0p Enable, Target Link
 	 * Width and any future addition) is written back unchanged.
+	 *
+	 * Note: bridge-side and link-event paths are handled separately by
+	 * pci_bridge_refresh_14bit_tag(), which runs from
+	 * pci_bridge_wait_for_secondary_bus() and __pcie_update_link_speed()
+	 * and clears the bit directly in hardware as soon as the link is
+	 * observed to leave flit mode.  This function's responsibility is
+	 * narrowed to the save-buffer-restore path.
 	 */
 	pci_read_config_dword(dev, pos + PCI_DEV3_CAP, &dev3_cap);
 	if (dev3_cap & PCI_DEV3_CAP_14BIT_TAG_REQ) {
 		/*
-		 * Check both LNKSTA2.Flit_Mode (link-level) and
-		 * DEV3_STA.Segment Captured (end-to-end); both must be
+		 * Re-check link state here too: pci_restore_state() may run
+		 * on paths where the link has changed mode but
+		 * pci_bridge_refresh_14bit_tag() has not yet been called for
+		 * this device.  Check both LNKSTA2.Flit_Mode (link-level)
+		 * and DEV3_STA.Segment Captured (end-to-end); both must be
 		 * active for 14-bit tags.  Refresh bus->flit_mode and
 		 * dev->fm_enabled in lock-step.
 		 */
@@ -1779,6 +1789,107 @@ static void pci_restore_dev3_state(struct pci_dev *dev)
 	pci_write_config_dword(dev, pos + PCI_DEV3_CTL, val);
 }
 
+/*
+ * Clear DEV3_CTL.14-Bit Tag Requester Enable on @dev if flit mode is no
+ * longer active.  Touches only @dev's own config space, so it is safe to
+ * call on a bridge before the first downstream TLP is issued after a
+ * reset.
+ *
+ * 14-Bit Tag Requester Enable is only meaningful in flit mode.  If the
+ * link came back as non-flit (e.g. after SBR, DPC, slot reset, or D3cold
+ * resume), a requester that still has it set will emit TLPs whose
+ * completions it cannot match, producing Completion Timeout plus
+ * Unexpected Completion on the first transaction.
+ */
+static void __pci_dev_clear_stale_14bit_tag(struct pci_dev *dev, bool flit_now)
+{
+	u32 dev3_cap, dev3_ctl, dev3_sta;
+	int pos;
+
+	if (!pci_is_pcie(dev))
+		return;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DEV3);
+	if (!pos)
+		return;
+
+	pci_read_config_dword(dev, pos + PCI_DEV3_CAP, &dev3_cap);
+	if (!(dev3_cap & PCI_DEV3_CAP_14BIT_TAG_REQ))
+		return;
+
+	pci_read_config_dword(dev, pos + PCI_DEV3_STA, &dev3_sta);
+	dev->fm_enabled = !!(dev3_sta & PCI_DEV3_STA_SEGMENT);
+
+	if (flit_now && dev->fm_enabled)
+		return;
+
+	pci_read_config_dword(dev, pos + PCI_DEV3_CTL, &dev3_ctl);
+	if (!(dev3_ctl & PCI_DEV3_CTL_14BIT_TAG_REQ_EN))
+		return;
+
+	dev3_ctl &= ~PCI_DEV3_CTL_14BIT_TAG_REQ_EN;
+	pci_write_config_dword(dev, pos + PCI_DEV3_CTL, dev3_ctl);
+	pci_info(dev, "cleared 14-Bit Tag Requester Enable: flit mode no longer active (DEV3_STA=%#010x)\n",
+		 dev3_sta);
+}
+
+/**
+ * pci_bridge_refresh_14bit_tag - Drop a stale 14-Bit Tag enable across a link
+ * @bridge: PCIe bridge whose link may have changed mode
+ *
+ * Re-evaluate the bridge's own DEV3_CTL.14-Bit Tag Requester Enable
+ * against the live LNKSTA2.Flit_Mode, then walk the bridge's subordinate
+ * bus and do the same for every device that advertises 14-Bit Tag
+ * Requester support.  Also refresh bus->flit_mode so the rest of the PCI
+ * core sees a consistent view of the link.
+ *
+ * Called from every kernel-visible link state change site:
+ *   - pci_bridge_wait_for_secondary_bus() (covers SBR, DPC release, slot
+ *     reset, AER bus reset, bridge D3cold->D0 resume).
+ *   - __pcie_update_link_speed() (covers manual retrain, bwctrl IRQ,
+ *     hotplug link status check, initial enumeration).
+ *
+ * Safe to call repeatedly; only writes hardware when the enable bit is
+ * set and flit mode is no longer active.
+ */
+void pci_bridge_refresh_14bit_tag(struct pci_dev *bridge)
+{
+	struct pci_bus *bus;
+	struct pci_dev *child;
+	u16 lnksta2 = 0;
+	bool flit_now;
+
+	if (!bridge || !pci_is_pcie(bridge))
+		return;
+
+	pcie_capability_read_word(bridge, PCI_EXP_LNKSTA2, &lnksta2);
+	flit_now = !!(lnksta2 & PCI_EXP_LNKSTA2_FLIT);
+
+	/*
+	 * Fix the bridge itself first.  The bridge is the requester for
+	 * outbound config/MMIO TLPs, so a stale 14-Bit Tag Requester Enable
+	 * here is what produces the post-reset Completion Timeout /
+	 * Unexpected Completion failure.
+	 */
+	__pci_dev_clear_stale_14bit_tag(bridge, flit_now);
+
+	bus = bridge->subordinate;
+	if (!bus)
+		return;
+
+	bus->flit_mode = flit_now;
+
+	/*
+	 * Walk the secondary bus.  pci_restore_dev3_state() only fires on
+	 * paths that go through pci_dev_restore(); DPC release, hotplug
+	 * link status updates, and similar paths do not.  Fix those too.
+	 */
+	down_read(&pci_bus_sem);
+	list_for_each_entry(child, &bus->devices, bus_list)
+		__pci_dev_clear_stale_14bit_tag(child, flit_now);
+	up_read(&pci_bus_sem);
+}
+
 static int pci_save_pcix_state(struct pci_dev *dev)
 {
 	int pos;
@@ -4871,6 +4982,14 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
 		pci_dbg(dev, "waiting %d ms for downstream link\n", delay);
 		msleep(delay);
 
+		/*
+		 * The link has had a chance to come back; refresh the
+		 * bridge's (and subtree's) DEV3_CTL.14-Bit Tag Requester
+		 * Enable against the live LNKSTA2.Flit_Mode before we issue
+		 * the first config TLP to the child.
+		 */
+		pci_bridge_refresh_14bit_tag(dev);
+
 		if (!pci_dev_wait(child, reset_type, PCI_RESET_WAIT - delay))
 			return 0;
 
@@ -4898,6 +5017,14 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
 		return -ENOTTY;
 	}
 
+	/*
+	 * Link is up.  Refresh the bridge's (and subtree's)
+	 * DEV3_CTL.14-Bit Tag Requester Enable against the live
+	 * LNKSTA2.Flit_Mode before we issue the first config TLP to the
+	 * child below.
+	 */
+	pci_bridge_refresh_14bit_tag(dev);
+
 	return pci_dev_wait(child, reset_type,
 			    PCIE_RESET_READY_POLL_MS - delay);
 }
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9f55868adac8..0298e250e254 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -682,6 +682,8 @@ void __pcie_update_link_speed(struct pci_bus *bus,
 
 void pcie_update_link_speed(struct pci_bus *bus, enum pcie_link_change_reason reason);
 
+void pci_bridge_refresh_14bit_tag(struct pci_dev *bridge);
+
 /* Single Root I/O Virtualization */
 struct pci_sriov {
 	int		pos;		/* Capability position */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 810114029ee0..d13094baf487 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -838,6 +838,17 @@ void __pcie_update_link_speed(struct pci_bus *bus,
 			      reason,
 			      FIELD_GET(PCI_EXP_LNKSTA_NLW, linksta),
 			      linksta & PCI_EXP_LNKSTA_LINK_STATUS_MASK);
+
+	/*
+	 * Re-evaluate DEV3_CTL.14-Bit Tag Requester Enable on this bridge
+	 * and its subordinate bus.  Any time bus->flit_mode is updated, the
+	 * link has just changed state; if flit mode is no longer active, the
+	 * bridge and downstream devices must drop that enable before
+	 * further TLPs are issued, or the requester (the bridge) will tag
+	 * config/MMIO requests with 14-bit tags that the completer can no
+	 * longer echo back in non-flit mode.
+	 */
+	pci_bridge_refresh_14bit_tag(bus->self);
 }
 
 void pcie_update_link_speed(struct pci_bus *bus,
-- 
2.43.0