[RFC PATCH 0/2] PCI: Reserve resources for a delayed hotplug subtree

[RFC PATCH 0/2] PCI: Reserve resources for a delayed hotplug subtree

[Maciej Grochowski]

On a two-level Microchip PM50052 (Switchtec PFX) hierarchy, a hard reset
of the top switch removes both switches from the PCI tree.  The top
switch returns before the nested switch, so pciehp enumerates its five
hotplug downstream ports while they are empty.  The port leading to the
nested switch receives three bus numbers and a roughly 206 MiB
prefetchable window.  When the nested switch appears, it needs eleven
bus numbers and a 1 GiB-aligned 1032 MiB window for its NTB BARs.  The
missing resources leave part of the hierarchy unusable and the nested
switchtec function cannot bind.

Switches from other vendors can encounter the same resource shortfall
when a child appears after its parent hotplug port is numbered and
sized, if the child's requirements exceed that allocation.

These patches explore pci=hpreserve= to specify minimum resources for a
selected hotplug bridge while its child is absent.  Patch 1 reserves
memory window size and alignment; patch 2 reserves bus numbers before
the remaining bus numbers are divided among sibling hotplug ports.  The
path form tolerates downstream bus renumbering.  With no parameter, the
existing allocation policy is unchanged.

On a v7.3-rc2 based kernel with both patches and
  pci=hpreserve=bus=11:mmiopref=1032M:mmioprefalign=1G@0000:80:01.1/00.0/03.0
we tested a cold boot, two top-switch hard resets, and one nested-switch
hard reset on the two-level system.  After each reset, the nested port
retained its eleven bus numbers and aligned 1032 MiB window, the 1 GiB
NTB BAR was assigned, and both switchtec devices responded to MRPC.

We are posting this RFC now because the failure affects our PCIe hotplug
infrastructure.  The two-level pciehp test shows that a per-bridge
reservation can preserve one delayed subtree.  With three or more
switch layers that return at different times, this implementation
appears to require a reservation at each port sized while empty, and
each ancestor must retain enough bus numbers and aligned memory space
for the eventual subtree.  We have not tested that case.

Consider a larger layout with three top-level switches, each feeding
six second-level switches with eight NTB functions apiece.  If their
child links return late, the current syntax could need 18 full
path-specific entries, plus entries for delayed ancestors.  The NTB
BARs affect the resources required per bridge, not the entry count.
Matching by device ID could shorten a homogeneous case, but risks
reserving resources on same-ID ports without delayed children.

One possible extension, not implemented here, would let several
explicit paths with the same requirements share one reservation, e.g.
<reservation>@<path1>;@<path2>.  Would that make a kernel command-line
interface practical, or should firmware, platform data, or a PCI core
policy describe the expected resources?  We welcome feedback on the
interface as well as on the underlying hotplug allocation problem.

Bus-number reservation is not yet honored on a plain pci_rescan_bus()
path; the hardware test above exercises pciehp.  A separate allocator
retry releases a sibling bridge window on this system.  This series
does not address that issue.

Maciej Grochowski (2):
  PCI: Add pci=hpreserve= to reserve memory windows for a hotplug bridge
  PCI: Allow pci=hpreserve= to reserve bus numbers for a hotplug bridge

 .../admin-guide/kernel-parameters.txt         |  42 +++++++
 drivers/pci/pci.c                             | 108 ++++++++++++++++++
 drivers/pci/pci.h                             |  22 ++++
 drivers/pci/probe.c                           |  58 +++++++++-
 drivers/pci/setup-bus.c                       |  33 ++++++
 5 files changed, 259 insertions(+), 4 deletions(-)


base-commit: 5225b8eec4c9bb21aecff6295fab6346a3c3738e
-- 
2.47.3

[RFC PATCH 1/2] PCI: Add pci=hpreserve= to reserve memory windows for a hotplug bridge

[Maciej Grochowski]

A hotplug bridge is sized for the devices that are present when its
windows are sized, plus the optional hpmmiosize/hpmmioprefsize space.
Some topologies have devices that are known to appear later.  After a
reset of a two-level PCIe switch hierarchy, for example, pciehp
re-enumerates the upstream switch before the link to the nested switch
is back up, so the downstream port leading to the nested switch is
sized with nothing below it.  When the nested switch appears, its BARs
do not fit.

On a two-level Microchip PM50052 (Switchtec PFX) hierarchy whose nested
switch has an NTB function with a 1 GiB prefetchable BAR, a hard reset
of the top-level switch on v7.3-rc2 leaves downstream port 82:03.0
with a 206 MiB prefetchable window, an equal share of its
parent's window.  The nested switch returns about 0.6 s later and
fails:

  pci 0000:8c:00.1: BAR 2 [mem size 0x40000000 64bit pref]: can't assign; no space
  switchtec 0000:8c:00.1: probe with driver switchtec failed with error -16

The global hpmmioprefsize cannot express this.  It applies to every
hotplug bridge, it is optional space that is dropped when it does not
fit, and it has no alignment.  Here the root port window is 1036 MiB:
exactly the 1 GiB + 4 MiB + 4 MiB of the nested switch plus the 4 MiB
BAR of the top switch's management function.  Only one port may get
the space, and it has to be 1 GiB aligned.

Add "pci=hpreserve=<reservation>@<pci_dev>[; ...]" to request a minimum
size and alignment for the memory windows of specific hotplug bridges,
using the usual device specification.  A full path is more robust than
a bare BDF when downstream bus numbers change.  pbus_size_mem() folds
the reservation into the required size and alignment of the window,
so it is assigned like the resources of a device already present below
the bridge.  The runtime distribution of spare space can shrink only
the optional part, leaving the required reservation intact.

