Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: "Bjorn Helgaas" <bhelgaas@google.com>,
	"Bjorn Helgaas" <helgaas@kernel.org>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Heiko Stuebner" <heiko@sntech.de>,
	"Yue Wang" <yue.wang@Amlogic.com>,
	"Hans Zhang" <18255117159@163.com>
Cc: "Keith Busch" <kbusch@kernel.org>,
	mx2pg@pm.me, "Niklas Cassel" <cassel@kernel.org>,
	"Lukas Wunner" <lukas@wunner.de>, "Frank Li" <Frank.Li@nxp.com>,
	"Mahesh Vaidya" <mahesh.vaidya@altera.com>,
	"Ricardo Pardini" <ricardo@pardini.net>,
	"Shawn Lin" <shawn.lin@rock-chips.com>,
	"Pali Rohár" <pali@kernel.org>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Rob Herring" <robh@kernel.org>,
	"Jingoo Han" <jingoohan1@gmail.com>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Martin Blumenstingl" <martin.blumenstingl@googlemail.com>,
	"Myron Stowe" <myron.stowe@redhat.com>,
	"Jon Mason" <jdmason@kudzu.us>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-amlogic@lists.infradead.org,
	linux-rockchip@lists.infradead.org
Subject: [PATCH v2 2/3] PCI: Configure Root Port MPS after scanning its hierarchy
Date: Tue, 20 Oct 2026 14:28:56 +0200	[thread overview]
Message-ID: <20261020122854.1500883-7-cassel@kernel.org> (raw)
In-Reply-To: <20261020122854.1500883-5-cassel@kernel.org>

With the default MPS strategy (PCIE_BUS_DEFAULT), pci_configure_mps()
only matches each device's Maximum Payload Size (MPS) to its upstream
bridge.  Root Ports have no upstream bridge, so their MPS stays at
whatever firmware or the hardware default (128 bytes) left there, and the
hierarchy below them inherits that value even if the Root Port and all
devices below it support more.

Once the hierarchy below a Root Port has been scanned, and before drivers
are bound, set the Root Port and every device below it to the largest MPS
they all support, even if that is more than firmware programmed.  Do this
from pcie_bus_configure_settings(), which the host bridge, ACPI and
hotplug paths call after scanning, and from pci_rescan_bus() and
pci_rescan_bus_bridge_resize(), which don't.

If the hierarchy contains a hotplug bridge other than the Root Port,
typically a Switch Downstream Port, limit it to 128 bytes instead, like
pcie_find_smpss() does for PCIE_BUS_SAFE: a device hot-added below such a
port later can't lower the MPS of a hierarchy in use, but every device
supports 128 bytes.  This costs throughput where firmware programmed a
larger MPS for such a hierarchy, in exchange for hot-added devices always
working.

An empty slot directly below a Root Port is left alone; it is evaluated
when a device is hot-added or rescanned there, which also allows raising
the Root Port again after a device that supported less has been replaced.
Hierarchies in which a device other than a Switch port may already have
a driver bound are never changed.  If the MPS of a device can't be set,
restore the previous MPS of the whole hierarchy rather than leaving it
with mismatched settings.

Since PCIE_BUS_DEFAULT can now raise MPS above what firmware programmed,
apply the Intel 5000/5100 read completion coalescing quirk to it as well.

The other strategies are unchanged: PCIE_BUS_TUNE_OFF doesn't touch MPS,
and PCIE_BUS_SAFE, PCIE_BUS_PERFORMANCE and PCIE_BUS_PEER2PEER already
configure the hierarchy in pcie_bus_configure_settings().

Suggested-by: Manivannan Sadhasivam <mani@kernel.org>
Co-developed-by: Hans Zhang <18255117159@163.com>
Signed-off-by: Hans Zhang <18255117159@163.com>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/probe.c  | 116 +++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/quirks.c |   3 +-
 2 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ccc3e9202c2c..ce3cd235a6a6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3102,6 +3102,115 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
 	return 0;
 }
 
