[RESEND RFC PATCH] PCI: Get the hotplug bridge window sizes from DT

[RESEND RFC PATCH] PCI: Get the hotplug bridge window sizes from DT

[Manivannan Sadhasivam]

Most of the PCI hotplug bridges used in Devicetree platforms do not have
their bridge windows populated by firmware. So the PCI core allocates the
bridge windows based on the requirements below:

1. Total size needed by the downstream devices
2. Default size (2MiB for MMIO, 256 bytes for IO) if no devices are
   available during the initial bus scan
3. Command line params like hpmemsize etc...

If no devices are available during the initial bus scan, then the PCI core
allocates the default conservative 2MiB MMIO bridge windows. But this size
sometimes becomes insufficient when a BAR hungry device like a GPU gets
plugged in later. So users have to resort to command line params to
increase the window sizes. But this is very inconvenient and causes a lot
of pain in distro integration.

To address this issue, parse the 'ranges' property from the bridge DT node
(if available) and use the sizes for each bridge window. This works well
because the 'ranges' property is already allowed by the DT binding for PCI
bridges and it lets system designers pre-define the hotplug bridge window
size requirements in firmware.

The DT derived sizes are applied as the additional (hotplug) reservation for
each window. Command line parameters (hpmemsize etc...) continue to take
precedence over DT.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---

[Resending the patch due to mailserver issue at my end]

Sending this patch as RFC because I'm not 100% sure if 'ranges' if the correct
property here. I chose this property as it is already allowed for PCI bridges
and allows specifying the window sizes. But for this usecase, bridge start
addresses specified with this property are ignored.

Other option would be to introduce a new property to specify the bridge window
size.

But nevertheless, having a DT property for specifying the window sizes will help
with usecases like USB4 PCIe Tunnelling, multi-Root Port setups where firmware
doesn't pre-allocate the bridge windows.

 drivers/pci/pci.c       |  3 --
 drivers/pci/pci.h       |  4 +++
 drivers/pci/setup-bus.c | 75 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d34266651ad0..5b6474a18b51 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -102,9 +102,6 @@ bool pci_reset_supported(struct pci_dev *dev)
 int pci_domains_supported = 1;
 #endif
 