Without the parameter nothing changes.

Tested-by: Maciej Grochowski <maciej.grochowski@sony.com>
Signed-off-by: Maciej Grochowski <maciej.grochowski@sony.com>
---
 .../admin-guide/kernel-parameters.txt         |  34 ++++++
 drivers/pci/pci.c                             | 106 ++++++++++++++++++
 drivers/pci/pci.h                             |  19 ++++
 drivers/pci/setup-bus.c                       |  33 ++++++
 4 files changed, 192 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 33cd30996e47..19b98e22b10e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5232,6 +5232,40 @@ Kernel parameters
 		hpbussize=nn	The minimum amount of additional bus numbers
 				reserved for buses below a hotplug bridge.
 				Default is 1.
+		hpreserve=
+				Format:
+				<reservation>@<pci_dev>[; ...]
+				where <reservation> is one or more of
+				<key>=<value> separated by colons.
+				Reserve resources for the hotplug bridges
+				specified (in the format described above)
+				for devices that are not present yet, e.g.
+				a switch that is expected behind the bridge
+				but whose link comes up only after the
+				bridge windows have been sized.  Unlike
+				hpmmiosize and hpmmioprefsize, which add
+				optional space that is dropped if it does
+				not fit, the reservation is required space:
+				the bridge window is sized to include it,
+				like the resources of devices already
+				present below the bridge.
+				Only hotplug bridges are affected.
+				Keys:
+				  mmio=nn[KMG]	Minimum size of the
+					non-prefetchable memory window.
+				  mmioalign=nn[KMG]  Minimum alignment of
+					the non-prefetchable memory window.
+				  mmiopref=nn[KMG]  Minimum size of the
+					prefetchable memory window.
+				  mmioprefalign=nn[KMG]  Minimum alignment
+					of the prefetchable memory window.
+				Alignments must be powers of two.  For
+				example,
+				  pci=hpreserve=mmiopref=1032M:mmioprefalign=1G@0000:80:01.1/00.0/03.0
+				reserves a 1 GiB-aligned 1032 MiB
+				prefetchable window below the downstream
+				port at devfn 03.0 of the switch behind root
+				port 0000:80:01.1.
 		realloc=	Enable/disable reallocating PCI bridge resources
 				if allocations done by BIOS are too small to
 				accommodate resources required by all child
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be5f8..f6922cf0d96e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -412,6 +412,109 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 	return 1;
 }
 
+/*
+ * "pci=hpreserve=<reservation>@<pci_dev>[; ...]", set in pci_setup() and
+ * copied in pci_realloc_setup_params().
+ */
+static const char *hotplug_reserve_param;
+
+static bool pci_hotplug_reserve_key(const char *p, size_t len, const char *key)
+{
+	return strlen(key) == len && !strncmp(p, key, len);
+}
+
+/*
+ * Parse one "<key>=<value>[:<key>=<value>]*@" reservation into @res.
+ * Returns a pointer to the device specification following the '@', or
+ * NULL if the reservation cannot be parsed.
+ */
+static const char *pci_parse_hotplug_reserve(const char *p,
+					     struct pci_hotplug_reserve *res)
+{
+	unsigned long long val;
+	size_t len;
+	char *end;
+
+	memset(res, 0, sizeof(*res));
+
+	for (;;) {
+		len = strcspn(p, "=:@;");
+		if (p[len] != '=')
+			return NULL;
+
+		val = memparse(p + len + 1, &end);
+		if (end == p + len + 1)
+			return NULL;
+
+		if (pci_hotplug_reserve_key(p, len, "mmio"))
+			res->mmio_size = val;
+		else if (pci_hotplug_reserve_key(p, len, "mmioalign"))
+			res->mmio_align = val;
+		else if (pci_hotplug_reserve_key(p, len, "mmiopref"))
+			res->mmio_pref_size = val;
+		else if (pci_hotplug_reserve_key(p, len, "mmioprefalign"))
+			res->mmio_pref_align = val;
+		else
+			return NULL;
+
+		p = end + 1;
+		if (*end == '@')
+			break;
+		if (*end != ':')
+			return NULL;
+	}
+
+	if ((res->mmio_align && !is_power_of_2(res->mmio_align)) ||
+	    (res->mmio_pref_align && !is_power_of_2(res->mmio_pref_align)))
+		return NULL;
+
+	return p;
+}
+
+/**
+ * pci_get_hotplug_reserve - get the resources reserved for a hotplug bridge
+ * @bridge: the bridge
+ * @res: filled in with the reservation for @bridge
+ *
+ * Look up @bridge in the "pci=hpreserve=" kernel parameter.  Only hotplug
+ * bridges are considered.
+ *
+ * Return: true if a reservation was requested for @bridge.
+ */
+bool pci_get_hotplug_reserve(struct pci_dev *bridge,
+			     struct pci_hotplug_reserve *res)
+{
+	const char *p = hotplug_reserve_param;
+	int ret;
+
+	if (!p || !bridge->is_hotplug_bridge)
+		return false;
+
+	while (*p) {
+		p = pci_parse_hotplug_reserve(p, res);
+		if (!p) {
+			pr_err_once("PCI: Can't parse hpreserve parameter\n");
+			return false;
+		}
+
+		ret = pci_dev_str_match(bridge, p, &p);
+		if (ret == 1)
+			return true;
+		if (ret < 0) {
+			pr_err_once("PCI: Can't parse hpreserve parameter\n");
+			return false;
+		}
+
+		if (*p != ';' && *p != ',') {
+			/* End of param or invalid format */
+			break;
+		}
+		p++;
+	}
+
+	return false;
+}
+
 static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
 			      u8 pos, int cap)
 {
@@ -6840,6 +6943,8 @@ static int __init pci_setup(char *str)
 					simple_strtoul(str + 10, &str, 0);
 				if (pci_hotplug_bus_size > 0xff)
 					pci_hotplug_bus_size = DEFAULT_HOTPLUG_BUS_SIZE;
+			} else if (!strncmp(str, "hpreserve=", 10)) {
+				hotplug_reserve_param = str + 10;
 			} else if (!strncmp(str, "pcie_bus_tune_off", 17)) {
 				pcie_bus_config = PCIE_BUS_TUNE_OFF;
 			} else if (!strncmp(str, "pcie_bus_safe", 13)) {
@@ -6879,6 +6984,7 @@ static int __init pci_realloc_setup_params(void)
 					   GFP_KERNEL);
 	disable_acs_redir_param = kstrdup(disable_acs_redir_param, GFP_KERNEL);
 	config_acs_param = kstrdup(config_acs_param, GFP_KERNEL);
