diff mbox series

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

Message ID 20261002193111.51637-2-maciej.grochowski@sony.com
State New
Headers show
Series PCI: Reserve resources for a delayed hotplug subtree | expand

Commit Message

Maciej Grochowski Oct. 2, 2026, 7:31 p.m. UTC
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(+)

Comments

sashiko-bot@kernel.org Oct. 3, 2026, 1:33 a.m. UTC | #1
> 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!
Ilpo Järvinen Oct. 5, 2026, 8:39 a.m. UTC | #2
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.
Maciej Grochowski Oct. 7, 2026, 1:35 p.m. UTC | #3
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
diff mbox series

Patch

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);