[PATCH] PCI: pcie_bus_perf: Allow MRRS above MPS when completions are safe

[PATCH] PCI: pcie_bus_perf: Allow MRRS above MPS when completions are safe

[Kai-Heng Feng]

In PCIE_BUS_PERFORMANCE mode, pcie_write_mrrs() sets MRRS equal to MPS
for all endpoints. This is overly conservative when completions from the
root complex cannot exceed what every receiver on the path can accept.

Completions are sized by the completer up to the root port's MPS. Cap
MRRS when an intermediate bridge or the requester itself has lower MPS
than the root port (e.g. root MPS=512, endpoint MPS=256). Otherwise set
MRRS from pcie_mpss (clamped to 4096), which raises MRRS above MPS on
paths such as Grace where root MPS is 256 and the endpoint MPSS is
larger.

Comparing only ancestor bridge MPS to the endpoint's MPS is not enough:
under top-down MPS configuration every ancestor already has MPS >= the
endpoint, so that check never triggers. Also keep the pcie_set_readrq()
clamp topology-aware; dropping it unconditionally would let drivers
request completions larger than the endpoint can receive on direct-attach
paths. Fail closed for non-PCIe or unidentifiable roots.

Update the pcie_bus_perf description in kernel-parameters.txt to match.

Tested-on: NVIDIA Grace GH200 (lego-cg1-qct-055) with pci=pcie_bus_perf
  ConnectX-7: MPS=256 MPSS=512  MRRS 256 -> 512
  SAS HBA:    MPS=256 MPSS=1024 MRRS 256 -> 1024
  Without pci=pcie_bus_perf: no change vs stock defaults

Link: https://lore.kernel.org/linux-pci/20180123174821.GF5317@bhelgaas-glaptop.roam.corp.google.com/T/
Signed-off-by: Kai-Heng Feng <kaihengf@nvidia.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 .../admin-guide/kernel-parameters.txt         |  9 +-
 drivers/pci/pci.c                             |  8 +-
 drivers/pci/pci.h                             |  1 +
 drivers/pci/probe.c                           | 87 +++++++++++++++++--
 4 files changed, 93 insertions(+), 12 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 3d35270dddef..902bc631b98b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5150,9 +5150,12 @@ Kernel parameters
 				supported by all devices below the root complex.
 		pcie_bus_perf	Set device MPS to the largest allowable MPS
 				based on its parent bus. Also set MRRS (Max
-				Read Request Size) to the largest supported
-				value (no larger than the MPS that the device
-				or bus can support) for best performance.
+				Read Request Size) appropriately for best
+				performance: raise it above the device MPS when
+				completions from the root complex cannot exceed
+				what every bridge and the device on the path can
+				accept; otherwise keep MRRS capped to the
+				device MPS.
 		pcie_bus_peer2peer	Set every device's MPS to 128B, which
 				every device is guaranteed to support. This
 				configuration allows peer-to-peer DMA between
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..3e17594e181d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5845,14 +5845,14 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
 		return -EINVAL;
 
 	/*
-	 * If using the "performance" PCIe config, we clamp the read rq
-	 * size to the max packet size to keep the host bridge from
-	 * generating requests larger than we can cope with.
+	 * If using the "performance" PCIe config, clamp the read rq size when
+	 * the path cannot safely accept root-sized completions (intermediate
+	 * bridge or this device has lower MPS than the root port).
 	 */
 	if (pcie_bus_config == PCIE_BUS_PERFORMANCE) {
 		int mps = pcie_get_mps(dev);
 
-		if (mps < rq)
+		if (pcie_path_needs_mrrs_cap(dev) && mps < rq)
 			rq = mps;
 	}
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4469e1a77f3c..48572fd5b62d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -118,6 +118,7 @@ extern struct mutex pci_rescan_remove_lock;
 bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
 bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
 bool pcie_cap_has_rtctl(const struct pci_dev *dev);