-#define DEFAULT_HOTPLUG_IO_SIZE		(256)
-#define DEFAULT_HOTPLUG_MMIO_SIZE	(2*1024*1024)
-#define DEFAULT_HOTPLUG_MMIO_PREF_SIZE	(2*1024*1024)
 /* hpiosize=nn can override this */
 unsigned long pci_hotplug_io_size  = DEFAULT_HOTPLUG_IO_SIZE;
 /*
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4a14f88e543a..055c9b8c052b 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -414,6 +414,10 @@ extern unsigned long pci_hotplug_mmio_size;
 extern unsigned long pci_hotplug_mmio_pref_size;
 extern unsigned long pci_hotplug_bus_size;
 
+#define DEFAULT_HOTPLUG_IO_SIZE		(256)
+#define DEFAULT_HOTPLUG_MMIO_SIZE	(2*1024*1024)
+#define DEFAULT_HOTPLUG_MMIO_PREF_SIZE	(2*1024*1024)
+
 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 4cf120ebe5ad..3dc82e3d3fd5 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -29,6 +29,8 @@
 #include <linux/sizes.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 #include "pci.h"
 
 #define PCI_RES_TYPE_MASK \
@@ -1385,6 +1387,54 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
 	}
 }
 
+/*
+ * pci_bridge_of_get_window - Get bridge window from DT.
+ *
+ * If the bridge's DT node has a "ranges" property, then parse the property and
+ * return the bridge window sizes. This allows per-bridge resource allocation
+ * from DT.
+ *
+ * Return: 'true' if a valid 'ranges' property was found in DT node, with bridge
+ * window resources populated in @io, @mmio, @mmio_pref arguments. 'false'
+ * otherwise.
+ */
+static bool pci_bridge_of_get_window(struct pci_dev *dev,
+				     struct resource *io,
+				     struct resource *mmio,
+				     struct resource *mmio_pref)
+{
+	struct device_node *np = dev->dev.of_node;
+	struct of_pci_range_parser parser;
+	struct of_pci_range range;
+	bool found = false;
+
+	if (!np || !of_property_present(np, "ranges"))
+		return false;
+
+	if (of_pci_range_parser_init(&parser, np))
+		return false;
+
+	for_each_of_pci_range(&parser, &range) {
+		struct resource *res;
+
+		if (range.flags & IORESOURCE_IO)
+			res = io;
+		else if (range.flags & IORESOURCE_PREFETCH)
+			res = mmio_pref;
+		else if (range.flags & IORESOURCE_MEM)
+			res = mmio;
+		else
+			continue;
+
+		res->flags = range.flags;
+		res->start = range.cpu_addr;
+		res->end = range.cpu_addr + range.size - 1;
+		found = true;
+	}
+
+	return found;
+}
+
 void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 {
 	struct pci_dev *dev;
@@ -1430,9 +1480,34 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 	case PCI_HEADER_TYPE_BRIDGE:
 		pci_bridge_check_ranges(bus);
 		if (bus->self->is_hotplug_bridge) {
+			struct resource dt_io = {}, dt_mmio = {}, dt_mmio_pref = {};
+
 			additional_io_size  = pci_hotplug_io_size;
 			additional_mmio_size = pci_hotplug_mmio_size;
 			additional_mmio_pref_size = pci_hotplug_mmio_pref_size;
+
+			/*
+			 * If the bridge has a DT ranges property, use that to
+			 * override the default window sizes. Command line
+			 * parameters (hpmemsize etc...) take precedence over
+			 * DT.
+			 */
+			if (pci_bridge_of_get_window(bus->self,
+						     &dt_io, &dt_mmio,
+						     &dt_mmio_pref)) {
+				if (dt_io.flags &&
+				    pci_hotplug_io_size == DEFAULT_HOTPLUG_IO_SIZE)
+					additional_io_size =
+						resource_size(&dt_io);
+				if (dt_mmio.flags &&
+				    pci_hotplug_mmio_size == DEFAULT_HOTPLUG_MMIO_SIZE)
+					additional_mmio_size =
+						resource_size(&dt_mmio);
+				if (dt_mmio_pref.flags &&
+				    pci_hotplug_mmio_pref_size == DEFAULT_HOTPLUG_MMIO_PREF_SIZE)
+					additional_mmio_pref_size =
+						resource_size(&dt_mmio_pref);
+			}
 		}
 		fallthrough;
 	default:
-- 
2.43.0

Re: [RESEND RFC PATCH] PCI: Get the hotplug bridge window sizes from DT

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Command-line parameters matching default hotplug window sizes are incorrectly overridden by Device Tree sizes.
--

commit a20e61c301c97bb0b8018d5f602c3d28d1cc752e
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

PCI: Get the hotplug bridge window sizes from DT

This commit allows parsing the 'ranges' property from PCI bridge device tree
nodes to define hotplug bridge window sizes based on firmware requirements.
The DT sizes apply as additional reservations for the bridge windows.
Command-line overrides are intended to continue taking precedence over the
DT settings.

> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index c0a949f2c9956..3fbd097728bd6 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c

[ ... ]

> @@ -1430,9 +1480,34 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
>  	case PCI_HEADER_TYPE_BRIDGE:
>  		pci_bridge_check_ranges(bus);
>  		if (bus->self->is_hotplug_bridge) {
> +			struct resource dt_io = {}, dt_mmio = {}, dt_mmio_pref = {};
> +
>  			additional_io_size  = pci_hotplug_io_size;
>  			additional_mmio_size = pci_hotplug_mmio_size;
>  			additional_mmio_pref_size = pci_hotplug_mmio_pref_size;
> +
> +			/*
> +			 * If the bridge has a DT ranges property, use that to
> +			 * override the default window sizes. Command line
> +			 * parameters (hpmemsize etc...) take precedence over
> +			 * DT.
> +			 */
> +			if (pci_bridge_of_get_window(bus->self,
> +						     &dt_io, &dt_mmio,
> +						     &dt_mmio_pref)) {
> +				if (dt_io.flags &&
> +				    pci_hotplug_io_size == DEFAULT_HOTPLUG_IO_SIZE)
> +					additional_io_size =
> +						resource_size(&dt_io);
> +				if (dt_mmio.flags &&
> +				    pci_hotplug_mmio_size == DEFAULT_HOTPLUG_MMIO_SIZE)

[Severity: Medium]
Will this correctly honor command line parameters if the user explicitly
requests the default size (e.g., pci=hpmemsize=2M)?

The commit message states that command line parameters continue to take
precedence over DT. However, checking against the default macros here fails
to distinguish between an unconfigured state and an explicitly configured
value that equals the default size.

If a user explicitly passes pci=hpmemsize=2M to restrict a large DT-provided
window size, wouldn't this condition incorrectly ignore their command-line
parameter and apply the DT size instead?

> +					additional_mmio_size =
> +						resource_size(&dt_mmio);
> +				if (dt_mmio_pref.flags &&
> +				    pci_hotplug_mmio_pref_size == DEFAULT_HOTPLUG_MMIO_PREF_SIZE)
> +					additional_mmio_pref_size =
> +						resource_size(&dt_mmio_pref);
> +			}
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706130230.546108-1-manivannan.sadhasivam@oss.qualcomm.com?part=1