+	hotplug_reserve_param = kstrdup(hotplug_reserve_param, GFP_KERNEL);
 
 	return 0;
 }
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index ba3c3fddddc2..5003d2c71200 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -410,6 +410,25 @@ extern unsigned long pci_hotplug_mmio_size;
 extern unsigned long pci_hotplug_mmio_pref_size;
 extern unsigned long pci_hotplug_bus_size;
 
+/**
+ * struct pci_hotplug_reserve - resources reserved for a hotplug bridge
+ * @mmio_size: minimum size of the non-prefetchable memory window
+ * @mmio_align: minimum alignment of the non-prefetchable memory window
+ * @mmio_pref_size: minimum size of the prefetchable memory window
+ * @mmio_pref_align: minimum alignment of the prefetchable memory window
+ *
+ * Requested with "pci=hpreserve=".  A value of zero means no reservation.
+ */
+struct pci_hotplug_reserve {
+	resource_size_t mmio_size;
+	resource_size_t mmio_align;
+	resource_size_t mmio_pref_size;
+	resource_size_t mmio_pref_align;
+};
+
+bool pci_get_hotplug_reserve(struct pci_dev *bridge,
+			     struct pci_hotplug_reserve *res);
+
 static inline bool pci_is_cardbus_bridge(struct pci_dev *dev)
 {
 	return dev->hdr_type == PCI_HEADER_TYPE_CARDBUS;
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index e8c94aa1d3c1..105bf8694cdc 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1258,6 +1258,38 @@ static bool pbus_size_mem_optional(struct pci_dev *dev, int resno,
 	return true;
 }
 
+/*
+ * Apply a "pci=hpreserve=" reservation to the memory window @b_res of the
+ * bridge leading to @bus.  Unlike the hpmmiosize/hpmmioprefsize space, the
+ * reservation is sized as required space, so it is in place before the
+ * devices that need it appear behind the bridge.
+ */
+static void pbus_size_mem_reserve(struct pci_bus *bus, struct resource *b_res,
+				  resource_size_t *size,
+				  resource_size_t *min_align)
+{
+	bool pref = b_res->flags & IORESOURCE_PREFETCH;
+	struct pci_hotplug_reserve res;
+	resource_size_t rsize, ralign;
+
+	if (!bus->self || !pci_get_hotplug_reserve(bus->self, &res))
+		return;
+
+	rsize = pref ? res.mmio_pref_size : res.mmio_size;
+	ralign = pref ? res.mmio_pref_align : res.mmio_align;
+	if (rsize <= *size && ralign <= *min_align)
+		return;
+
+	*size = max(*size, rsize);
+	if (!*size)
+		return;
+	*min_align = max(*min_align, ralign);
+
+	pci_info(bus->self, "bridge window to %pR: reserving %s size %#llx align %#llx\n",
+		 &bus->busn_res, pref ? "prefetchable" : "non-prefetchable",
+		 (unsigned long long)*size, (unsigned long long)*min_align);
+}
+
 /**
  * pbus_size_mem() - Size the memory window of a given bus
  *
@@ -1349,6 +1381,7 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
 	win_align = pci_min_window_alignment(bus, b_res->flags);
 	min_align = calculate_head_align(aligns, max_order);
 	min_align = max(min_align, win_align);
+	pbus_size_mem_reserve(bus, b_res, &size, &min_align);
 	size0 = calculate_memsize(size, realloc_head ? 0 : add_size,
 				  0, win_align);
 
-- 
2.47.3

[RFC PATCH 2/2] PCI: Allow pci=hpreserve= to reserve bus numbers for a hotplug bridge

[Maciej Grochowski]

When the kernel assigns bus numbers to the bridges on a bus,
pci_scan_child_bus_extend() divides the spare bus numbers equally
between the hotplug bridges.  A switch that appears behind one of them
later cannot use more than that share.

After a hard reset of the top-level switch of a two-level
Microchip PM50052 hierarchy on v7.3-rc2, the 15 spare bus numbers below
the upstream port are split 3/3/3/3/3 between its five hotplug
downstream ports, leaving 82:03.0 with [bus 8c-8e].  The nested switch
that comes back behind it needs 11 (its upstream port's bus, its
internal bus and nine downstream ports), so part of it is unusable:

  pci 0000:8c:00.0: devices behind bridge are unusable because [bus 8d-96] cannot be assigned for them

hpbussize does not help: it applies to every hotplug bridge, so it
cannot give one port 11 bus numbers without asking the same for its
four siblings.

Add a "bus=" key to pci=hpreserve=.  A reserved hotplug bridge gets the
number of bus numbers it asks for, counted like hpbussize, and the
other hotplug bridges on the bus share what is left.  With "bus=11" on
82:03.0 the split above becomes 1/1/1/11/1.  If fewer bus numbers are
left than requested, the bridge gets what is left and a warning is
logged.

Without the parameter the distribution is unchanged.

Tested-by: Maciej Grochowski <maciej.grochowski@sony.com>
Signed-off-by: Maciej Grochowski <maciej.grochowski@sony.com>
---
 .../admin-guide/kernel-parameters.txt         | 18 ++++--
 drivers/pci/pci.c                             |  2 +
 drivers/pci/pci.h                             |  3 +
 drivers/pci/probe.c                           | 58 +++++++++++++++++--
 4 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 19b98e22b10e..fea5f1bb455f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5251,6 +5251,14 @@ Kernel parameters
 				present below the bridge.
 				Only hotplug bridges are affected.
 				Keys:
+				  bus=nn	Number of bus numbers for the
+					buses below the bridge, counted like
+					hpbussize.  When the kernel distributes
+					spare bus numbers between the hotplug
+					bridges on a bus, e.g. when a switch is
+					hot-added, the reserved bridge gets its
+					reservation first and the others share
+					what is left.
 				  mmio=nn[KMG]	Minimum size of the
 					non-prefetchable memory window.
 				  mmioalign=nn[KMG]  Minimum alignment of
@@ -5261,11 +5269,11 @@ Kernel parameters
 					of the prefetchable memory window.
 				Alignments must be powers of two.  For
 				example,
-				  pci=hpreserve=mmiopref=1032M:mmioprefalign=1G@0000:80:01.1/00.0/03.0
-				reserves a 1 GiB-aligned 1032 MiB
-				prefetchable window below the downstream
-				port at devfn 03.0 of the switch behind root
-				port 0000:80:01.1.
+				  pci=hpreserve=bus=11:mmiopref=1032M:mmioprefalign=1G@0000:80:01.1/00.0/03.0
+				reserves 11 bus numbers and a 1 GiB-aligned
+				1032 MiB prefetchable window below the
+				downstream port at devfn 03.0 of the switch
+				behind root port 0000:80:01.1.
 		realloc=	Enable/disable reallocating PCI bridge resources
 				if allocations done by BIOS are too small to
 				accommodate resources required by all child
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f6922cf0d96e..81123789f66b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -454,6 +454,8 @@ static const char *pci_parse_hotplug_reserve(const char *p,
 			res->mmio_pref_size = val;
 		else if (pci_hotplug_reserve_key(p, len, "mmioprefalign"))
 			res->mmio_pref_align = val;
+		else if (pci_hotplug_reserve_key(p, len, "bus") && val <= 0xff)
+			res->buses = val;
 		else
 			return NULL;
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 5003d2c71200..56efd486cccd 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -412,6 +412,8 @@ extern unsigned long pci_hotplug_bus_size;
 
 /**
  * struct pci_hotplug_reserve - resources reserved for a hotplug bridge
+ * @buses: bus numbers for the hierarchy below the bridge, including its
+ *	   secondary bus
  * @mmio_size: minimum size of the non-prefetchable memory window
  * @mmio_align: minimum alignment of the non-prefetchable memory window
  * @mmio_pref_size: minimum size of the prefetchable memory window
@@ -420,6 +422,7 @@ extern unsigned long pci_hotplug_bus_size;
  * Requested with "pci=hpreserve=".  A value of zero means no reservation.
  */
 struct pci_hotplug_reserve {
+	unsigned int buses;
 	resource_size_t mmio_size;
 	resource_size_t mmio_align;
 	resource_size_t mmio_pref_size;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 27008e2ea5af..086b7a549c27 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3081,10 +3081,22 @@ void __weak pcibios_fixup_bus(struct pci_bus *bus)
  * equally between hotplug-capable bridges to allow future extension of the
  * hierarchy.
  */
+static unsigned int pci_hotplug_reserved_buses(struct pci_dev *bridge)
+{
+	struct pci_hotplug_reserve res;
+
+	if (!pci_get_hotplug_reserve(bridge, &res))
+		return 0;
+
+	return res.buses;
+}
+
 static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
 					      unsigned int available_buses)
 {
 	unsigned int used_buses, normal_bridges = 0, hotplug_bridges = 0;
+	unsigned int reserved_bridges = 0, reserved_buses = 0;
+	unsigned int hotplug_share = 0;
 	unsigned int start = bus->busn_res.start;
 	unsigned int devnr, cmax, max = start;
 	struct pci_dev *dev;
@@ -3115,12 +3127,28 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
 	 * buses between hotplug bridges.
 	 */
 	for_each_pci_bridge(dev, bus) {
-		if (dev->is_hotplug_bridge)
+		if (dev->is_hotplug_bridge) {
+			unsigned int reserved = pci_hotplug_reserved_buses(dev);
+
 			hotplug_bridges++;
-		else
+			if (reserved) {
+				reserved_bridges++;
+				reserved_buses += reserved;
+			}
+		} else {
 			normal_bridges++;
+		}
 	}
 
+	/*
+	 * Hotplug bridges with a "pci=hpreserve=" bus reservation get the
+	 * number of buses they asked for; the others share what is left.
+	 */
+	if (hotplug_bridges > reserved_bridges &&
+	    available_buses > reserved_buses)
+		hotplug_share = (available_buses - reserved_buses) /
+				(hotplug_bridges - reserved_bridges);
+
 	/*
 	 * Scan bridges that are already configured. We don't touch them
 	 * unless they are misconfigured (which will be done in the second
@@ -3152,12 +3180,34 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
 			 */
 			buses = available_buses;
 		} else if (dev->is_hotplug_bridge) {
+			unsigned int reserved = pci_hotplug_reserved_buses(dev);
+
 			/*
 			 * Distribute the extra buses between hotplug
 			 * bridges if any.
 			 */
-			buses = available_buses / hotplug_bridges;
-			buses = min(buses, available_buses - used_buses + 1);
+			if (!reserved) {
+				buses = hotplug_share;
+				buses = min(buses, available_buses - used_buses + 1);
+			} else {
+				/* used_buses already includes this bridge's bus */
+				buses = available_buses + 1 > used_buses ?
+					available_buses + 1 - used_buses : 0;
+				buses = min(buses, reserved);
+			}
+
+			/* Bridges configured in the first pass keep their buses */
+			if (reserved && !dev->subordinate) {
+				/* The secondary bus is allocated even for 0 */
+				unsigned int granted = max(buses, 1U);
+
+				if (granted < reserved)
+					pci_warn(dev, "only %u of %u reserved bus numbers available\n",
+						 granted, reserved);
+				else
+					pci_info(dev, "reserving %u bus numbers\n",
+						 granted);
+			}
 		}
 
 		cmax = max;
-- 
2.47.3

Re: [RFC PATCH 1/2] PCI: Add pci=hpreserve= to reserve memory windows for a hotplug bridge

[sashiko-bot]

> A hotplug bridge is sized for the devices that are present when its
> windows are sized, plus the optional hpmmiosize/hpmmioprefsize space.
> Some topologies have devices that are known to appear later.  After a
> reset of a two-level PCIe switch hierarchy, for example, pciehp
> re-enumerates the upstream switch before the link to the nested switch
> is back up, so the downstream port leading to the nested switch is
> sized with nothing below it.  When the nested switch appears, its BARs
> do not fit.
> 
> On a two-level Microchip PM50052 (Switchtec PFX) hierarchy whose nested
> switch has an NTB function with a 1 GiB prefetchable BAR, a hard reset
> of the top-level switch on v7.3-rc2 leaves downstream port 82:03.0
> with a 206 MiB prefetchable window, an equal share of its
> parent's window.  The nested switch returns about 0.6 s later and
> fails:
> [ ... ]
> 
> Without the parameter nothing changes.
> 
> Tested-by: Maciej Grochowski <maciej.grochowski@sony.com>
> Signed-off-by: Maciej Grochowski <maciej.grochowski@sony.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002193111.51637-1-maciej.grochowski@sony.com?part=1

Re: [RFC PATCH 2/2] PCI: Allow pci=hpreserve= to reserve bus numbers for a hotplug bridge

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pci: hotplug_share calculation ignores buses used by normal bridges
- [Medium] pci: reserved bus numbers are lost if bridge is firmware-configured
--

commit 34e2d44bc1e7840b0e80a2d0ea3f891f8e316ab9
Author: Maciej Grochowski <maciej.grochowski@sony.com>

PCI: Allow pci=hpreserve= to reserve bus numbers for a hotplug bridge

When the kernel assigns bus numbers to the bridges on a bus,
pci_scan_child_bus_extend() divides the spare bus numbers equally
between the hotplug bridges. This commit adds a bus= key to
pci=hpreserve= to allow a reserved hotplug bridge to get a
requested number of buses, with others sharing the remainder.

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 27008e2ea5afc..086b7a549c27e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
[ ... ]
> @@ -3115,12 +3127,28 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
>  	 * buses between hotplug bridges.
>  	 */
>  	for_each_pci_bridge(dev, bus) {
> -		if (dev->is_hotplug_bridge)
> +		if (dev->is_hotplug_bridge) {
> +			unsigned int reserved = pci_hotplug_reserved_buses(dev);
> +
>  			hotplug_bridges++;
> -		else
> +			if (reserved) {
> +				reserved_bridges++;
> +				reserved_buses += reserved;
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Will these reserved bus numbers be lost if the bridge was already configured by
firmware?

When a user reserves buses for a bridge that was assigned a smaller number of
secondary buses by firmware, the full reservation is deducted from the shared
pool here.

However, if the bridge is already configured, it retains its original
allocation and the remaining reserved buses are never added to used_buses.

> +			}
> +		} else {
>  			normal_bridges++;
> +		}
>  	}
>  
> +	/*
> +	 * Hotplug bridges with a "pci=hpreserve=" bus reservation get the
> +	 * number of buses they asked for; the others share what is left.
> +	 */
> +	if (hotplug_bridges > reserved_bridges &&
> +	    available_buses > reserved_buses)
> +		hotplug_share = (available_buses - reserved_buses) /
> +				(hotplug_bridges - reserved_bridges);
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does this calculation over-allocate buses to non-reserved hotplug bridges?

When the parent bus contains a mix of normal bridges and non-reserved hotplug
bridges, available_buses does not account for buses already taken by the
normal bridges.

This inflates the calculated hotplug_share.

> +
>  	/*
>  	 * Scan bridges that are already configured. We don't touch them
>  	 * unless they are misconfigured (which will be done in the second
[ ... ]
> @@ -3152,12 +3180,34 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
>  			 */
>  			buses = available_buses;
>  		} else if (dev->is_hotplug_bridge) {
> +			unsigned int reserved = pci_hotplug_reserved_buses(dev);
> +
>  			/*
>  			 * Distribute the extra buses between hotplug
>  			 * bridges if any.
>  			 */
> -			buses = available_buses / hotplug_bridges;
> -			buses = min(buses, available_buses - used_buses + 1);
> +			if (!reserved) {
> +				buses = hotplug_share;
                                ^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
If a non-reserved bridge is processed before the reserved bridge, it consumes
this inflated hotplug_share. This can deplete the pool and leave the reserved
bridge with fewer buses than requested.

> +				buses = min(buses, available_buses - used_buses + 1);
> +			} else {
> +				/* used_buses already includes this bridge's bus */
> +				buses = available_buses + 1 > used_buses ?
> +					available_buses + 1 - used_buses : 0;
> +				buses = min(buses, reserved);
> +			}
> +
> +			/* Bridges configured in the first pass keep their buses */
> +			if (reserved && !dev->subordinate) {
                                        ^^^^^^^^^^^^^^^^

[Severity: Medium]
If the bridge is already firmware-configured, dev->subordinate evaluates to
true. This suppresses the warning about failing to fulfill the reservation,
masking the silent loss of the reserved bus numbers deducted from the pool
earlier.

> +				/* The secondary bus is allocated even for 0 */
> +				unsigned int granted = max(buses, 1U);
> +
> +				if (granted < reserved)
> +					pci_warn(dev, "only %u of %u reserved bus numbers available\n",
> +						 granted, reserved);
> +				else
> +					pci_info(dev, "reserving %u bus numbers\n",
> +						 granted);
> +			}
>  		}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002193111.51637-1-maciej.grochowski@sony.com?part=2

Re: [RFC PATCH 1/2] PCI: Add pci=hpreserve= to reserve memory windows for a hotplug bridge

[Ilpo Järvinen]

On Fri, 2 Oct 2026, Maciej Grochowski wrote:

> A hotplug bridge is sized for the devices that are present when its
> windows are sized, plus the optional hpmmiosize/hpmmioprefsize space.
> Some topologies have devices that are known to appear later.  After a
> reset of a two-level PCIe switch hierarchy, for example, pciehp
> re-enumerates the upstream switch before the link to the nested switch
> is back up, so the downstream port leading to the nested switch is
> sized with nothing below it.  When the nested switch appears, its BARs
> do not fit.
> 
> On a two-level Microchip PM50052 (Switchtec PFX) hierarchy whose nested
> switch has an NTB function with a 1 GiB prefetchable BAR, a hard reset
> of the top-level switch on v7.3-rc2 leaves downstream port 82:03.0
> with a 206 MiB prefetchable window, an equal share of its
> parent's window.  The nested switch returns about 0.6 s later and
> fails:
> 
>   pci 0000:8c:00.1: BAR 2 [mem size 0x40000000 64bit pref]: can't assign; no space
>   switchtec 0000:8c:00.1: probe with driver switchtec failed with error -16
> 
> The global hpmmioprefsize cannot express this.  It applies to every
> hotplug bridge, it is optional space that is dropped when it does not
> fit, and it has no alignment.  Here the root port window is 1036 MiB:
> exactly the 1 GiB + 4 MiB + 4 MiB of the nested switch plus the 4 MiB
> BAR of the top switch's management function.  Only one port may get
> the space, and it has to be 1 GiB aligned.
> 
> Add "pci=hpreserve=<reservation>@<pci_dev>[; ...]" to request a minimum

Hi,

Why is this a new parameter instead of adding the per device functionality 
into hpmmio{,pref}size=x ?

> size and alignment for the memory windows of specific hotplug bridges,
> using the usual device specification.  A full path is more robust than
> a bare BDF when downstream bus numbers change.  pbus_size_mem() folds
> the reservation into the required size and alignment of the window,
> so it is assigned like the resources of a device already present below
> the bridge.  The runtime distribution of spare space can shrink only
> the optional part, leaving the required reservation intact.
> 
> Without the parameter nothing changes.
> 
> Tested-by: Maciej Grochowski <maciej.grochowski@sony.com>

The usual assumption the patch submitter has actually tested the patch so 
normal custom is to not include self tested-bys.

-- 
 i.

> Signed-off-by: Maciej Grochowski <maciej.grochowski@sony.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  34 ++++++
>  drivers/pci/pci.c                             | 106 ++++++++++++++++++
>  drivers/pci/pci.h                             |  19 ++++
>  drivers/pci/setup-bus.c                       |  33 ++++++
>  4 files changed, 192 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 33cd30996e47..19b98e22b10e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5232,6 +5232,40 @@ Kernel parameters
>  		hpbussize=nn	The minimum amount of additional bus numbers
>  				reserved for buses below a hotplug bridge.
>  				Default is 1.
> +		hpreserve=
> +				Format:
> +				<reservation>@<pci_dev>[; ...]
> +				where <reservation> is one or more of
> +				<key>=<value> separated by colons.
> +				Reserve resources for the hotplug bridges
> +				specified (in the format described above)
> +				for devices that are not present yet, e.g.
> +				a switch that is expected behind the bridge
> +				but whose link comes up only after the
> +				bridge windows have been sized.  Unlike
> +				hpmmiosize and hpmmioprefsize, which add
> +				optional space that is dropped if it does
> +				not fit, the reservation is required space:
> +				the bridge window is sized to include it,
> +				like the resources of devices already
> +				present below the bridge.
> +				Only hotplug bridges are affected.
> +				Keys:
> +				  mmio=nn[KMG]	Minimum size of the
> +					non-prefetchable memory window.
> +				  mmioalign=nn[KMG]  Minimum alignment of
> +					the non-prefetchable memory window.
> +				  mmiopref=nn[KMG]  Minimum size of the
> +					prefetchable memory window.
> +				  mmioprefalign=nn[KMG]  Minimum alignment
> +					of the prefetchable memory window.
> +				Alignments must be powers of two.  For
> +				example,
> +				  pci=hpreserve=mmiopref=1032M:mmioprefalign=1G@0000:80:01.1/00.0/03.0
> +				reserves a 1 GiB-aligned 1032 MiB
> +				prefetchable window below the downstream
> +				port at devfn 03.0 of the switch behind root
> +				port 0000:80:01.1.
>  		realloc=	Enable/disable reallocating PCI bridge resources
>  				if allocations done by BIOS are too small to
>  				accommodate resources required by all child
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be5f8..f6922cf0d96e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -412,6 +412,109 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
>  	return 1;
>  }
>  
> +/*
> + * "pci=hpreserve=<reservation>@<pci_dev>[; ...]", set in pci_setup() and
> + * copied in pci_realloc_setup_params().
> + */
> +static const char *hotplug_reserve_param;
> +
> +static bool pci_hotplug_reserve_key(const char *p, size_t len, const char *key)
> +{
> +	return strlen(key) == len && !strncmp(p, key, len);
> +}
> +
> +/*
> + * Parse one "<key>=<value>[:<key>=<value>]*@" reservation into @res.
> + * Returns a pointer to the device specification following the '@', or
> + * NULL if the reservation cannot be parsed.
> + */
> +static const char *pci_parse_hotplug_reserve(const char *p,
> +					     struct pci_hotplug_reserve *res)
> +{
> +	unsigned long long val;
> +	size_t len;
> +	char *end;
> +
> +	memset(res, 0, sizeof(*res));
> +
> +	for (;;) {
> +		len = strcspn(p, "=:@;");
> +		if (p[len] != '=')
> +			return NULL;
> +
> +		val = memparse(p + len + 1, &end);
> +		if (end == p + len + 1)
> +			return NULL;
> +
> +		if (pci_hotplug_reserve_key(p, len, "mmio"))
> +			res->mmio_size = val;
> +		else if (pci_hotplug_reserve_key(p, len, "mmioalign"))
> +			res->mmio_align = val;
> +		else if (pci_hotplug_reserve_key(p, len, "mmiopref"))
> +			res->mmio_pref_size = val;
> +		else if (pci_hotplug_reserve_key(p, len, "mmioprefalign"))
> +			res->mmio_pref_align = val;
> +		else
> +			return NULL;
> +
> +		p = end + 1;
> +		if (*end == '@')
> +			break;
> +		if (*end != ':')
> +			return NULL;
> +	}
> +
> +	if ((res->mmio_align && !is_power_of_2(res->mmio_align)) ||
> +	    (res->mmio_pref_align && !is_power_of_2(res->mmio_pref_align)))
> +		return NULL;
> +
> +	return p;
> +}
> +
> +/**
> + * pci_get_hotplug_reserve - get the resources reserved for a hotplug bridge
> + * @bridge: the bridge
> + * @res: filled in with the reservation for @bridge
> + *
> + * Look up @bridge in the "pci=hpreserve=" kernel parameter.  Only hotplug
> + * bridges are considered.
> + *
> + * Return: true if a reservation was requested for @bridge.
> + */
> +bool pci_get_hotplug_reserve(struct pci_dev *bridge,
> +			     struct pci_hotplug_reserve *res)
> +{
> +	const char *p = hotplug_reserve_param;
> +	int ret;
> +
> +	if (!p || !bridge->is_hotplug_bridge)
> +		return false;
> +
> +	while (*p) {
> +		p = pci_parse_hotplug_reserve(p, res);
> +		if (!p) {
> +			pr_err_once("PCI: Can't parse hpreserve parameter\n");
> +			return false;
> +		}
> +
> +		ret = pci_dev_str_match(bridge, p, &p);
> +		if (ret == 1)
> +			return true;
> +		if (ret < 0) {
> +			pr_err_once("PCI: Can't parse hpreserve parameter\n");
> +			return false;
> +		}
> +
> +		if (*p != ';' && *p != ',') {
> +			/* End of param or invalid format */
> +			break;
> +		}
> +		p++;
> +	}
> +
> +	return false;
> +}
> +
>  static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
>  			      u8 pos, int cap)
>  {
> @@ -6840,6 +6943,8 @@ static int __init pci_setup(char *str)
>  					simple_strtoul(str + 10, &str, 0);
>  				if (pci_hotplug_bus_size > 0xff)
>  					pci_hotplug_bus_size = DEFAULT_HOTPLUG_BUS_SIZE;
> +			} else if (!strncmp(str, "hpreserve=", 10)) {
> +				hotplug_reserve_param = str + 10;
>  			} else if (!strncmp(str, "pcie_bus_tune_off", 17)) {
>  				pcie_bus_config = PCIE_BUS_TUNE_OFF;
>  			} else if (!strncmp(str, "pcie_bus_safe", 13)) {
> @@ -6879,6 +6984,7 @@ static int __init pci_realloc_setup_params(void)
>  					   GFP_KERNEL);
>  	disable_acs_redir_param = kstrdup(disable_acs_redir_param, GFP_KERNEL);
>  	config_acs_param = kstrdup(config_acs_param, GFP_KERNEL);
> +	hotplug_reserve_param = kstrdup(hotplug_reserve_param, GFP_KERNEL);
>  
>  	return 0;
>  }
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index ba3c3fddddc2..5003d2c71200 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -410,6 +410,25 @@ extern unsigned long pci_hotplug_mmio_size;
>  extern unsigned long pci_hotplug_mmio_pref_size;
>  extern unsigned long pci_hotplug_bus_size;
>  
> +/**
> + * struct pci_hotplug_reserve - resources reserved for a hotplug bridge
> + * @mmio_size: minimum size of the non-prefetchable memory window
> + * @mmio_align: minimum alignment of the non-prefetchable memory window
> + * @mmio_pref_size: minimum size of the prefetchable memory window
> + * @mmio_pref_align: minimum alignment of the prefetchable memory window
> + *
> + * Requested with "pci=hpreserve=".  A value of zero means no reservation.
> + */
> +struct pci_hotplug_reserve {
> +	resource_size_t mmio_size;
> +	resource_size_t mmio_align;
> +	resource_size_t mmio_pref_size;
> +	resource_size_t mmio_pref_align;
> +};
> +
> +bool pci_get_hotplug_reserve(struct pci_dev *bridge,
> +			     struct pci_hotplug_reserve *res);
> +
>  static inline bool pci_is_cardbus_bridge(struct pci_dev *dev)
>  {
>  	return dev->hdr_type == PCI_HEADER_TYPE_CARDBUS;
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index e8c94aa1d3c1..105bf8694cdc 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1258,6 +1258,38 @@ static bool pbus_size_mem_optional(struct pci_dev *dev, int resno,
>  	return true;
>  }
>  
> +/*
> + * Apply a "pci=hpreserve=" reservation to the memory window @b_res of the
> + * bridge leading to @bus.  Unlike the hpmmiosize/hpmmioprefsize space, the
> + * reservation is sized as required space, so it is in place before the
> + * devices that need it appear behind the bridge.
> + */
> +static void pbus_size_mem_reserve(struct pci_bus *bus, struct resource *b_res,
> +				  resource_size_t *size,
> +				  resource_size_t *min_align)
> +{
> +	bool pref = b_res->flags & IORESOURCE_PREFETCH;
> +	struct pci_hotplug_reserve res;
> +	resource_size_t rsize, ralign;
> +
> +	if (!bus->self || !pci_get_hotplug_reserve(bus->self, &res))
> +		return;
> +
> +	rsize = pref ? res.mmio_pref_size : res.mmio_size;
> +	ralign = pref ? res.mmio_pref_align : res.mmio_align;
> +	if (rsize <= *size && ralign <= *min_align)
> +		return;
> +
> +	*size = max(*size, rsize);
> +	if (!*size)
> +		return;
> +	*min_align = max(*min_align, ralign);
> +
> +	pci_info(bus->self, "bridge window to %pR: reserving %s size %#llx align %#llx\n",
> +		 &bus->busn_res, pref ? "prefetchable" : "non-prefetchable",
> +		 (unsigned long long)*size, (unsigned long long)*min_align);
> +}
> +
>  /**
>   * pbus_size_mem() - Size the memory window of a given bus
>   *
> @@ -1349,6 +1381,7 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
>  	win_align = pci_min_window_alignment(bus, b_res->flags);
>  	min_align = calculate_head_align(aligns, max_order);
>  	min_align = max(min_align, win_align);
> +	pbus_size_mem_reserve(bus, b_res, &size, &min_align);
>  	size0 = calculate_memsize(size, realloc_head ? 0 : add_size,
>  				  0, win_align);
>  
> 

Re: [RFC PATCH 1/2] PCI: Add pci=hpreserve= to reserve memory windows for a hotplug bridge

[Maciej Grochowski]

On Mon, 5 Oct 2026, Ilpo Järvinen wrote:
> > Add "pci=hpreserve=<reservation>@<pci_dev>[; ...]" to request a minimum
>
> Why is this a new parameter instead of adding the per device functionality
> into hpmmio{,pref}size=x ?

Hi Ilpo,

Mainly to avoid giving one option two meanings.  As I read the
allocator, on the hotplug path (pci_assign_unassigned_bridge_resources())
hpmmio{,pref}size is only an optional size: it is passed as add_size, so
it lands on the realloc list, the required-only retry can drop it, and
pci_bridge_distribute_available_resources() then sets each empty
hotplug port's window to an equal share of what remains of the parent
window.  It also has no alignment of its own.  This case needs a required, aligned size
on one specific port, so I kept it separate rather than have
"hpmmioprefsize=1032M" mean "best effort" globally and "must fit" per
device.  I also wanted the bus number reservation in the same entry.

That said, the separation isn't essential, and I'd be happy to fold it
into the existing options for v2, e.g.:

  pci=hpmmioprefsize=1032M@0000:80:01.1/00.0/03.0,hpbussize=11@0000:80:01.1/00.0/03.0

where a bare value keeps today's global, optional meaning and a value
with a device specification applies only to the matching hotplug
bridges.  "[; ...]" would also cover the cover letter's grouping
question (same size for several paths), at the cost of repeating the
path once per option.  A couple of questions before I respin:

1. Should a per-device size be required, as in this RFC, or optional
   like the global one?  My understanding is that optional is not enough
   here, since the retry can drop it and distribution hands each empty
   port only an equal share of the parent window.  I haven't isolated
   this in a test with valid bus numbers; the run I did with the global
   "hpmmioprefsize=1032M,hpbussize=11" failed earlier on bus numbers
   (hpbussize is a per-bridge minimum, so the first ports drained the
   pool).  If you'd rather keep the semantics uniform, I could make it
   optional and teach distribution to honour it, but I expect that to be
   a bigger change.

2. How far should the @<pci_dev> form reach?  This case needs it only
   on hpmmioprefsize and hpbussize; I'd add hpmmiosize too.  hpiosize and
   hpmemsize are a few more parser branches, but nothing here exercises
   them.  Add them anyway, or only the knobs with a real need?

> > Tested-by: Maciej Grochowski <maciej.grochowski@sony.com>
>
> The usual assumption the patch submitter has actually tested the patch so
> normal custom is to not include self tested-bys.

Understood, I'll drop it.

Thanks,
Maciej