+bool pcie_path_needs_mrrs_cap(struct pci_dev *dev);
 
 /* Standard Capability finder */
 /**
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index dd0abbc63e18..7ab2bd66f3a5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2961,6 +2961,76 @@ static void pcie_write_mps(struct pci_dev *dev, int mps)
 		pci_err(dev, "Failed attempting to set the MPS\n");
 }
 
+/*
+ * Check whether MRRS must be capped to the device's MPS.
+ *
+ * In PCIE_BUS_PERFORMANCE mode, MPS is configured top-down. Completions are
+ * sized by the completer (root complex) up to the root port's MPS. Every
+ * receiver on the path -- each intermediate bridge and the requester itself --
+ * must accept that completion size, so MRRS must be capped whenever
+ * MPS(root port) exceeds MPS of any of those receivers.
+ *
+ * Comparing only ancestor bridge MPS to the endpoint's MPS is not enough:
+ * under top-down configuration every ancestor already has MPS >= the
+ * endpoint, so that check never triggers. Required checks are:
+ *   - intermediate bridge MPS < root port MPS, and
+ *   - requester MPS < root port MPS (needed by pcie_set_readrq() callers
+ *     that request values larger than MPS on direct-attach paths).
+ *
+ * When the path cannot be characterised (unknown root, non-PCIe bridge),
+ * fail closed and keep the cap.
+ *
+ * Note: this assumes no peer-to-peer DMA, which is the standing assumption for
+ * PCIE_BUS_PERFORMANCE mode.
+ */
+bool pcie_path_needs_mrrs_cap(struct pci_dev *dev)
+{
+	struct pci_bus *bus = dev->bus;
+	int root_mps = -1;
+	int min_inter_mps = -1;
+
+	while (bus->parent) {
+		struct pci_dev *bridge = bus->self;
+
+		if (bridge) {
+			int mps;
+
+			if (!pci_is_pcie(bridge))
+				return true;	/* unknown; keep the cap */
+
+			mps = pcie_get_mps(bridge);
+
+			/*
+			 * The bridge directly above the root bus is treated as
+			 * the root port. Everything between the device and
+			 * that bridge must forward completion TLPs.
+			 */
+			if (bus->parent->parent) {
+				if (min_inter_mps < 0 || mps < min_inter_mps)
+					min_inter_mps = mps;
+			} else {
+				root_mps = mps;
+			}
+		}
+		bus = bus->parent;
+	}
+
+	/* No identifiable root port MPS: keep the conservative cap. */
+	if (root_mps < 0)
+		return true;
+
+	if (min_inter_mps >= 0 && min_inter_mps < root_mps)
+		return true;
+
+	/*
+	 * Direct-attach and uniform-vs-root paths still need a cap when this
+	 * device's own MPS is below the root port's -- otherwise drivers
+	 * calling pcie_set_readrq() with a large value can request completions
+	 * larger than the requester can receive.
+	 */
+	return pcie_get_mps(dev) < root_mps;
+}
+
 static void pcie_write_mrrs(struct pci_dev *dev)
 {
 	int rc, mrrs;
@@ -2973,12 +3043,19 @@ static void pcie_write_mrrs(struct pci_dev *dev)
 		return;
 
 	/*
-	 * For max performance, the MRRS must be set to the largest supported
-	 * value.  However, it cannot be configured larger than the MPS the
-	 * device or the bus can support.  This should already be properly
-	 * configured by a prior call to pcie_write_mps().
+	 * Prefer a larger MRRS when completions from the root complex cannot
+	 * exceed what every receiver on the path (bridges and this device)
+	 * can accept. Otherwise cap to the device's MPS.
+	 *
+	 * There is no Max_Read_Request_Size capability field; pcie_mpss is
+	 * used as a conservative upper bound (clamped to the architectural
+	 * 4096B MRRS maximum). Hardware that rejects the value is handled by
+	 * the shrink loop below.
 	 */
-	mrrs = pcie_get_mps(dev);
+	if (pcie_path_needs_mrrs_cap(dev))
+		mrrs = pcie_get_mps(dev);
+	else
+		mrrs = min(128 << dev->pcie_mpss, 4096);
 
 	/*
 	 * MRRS is a R/W register.  Invalid values can be written, but a
-- 
2.50.1 (Apple Git-155)