+struct pcie_mps_update {
+	int mps;
+	bool restore;
+	int err;
+};
+
+static int pcie_update_mps(struct pci_dev *dev, void *data)
+{
+	struct pcie_mps_update *update = data;
+	int ret;
+
+	/* MPS is of type 'RsvdP' for VFs */
+	if (!pci_is_pcie(dev) || dev->is_virtfn ||
+	    pcie_get_mps(dev) == update->mps)
+		return 0;
+
+	ret = pcie_set_mps(dev, update->mps);
+	if (!ret)
+		return 0;
+
+	pci_err(dev, "can't set Max Payload Size to %d\n", update->mps);
+	update->err = ret;
+
+	/* When restoring, keep going to restore as many devices as possible */
+	return update->restore ? 0 : ret;
+}
+
+/*
+ * Set the MPS of @rp and of every device below it to @mps, top-down.  Unless
+ * @restore is set, stop at the first device that can't be set and return its
+ * error, so the caller can restore the previous MPS.
+ */
+static int pcie_set_hierarchy_mps(struct pci_dev *rp, int mps, bool restore)
+{
+	struct pcie_mps_update update = { .mps = mps, .restore = restore };
+
+	if (!pcie_update_mps(rp, &update))
+		pci_walk_bus(rp->subordinate, pcie_update_mps, &update);
+
+	return update.err;
+}
+
+/*
+ * With PCIE_BUS_DEFAULT, pci_configure_mps() only matches each device to its
+ * upstream bridge, so a hierarchy inherits whatever MPS firmware, or the
+ * hardware default of 128 bytes, left in its Root Port.  Once the hierarchy
+ * below a Root Port has been enumerated, set it to the largest MPS that all
+ * of its devices support.
+ *
+ * A device hot-added below a hotplug bridge other than the Root Port can't
+ * lower the MPS once the hierarchy is in use, so limit such hierarchies to
+ * 128 bytes, which every device supports, like pcie_find_smpss() does for
+ * PCIE_BUS_SAFE.  A hotplug slot directly below the Root Port doesn't need
+ * this: it is evaluated when a device is hot-added there, and an empty slot
+ * is left alone until then.  Hierarchies are also left alone once any of
+ * their devices, other than Switch ports, may have a driver bound.
+ */
+static void pcie_bus_configure_default_mps(struct pci_bus *bus)
+{
+	struct pci_dev *rp = bus->self;
+	u8 smpss = rp->pcie_mpss;
+	int mps, old_mps;
+
+	if (pci_pcie_type(rp) != PCI_EXP_TYPE_ROOT_PORT ||
+	    list_empty(&bus->devices) || pci_bus_in_use(bus))
+		return;
+
+	pci_walk_bus(bus, pcie_find_smpss, &smpss);
+	mps = 128 << smpss;
+	old_mps = pcie_get_mps(rp);
+
+	/* Don't leave the hierarchy with mismatched MPS settings on failure */
+	if (pcie_set_hierarchy_mps(rp, mps, false)) {
+		pci_warn(rp, "can't set Max Payload Size of hierarchy to %d, restoring %d\n",
+			 mps, old_mps);
+		pcie_set_hierarchy_mps(rp, old_mps, true);
+		return;
+	}
+
+	if (mps != old_mps)
+		pci_info(rp, "Max Payload Size of hierarchy set to %d (was %d)\n",
+			 mps, old_mps);
+}
+
+/*
+ * A rescan doesn't go through pcie_bus_configure_settings(), so configure
+ * the Root Port hierarchies it may have populated here: every hierarchy
+ * below a rescanned root bus, or the one @bus belongs to.
+ */
+static void pcie_rescan_configure_default_mps(struct pci_bus *bus)
+{
+	struct pci_bus *child;
+	struct pci_dev *rp;
+
+	if (pcie_bus_config != PCIE_BUS_DEFAULT)
+		return;
+
+	if (pci_is_root_bus(bus)) {
+		list_for_each_entry(child, &bus->children, node)
+			if (child->self && pci_is_pcie(child->self))
+				pcie_bus_configure_default_mps(child);
+		return;
+	}
+
+	rp = pcie_find_root_port(bus->self);
+	if (rp && rp->subordinate)
+		pcie_bus_configure_default_mps(rp->subordinate);
+}
+
 /*
  * pcie_bus_configure_settings() requires that pci_walk_bus work in a top-down,
  * parents then children fashion.  If this changes, then this code will not
@@ -3117,6 +3226,11 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
 	if (!pci_is_pcie(bus->self))
 		return;
 
+	if (pcie_bus_config == PCIE_BUS_DEFAULT) {
+		pcie_bus_configure_default_mps(bus);
+		return;
+	}
+
 	/*
 	 * FIXME - Peer to peer DMA is possible, though the endpoint would need
 	 * to be aware of the MPS of the destination.  To work around this,
@@ -3555,6 +3669,7 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
 	max = pci_scan_child_bus(bus);
 
 	pci_assign_unassigned_bridge_resources(bridge);
+	pcie_rescan_configure_default_mps(bus);
 
 	pci_bus_add_devices(bus);
 
@@ -3576,6 +3691,7 @@ unsigned int pci_rescan_bus(struct pci_bus *bus)
 
 	max = pci_scan_child_bus(bus);
 	pci_assign_unassigned_bus_resources(bus);
+	pcie_rescan_configure_default_mps(bus);
 	pci_bus_add_devices(bus);
 
 	return max;
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index f1e6566542a4..31cbe7e91bb1 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3477,8 +3477,7 @@ static void quirk_intel_mc_errata(struct pci_dev *dev)
 	int err;
 	u16 rcc;
 
-	if (pcie_bus_config == PCIE_BUS_TUNE_OFF ||
-	    pcie_bus_config == PCIE_BUS_DEFAULT)
+	if (pcie_bus_config == PCIE_BUS_TUNE_OFF)
 		return;
 
 	/*
-- 
2.56.0


  parent reply	other threads:[~2026-10-20 12:33 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 12:28 [PATCH v2 0/3] PCI: Fix and improve default MPS configuration Niklas Cassel
2026-10-20 12:28 ` [PATCH v2 1/3] PCI: Match the hierarchy's MPS to a device's MPSS as necessary Niklas Cassel
2026-10-20 12:44   ` sashiko-bot
2026-10-20 12:28 ` Niklas Cassel [this message]
2026-10-20 13:10   ` [PATCH v2 2/3] PCI: Configure Root Port MPS after scanning its hierarchy sashiko-bot
2026-10-20 12:28 ` [PATCH v2 3/3] PCI: meson: Remove redundant MPS configuration Niklas Cassel
2026-10-20 12:36   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261020122854.1500883-7-cassel@kernel.org \
    --to=cassel@kernel.org \
    --cc=18255117159@163.com \
    --cc=Frank.Li@nxp.com \
    --cc=bhelgaas@google.com \
    --cc=heiko@sntech.de \
    --cc=helgaas@kernel.org \
    --cc=jbrunet@baylibre.com \
    --cc=jdmason@kudzu.us \
    --cc=jingoohan1@gmail.com \
    --cc=kbusch@kernel.org \
    --cc=khilman@baylibre.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=lpieralisi@kernel.org \
    --cc=lukas@wunner.de \
    --cc=mahesh.vaidya@altera.com \
    --cc=mani@kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=mx2pg@pm.me \
    --cc=myron.stowe@redhat.com \
    --cc=neil.armstrong@linaro.org \
    --cc=pali@kernel.org \
    --cc=ricardo@pardini.net \
    --cc=robh@kernel.org \
    --cc=shawn.lin@rock-chips.com \
    --cc=yue.wang@Amlogic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox