[PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Ilpo Järvinen]

Hi,

Here's a series for Geert to test if this fixes the improper coalescing
of resources as was experienced with the pci_add_resource() change (I
know the breaking change was pulled before 6.18 main PR but I'd want to
retry it later once the known issues have been addressed). The expected
result is there'll be two adjacent host bridge resources in the
resource tree as the different name should disallow coalescing them
together, and therefore BAR0 has a window into which it belongs to.

Generic info for the series:

PCI host bridge windows were coalesced in place into one of the structs
on the resources list. The host bridge window coalescing code does not
know who holds references and still needs the struct resource it's
coalescing from/to so it is safer to perform coalescing into entirely
a new struct resource instead and leave the old resource addresses as
they were.

The checks when coalescing is allowed are also made stricter so that
only resources that have identical the metadata can be coalesced.

As a bonus, there's also a bit of framework to easily create kunit
tests for resource tree functions (beyond just resource_coalesce()).

Ilpo Järvinen (3):
  PCI: Refactor host bridge window coalescing loop to use prev
  PCI: Do not coalesce host bridge resource structs in place
  resource, kunit: add test case for resource_coalesce()

 drivers/pci/probe.c          |  26 +++----
 include/linux/ioport.h       |   5 ++
 include/linux/resource_ext.h |   3 +
 kernel/resource.c            | 139 ++++++++++++++++++++++++++++++++++-
 kernel/resource_kunit.c      | 121 ++++++++++++++++++++++++++++++
 5 files changed, 279 insertions(+), 15 deletions(-)


base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.39.5

[PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev

[Ilpo Järvinen]

pci_register_host_bridge() has loop for coalescing host bridge windows
that are contiguous. It loops through all but the last entry and if
necessary, coalesces the current entry with the next.

The resource coalescing modifies the resources in place which can cause
issues if other parts of the kernel hold a pointer to the same
resource. This problem was demonstrated to happen when trying to
address an issue with the host bridge window setup on R-Car M2-W (see
the link below).

An upcoming change will perform a safe merge of the resources within a
new function in resource.c. That results in removing both old resource
entries from the resources list and despite using
resource_list_for_each_entry_safe(), the loop will no longer hold valid
pointer to any resources list as the next entry is removed from the
list.

Alter the loop to look back instead of ahead. That is, change next to
prev. When merging previous with the current resource, the next
resource remains valid so resource_list_for_each_entry_safe() is able
to continue iterating the list.

Link: https://lore.kernel.org/linux-pci/CAMuHMdVgCHU80mRm1Vwo6GFgNAtQcf50yHBz_oAk4TrtjcMpYg@mail.gmail.com/
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/probe.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index f41128f91ca7..04523dea7d96 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -952,11 +952,11 @@ static bool pci_preserve_config(struct pci_host_bridge *host_bridge)
 static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 {
 	struct device *parent = bridge->dev.parent;
-	struct resource_entry *window, *next, *n;
+	struct resource_entry *window, *prev, *n;
 	struct pci_bus *bus, *b;
-	resource_size_t offset, next_offset;
+	resource_size_t offset, prev_offset;
 	LIST_HEAD(resources);
-	struct resource *res, *next_res;
+	struct resource *res, *prev_res;
 	bool bus_registered = false;
 	char addr[64], *fmt;
 	const char *name;
@@ -1049,21 +1049,21 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 
 	/* Coalesce contiguous windows */
 	resource_list_for_each_entry_safe(window, n, &resources) {
-		if (list_is_last(&window->node, &resources))
-			break;
+		if (list_is_first(&window->node, &resources))
+			continue;
 
-		next = list_next_entry(window, node);
+		prev = list_prev_entry(window, node);
 		offset = window->offset;
 		res = window->res;
-		next_offset = next->offset;
-		next_res = next->res;
+		prev_offset = prev->offset;
+		prev_res = prev->res;
 
-		if (res->flags != next_res->flags || offset != next_offset)
+		if (prev_res->flags != res->flags || prev_offset != offset)
 			continue;
 
-		if (res->end + 1 == next_res->start) {
-			next_res->start = res->start;
-			res->flags = res->start = res->end = 0;
+		if (prev_res->end + 1 == res->start) {
+			res->start = prev_res->start;
+			prev_res->flags = prev_res->start = prev_res->end = 0;
 		}
 	}
 
-- 
2.39.5

[PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place

[Ilpo Järvinen]

The resource coalescing for host bridge windows in
pci_register_host_bridge() can alter the underlying struct resource
which is inherently dangerous as this code has no knowledge of who else
holds a pointer the same resource.

Merge the struct resource inside a newly added
resource_list_merge_entries() which uses the internal __res member of
the struct resource_entry to store the merged resource, thus preserving
the original resource structs.

Link: https://lore.kernel.org/linux-pci/CAMuHMdUjhq2ZLFyMYv9KYRW8brsvXMH5xb5NW8shsHJmx7w2QQ@mail.gmail.com/
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/probe.c          |   8 +--
 include/linux/resource_ext.h |   3 +
 kernel/resource.c            | 135 ++++++++++++++++++++++++++++++++++-
 3 files changed, 140 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 04523dea7d96..053bffc17146 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1061,10 +1061,10 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 		if (prev_res->flags != res->flags || prev_offset != offset)
 			continue;
 
-		if (prev_res->end + 1 == res->start) {
-			res->start = prev_res->start;
-			prev_res->flags = prev_res->start = prev_res->end = 0;
-		}
+		if (prev_res->end + 1 != res->start)
+			continue;
+
+		resource_list_merge_entries(prev, window);
 	}
 
 	/* Add initial resources to the bus */
diff --git a/include/linux/resource_ext.h b/include/linux/resource_ext.h
index ff0339df56af..4d6f2a926e6d 100644
--- a/include/linux/resource_ext.h
+++ b/include/linux/resource_ext.h
@@ -60,6 +60,9 @@ resource_list_destroy_entry(struct resource_entry *entry)
 	resource_list_free_entry(entry);
 }
 
+struct resource_entry *resource_list_merge_entries(struct resource_entry *entry,
+						   struct resource_entry *next);
+
 #define resource_list_for_each_entry(entry, list)	\
 	list_for_each_entry((entry), (list), node)
 
diff --git a/kernel/resource.c b/kernel/resource.c
index f9bb5481501a..c6e1872abb78 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -10,6 +10,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/cleanup.h>
 #include <linux/export.h>
 #include <linux/errno.h>
 #include <linux/ioport.h>
@@ -206,6 +207,13 @@ static struct resource * __request_resource(struct resource *root, struct resour
 	}
 }
 
+static void resource_clear_tree_links(struct resource *res)
+{
+	res->parent = NULL;
+	res->child = NULL;
+	res->sibling = NULL;
+}
+
 static int __release_resource(struct resource *old, bool release_child)
 {
 	struct resource *tmp, **p, *chd;
@@ -265,6 +273,101 @@ void release_child_resources(struct resource *r)
 	write_unlock(&resource_lock);
 }
 
+/**
+ * resource_mergeable - Test if resources are contiguous and can be merged
+ * @r1: first resource
+ * @r2: second resource
+ *
+ * Tests @r1 is followed by @r2 contiguously and share the metadata.
+ *
+ * Return: %true if resources are mergeable non-destructively.
+ */
+static bool resource_mergeable(struct resource *r1, struct resource *r2)
+{
+	if ((r1->flags != r2->flags) ||
+	    (r1->desc != r2->desc) ||
+	    (r1->parent != r2->parent) ||
+	    (r1->end + 1 != r2->start))
+		return false;
+
+	if (r1->name == r2->name)
+		return true;
+
+	if (r1->name && r2->name && !strcmp(r1->name, r2->name))
+		return true;
+
+	return false;
+}
+
+static int resource_coalesce(struct resource *res, struct resource *next_res,
+			     struct resource *new_res)
+{
+	struct resource *parent, *tmp;
+	struct resource **p, **c;
+
+	if (!resource_mergeable(res, next_res))
+		return -EINVAL;
+
+	guard(write_lock)(&resource_lock);
+	parent = res->parent;
+
+	if (parent != next_res->parent)
+		return -EINVAL;
+
+	if (parent && res->sibling != next_res)
+		return -EINVAL;
+
+	new_res->start = res->start;
+	new_res->end = next_res->end;
+	new_res->name = res->name;
+	new_res->flags = res->flags;
+	new_res->desc = res->desc;
+
+	if (!parent)
+		return 0;
+
+	/* prepare for step 2), find res & next_res from child/sibling chain. */
+	p = &parent->child;
+	while (1) {
+		tmp = *p;
+		if (tmp == res)
+			break;
+
+		/*  No res in child/sibling, the resource tree is corrupted! */
+		if (WARN_ON_ONCE(!tmp))
+			return -EINVAL;
+
+		p = &tmp->sibling;
+	}
+
+	/* 1) set the parent */
+	new_res->parent = parent;
+
+	/* 2) inject into parent's child/sibling chain */
+	*p = new_res;
+	new_res->sibling = next_res->sibling;
+
+	/* 3) move children over and switch them to the new parent. */
+	c = &new_res->child;
+	*c = res->child;
+	while (*c) {
+		tmp = *c;
+		tmp->parent = new_res;
+		c = &tmp->sibling;
+	}
+	*c = next_res->child;
+	while (*c) {
+		tmp = *c;
+		tmp->parent = new_res;
+		c = &tmp->sibling;
+	}
+
+	resource_clear_tree_links(res);
+	resource_clear_tree_links(next_res);
+
+	return 0;
+}
+
 /**
  * request_resource_conflict - request and reserve an I/O or memory resource
  * @root: root resource descriptor
@@ -1503,8 +1606,7 @@ static bool system_ram_resources_mergeable(struct resource *r1,
 					   struct resource *r2)
 {
 	/* We assume either r1 or r2 is IORESOURCE_SYSRAM_MERGEABLE. */
-	return r1->flags == r2->flags && r1->end + 1 == r2->start &&
-	       r1->name == r2->name && r1->desc == r2->desc &&
+	return resource_mergeable(r1, r2) &&
 	       !r1->child && !r2->child;
 }
 
@@ -1860,6 +1962,35 @@ void resource_list_free(struct list_head *head)
 }
 EXPORT_SYMBOL(resource_list_free);
 
+struct resource_entry *resource_list_merge_entries(struct resource_entry *entry,
+						   struct resource_entry *next)
+{
+	struct resource *res = entry->res, *next_res = next->res, *new_res;
+	int ret;
+
+	if ((entry->offset != next->offset) ||
+	    !resource_mergeable(res, next_res))
+		return ERR_PTR(-EINVAL);
+
+	/* Use the internal __res to not mutate the input resources. */
+	struct resource_entry __free(kfree) *new = resource_list_create_entry(NULL, 0);
+	if (!new)
+		return ERR_PTR(-ENOMEM);
+
+	new->offset = next->offset;
+	new_res = new->res;
+
+	ret = resource_coalesce(res, next_res, new_res);
+	if (ret)
+		return ERR_PTR(ret);
+
+	resource_list_add_tail(new, &entry->node);
+	resource_list_destroy_entry(entry);
+	resource_list_destroy_entry(next);
+
+	return no_free_ptr(new);
+}
+
 #ifdef CONFIG_GET_FREE_REGION
 #define GFR_DESCENDING		(1UL << 0)
 #define GFR_REQUEST_REGION	(1UL << 1)
-- 
2.39.5

[PATCH 3/3] resource, kunit: add test case for resource_coalesce()

[Ilpo Järvinen]

Add a test case for resource_coalesce() which has both parent and
children resources to adjust.

Add framework to build resource trees in a human-readable way and to
verify them which could be used to kunit test also other resource tree
manipulation functions.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 include/linux/ioport.h  |   5 ++
 kernel/resource.c       |   8 ++-
 kernel/resource_kunit.c | 121 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+), 2 deletions(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index e8b2d6aa4013..56f4d1cfde29 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -426,5 +426,10 @@ static inline void irqresource_disabled(struct resource *res, u32 irq)
 
 extern struct address_space *iomem_get_mapping(void);
 
+#if IS_ENABLED(CONFIG_KUNIT)
+int resource_coalesce(struct resource *res, struct resource *next_res,
+		      struct resource *new_res);
+#endif
+
 #endif /* __ASSEMBLY__ */
 #endif	/* _LINUX_IOPORT_H */
diff --git a/kernel/resource.c b/kernel/resource.c
index c6e1872abb78..f7e7b49dc9a4 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -10,6 +10,8 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <kunit/visibility.h>
+
 #include <linux/cleanup.h>
 #include <linux/export.h>
 #include <linux/errno.h>
@@ -299,8 +301,9 @@ static bool resource_mergeable(struct resource *r1, struct resource *r2)
 	return false;
 }
 
-static int resource_coalesce(struct resource *res, struct resource *next_res,
-			     struct resource *new_res)
+VISIBLE_IF_KUNIT int resource_coalesce(struct resource *res,
+				       struct resource *next_res,
+				       struct resource *new_res)
 {
 	struct resource *parent, *tmp;
 	struct resource **p, **c;
@@ -367,6 +370,7 @@ static int resource_coalesce(struct resource *res, struct resource *next_res,
 
 	return 0;
 }
+EXPORT_SYMBOL_IF_KUNIT(resource_coalesce);
 
 /**
  * request_resource_conflict - request and reserve an I/O or memory resource
diff --git a/kernel/resource_kunit.c b/kernel/resource_kunit.c
index b8ef75b99eb2..3b5d09bb612a 100644
--- a/kernel/resource_kunit.c
+++ b/kernel/resource_kunit.c
@@ -4,6 +4,8 @@
  */
 
 #include <kunit/test.h>
+
+#include <linux/array_size.h>
 #include <linux/ioport.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
@@ -20,6 +22,16 @@
 #define R3_END		0x789a
 #define R4_START	0x2000
 #define R4_END		0x7000
+#define  R4A_START	0x2000
+#define  R4A_END	0x3000
+#define  R4B_START	0x3001
+#define  R4B_END	0x4000
+#define R5_START	0x7001
+#define R5_END		0x8000
+#define  R5A_START	0x7001
+#define  R5A_END	0x7800
+#define  R5B_START	0x7801
+#define  R5B_END	0x7c00
 
 static struct resource r0 = { .start = R0_START, .end = R0_END };
 static struct resource r1 = { .start = R1_START, .end = R1_END };
@@ -34,6 +46,11 @@ struct result {
 	bool ret;
 };
 
+struct resource_tree_def {
+	struct resource r;
+	unsigned int parent, child, sibling;
+};
+
 static struct result results_for_union[] = {
 	{
 		.r1 = &r1, .r2 = &r0, .r.start = R0_START, .r.end = R0_END, .ret = true,
@@ -139,6 +156,107 @@ static void resource_test_intersection(struct kunit *test)
 	} while (++i < ARRAY_SIZE(results_for_intersection));
 }
 
+static struct resource *copy_resource_tree(struct kunit *test,
+					   struct resource_tree_def *def,
+					   size_t len, size_t extra)
+{
+	struct resource *tree;
+	unsigned int i;
+
+	tree = kunit_kcalloc(test, len + extra, sizeof(*tree), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_NULL(test, tree);
+
+	for (i = 0; i < len; i++) {
+		tree[i] = def[i].r;
+
+		if (def[i].parent)
+			tree[i].parent = &tree[def[i].parent];
+		if (def[i].child)
+			tree[i].child = &tree[def[i].child];
+		if (def[i].sibling)
+			tree[i].sibling = &tree[def[i].sibling];
+	}
+
+	return tree;
+}
+
+static void resource_do_test_tree(struct kunit *test, struct resource *tree,
+				  struct resource_tree_def *exp_tree,
+				  size_t len)
+{
+	unsigned int link, i;
+
+	for (i = 0; i < len; i++) {
+		KUNIT_EXPECT_EQ_MSG(test, tree[i].start, exp_tree[i].r.start,
+				    "Start elements for resource %u are not equal", i);
+		KUNIT_EXPECT_EQ_MSG(test, tree[i].end, exp_tree[i].r.end,
+				    "End elements for resource %u are not equal", i);
+
+		link = tree[i].parent ? tree[i].parent - &tree[0] : 0;
+		KUNIT_EXPECT_EQ_MSG(test, link, exp_tree[i].parent,
+				    "Parent link for resource %u is not equal", i);
+
+		link = tree[i].child ? tree[i].child - &tree[0] : 0;
+		KUNIT_EXPECT_EQ_MSG(test, link, exp_tree[i].child,
+				    "Child link for resource %u is not equal", i);
+
+		link = tree[i].sibling ? tree[i].sibling - &tree[0] : 0;
+		KUNIT_EXPECT_EQ_MSG(test, link, exp_tree[i].sibling,
+				    "Sibling link for resource %u is not equal", i);
+	}
+}
+
+struct resource_tree_def resource_test_tree[8] = {
+	/* [0] is empty intentionally. */
+	[1] = { .r.start = R0_START, .r.end = R0_END, .child = 2 },
+	[2] = { .r.start = R4_START, .r.end = R4_END, .parent = 1, .sibling = 3, .child = 4 },
+	[3] = { .r.start = R5_START, .r.end = R5_END, .parent = 1, .child = 6 },
+	[4] = { .r.start = R4A_START, .r.end = R4A_END, .parent = 2, .sibling = 5 },
+	[5] = { .r.start = R4B_START, .r.end = R4B_END, .parent = 2 },
+	[6] = { .r.start = R5A_START, .r.end = R5A_END, .parent = 3, .sibling = 7 },
+	[7] = { .r.start = R5B_START, .r.end = R5B_END, .parent = 3 },
+};
+
+struct resource_tree_def result_for_coalesce_2_and_3[9] = {
+	/* [0] is empty intentionally. */
+	[1] = { .r.start = R0_START, .r.end = R0_END, .child = 8 },
+	[2] = { .r.start = R4_START, .r.end = R4_END },
+	[3] = { .r.start = R5_START, .r.end = R5_END },
+	[4] = { .r.start = R4A_START, .r.end = R4A_END, .parent = 8, .sibling = 5 },
+	[5] = { .r.start = R4B_START, .r.end = R4B_END, .parent = 8, .sibling = 6 },
+	[6] = { .r.start = R5A_START, .r.end = R5A_END, .parent = 8, .sibling = 7 },
+	[7] = { .r.start = R5B_START, .r.end = R5B_END, .parent = 8 },
+	[8] = { .r.start = R4_START, .r.end = R5_END, .parent = 1, .child = 4 },
+};
+
+static void resource_test_tree_test_harness(struct kunit *test)
+{
+	struct resource *tree;
+
+	tree = copy_resource_tree(test, resource_test_tree,
+				  ARRAY_SIZE(resource_test_tree), 0);
+
+	/* Sanity-check test harness with identity check */
+	resource_do_test_tree(test, tree, resource_test_tree,
+			      ARRAY_SIZE(resource_test_tree));
+}
+
+static void resource_test_coalesce(struct kunit *test)
+{
+	struct resource *tree, *result;
+	int ret;
+
+	tree = copy_resource_tree(test, resource_test_tree,
+				  ARRAY_SIZE(resource_test_tree), 1);
+
+	result = &tree[ARRAY_SIZE(resource_test_tree)];
+
+	ret = resource_coalesce(&tree[2], &tree[3], result);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	resource_do_test_tree(test, tree, result_for_coalesce_2_and_3,
+			      ARRAY_SIZE(result_for_coalesce_2_and_3));
+}
+
 /*
  * The test resource tree for region_intersects() test:
  *
@@ -292,6 +410,8 @@ static void resource_test_region_intersects(struct kunit *test)
 static struct kunit_case resource_test_cases[] = {
 	KUNIT_CASE(resource_test_union),
 	KUNIT_CASE(resource_test_intersection),
+	KUNIT_CASE(resource_test_tree_test_harness),
+	KUNIT_CASE(resource_test_coalesce),
 	KUNIT_CASE(resource_test_region_intersects),
 	{}
 };
@@ -304,3 +424,4 @@ kunit_test_suite(resource_test_suite);
 
 MODULE_DESCRIPTION("I/O Port & Memory Resource manager unit tests");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
-- 
2.39.5

Re: [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place

[Andy Shevchenko]

On Fri, Oct 10, 2025 at 05:42:30PM +0300, Ilpo Järvinen wrote:
> The resource coalescing for host bridge windows in
> pci_register_host_bridge() can alter the underlying struct resource
> which is inherently dangerous as this code has no knowledge of who else
> holds a pointer the same resource.
> 
> Merge the struct resource inside a newly added
> resource_list_merge_entries() which uses the internal __res member of
> the struct resource_entry to store the merged resource, thus preserving
> the original resource structs.

...

> +static void resource_clear_tree_links(struct resource *res)
> +{
> +	res->parent = NULL;
> +	res->child = NULL;
> +	res->sibling = NULL;
> +}

Not sure if this is the best location to inject a new helper to in the code
(I mean the position in the file). But I leave it to you just to give another
look in case something more suitable can be found.

>  static int __release_resource(struct resource *old, bool release_child)

...

> +/**
> + * resource_mergeable - Test if resources are contiguous and can be merged
> + * @r1: first resource
> + * @r2: second resource
> + *
> + * Tests @r1 is followed by @r2 contiguously and share the metadata.

This needs an additional explanation about name equivalence that's not only by
pointers, but by a content.

> + * Return: %true if resources are mergeable non-destructively.
> + */
> +static bool resource_mergeable(struct resource *r1, struct resource *r2)
> +{
> +	if ((r1->flags != r2->flags) ||
> +	    (r1->desc != r2->desc) ||
> +	    (r1->parent != r2->parent) ||
> +	    (r1->end + 1 != r2->start))
> +		return false;

> +	if (r1->name == r2->name)
> +		return true;
> +
> +	if (r1->name && r2->name && !strcmp(r1->name, r2->name))
> +		return true;
> +
> +	return false;

Hmm... Can we keep the logic more straight as in returning false cases as soon
as possible?

I think of something like this:

	if (r1->name && r2->name)
		return strcmp(r1->name, r2->name) == 0;

	return r1->name == r2->name;

> +}

...

> +	new_res->start = res->start;
> +	new_res->end = next_res->end;
> +	new_res->name = res->name;
> +	new_res->flags = res->flags;
> +	new_res->desc = res->desc;

Hmm... IIRC I saw similar code lines a few times in the kernel in resource.c
and might be elsewhere. Perhaps a time for another helper?


...

> +	/* prepare for step 2), find res & next_res from child/sibling chain. */
> +	p = &parent->child;
> +	while (1) {
> +		tmp = *p;
> +		if (tmp == res)
> +			break;
> +
> +		/*  No res in child/sibling, the resource tree is corrupted! */

Extra space which is not needed.

> +		if (WARN_ON_ONCE(!tmp))
> +			return -EINVAL;
> +
> +		p = &tmp->sibling;
> +	}

...

> static bool system_ram_resources_mergeable(struct resource *r1,
>  					   struct resource *r2)
>  {
>  	/* We assume either r1 or r2 is IORESOURCE_SYSRAM_MERGEABLE. */
> -	return r1->flags == r2->flags && r1->end + 1 == r2->start &&
> -	       r1->name == r2->name && r1->desc == r2->desc &&
> +	return resource_mergeable(r1, r2) &&
>  	       !r1->child && !r2->child;

Now one line?

>  }

> +struct resource_entry *resource_list_merge_entries(struct resource_entry *entry,
> +						   struct resource_entry *next)
> +{
> +	struct resource *res = entry->res, *next_res = next->res, *new_res;
> +	int ret;
> +
> +	if ((entry->offset != next->offset) ||
> +	    !resource_mergeable(res, next_res))

One line? (It's only 82 characters long)

> +		return ERR_PTR(-EINVAL);
> +
> +	/* Use the internal __res to not mutate the input resources. */
> +	struct resource_entry __free(kfree) *new = resource_list_create_entry(NULL, 0);
> +	if (!new)
> +		return ERR_PTR(-ENOMEM);
> +
> +	new->offset = next->offset;
> +	new_res = new->res;
> +
> +	ret = resource_coalesce(res, next_res, new_res);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	resource_list_add_tail(new, &entry->node);
> +	resource_list_destroy_entry(entry);
> +	resource_list_destroy_entry(next);
> +
> +	return no_free_ptr(new);
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Geert Uytterhoeven]

Hi Ilpo,

On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
> Here's a series for Geert to test if this fixes the improper coalescing
> of resources as was experienced with the pci_add_resource() change (I
> know the breaking change was pulled before 6.18 main PR but I'd want to
> retry it later once the known issues have been addressed). The expected
> result is there'll be two adjacent host bridge resources in the
> resource tree as the different name should disallow coalescing them
> together, and therefore BAR0 has a window into which it belongs to.
>
> Generic info for the series:
>
> PCI host bridge windows were coalesced in place into one of the structs
> on the resources list. The host bridge window coalescing code does not
> know who holds references and still needs the struct resource it's
> coalescing from/to so it is safer to perform coalescing into entirely
> a new struct resource instead and leave the old resource addresses as
> they were.
>
> The checks when coalescing is allowed are also made stricter so that
> only resources that have identical the metadata can be coalesced.
>
> As a bonus, there's also a bit of framework to easily create kunit
> tests for resource tree functions (beyond just resource_coalesce()).
>
> Ilpo Järvinen (3):
>   PCI: Refactor host bridge window coalescing loop to use prev
>   PCI: Do not coalesce host bridge resource structs in place
>   resource, kunit: add test case for resource_coalesce()

Thanks for your series!

I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
Mark resources IORESOURCE_UNSET when outside bridge windows"), and
gave it a a try on Koelsch (R-Car M2-W).

Impact on dmesg (for the first PCI/USB) instance:

     pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
     pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff
-> 0x00ee080000
     pci-rcar-gen2 ee090000.pci: PCI: revision 11
     pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
     pci_bus 0000:00: root bus resource [bus 00]
     pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
     pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional
PCI endpoint
    -pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]: no initial
claim (no window)
     pci 0000:00:00.0: BAR 0 [mem size 0x00000400]
    -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no
initial claim (no window)
     pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
     pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310 conventional
PCI endpoint
    -pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]: no initial
claim (no window)
     pci 0000:00:01.0: BAR 0 [mem size 0x00001000]
     pci 0000:00:01.0: supports D1 D2
     pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
     pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320 conventional
PCI endpoint
    -pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]: no initial
claim (no window)
     pci 0000:00:02.0: BAR 0 [mem size 0x00000100]
     pci 0000:00:02.0: supports D1 D2
     pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
     PCI: bus0: Fast back to back transfers disabled
     pci 0000:00:01.0: BAR 0 [mem 0xee080000-0xee080fff]: assigned
     pci 0000:00:02.0: BAR 0 [mem 0xee081000-0xee0810ff]: assigned
     pci_bus 0000:00: resource 4 [mem 0xee080000-0xee08ffff]
     pci 0000:00:01.0: enabling device (0140 -> 0142)
     pci 0000:00:02.0: enabling device (0140 -> 0142)

I.e. the "no initial claim (no window)" messages introduced by commit
06b77d5647a4d6a7 are no longer seen.
The BARs still show "mem size <n>" instead of the "mem <start>-<end>"
before commit 06b77d5647a4d6a7, though.

This series has not impact on /proc/iomem, or on the output of
"lspci -v" (commit 06b77d5647a4d6a7 also had no impact here).
I.e. the part of /proc/iomem related to the first PCI/USB instance
still looks like:

    ee080000-ee08ffff : pci@ee090000
      ee080000-ee080fff : 0000:00:01.0
        ee080000-ee080fff : ohci_hcd
      ee081000-ee0810ff : 0000:00:02.0
        ee081000-ee0810ff : ehci_hcd
    ee090000-ee090bff : ee090000.pci pci@ee090000

I hope this matches your expectation.s

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 5610 bytes --]

On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> > Here's a series for Geert to test if this fixes the improper coalescing
> > of resources as was experienced with the pci_add_resource() change (I
> > know the breaking change was pulled before 6.18 main PR but I'd want to
> > retry it later once the known issues have been addressed). The expected
> > result is there'll be two adjacent host bridge resources in the
> > resource tree as the different name should disallow coalescing them
> > together, and therefore BAR0 has a window into which it belongs to.
> >
> > Generic info for the series:
> >
> > PCI host bridge windows were coalesced in place into one of the structs
> > on the resources list. The host bridge window coalescing code does not
> > know who holds references and still needs the struct resource it's
> > coalescing from/to so it is safer to perform coalescing into entirely
> > a new struct resource instead and leave the old resource addresses as
> > they were.
> >
> > The checks when coalescing is allowed are also made stricter so that
> > only resources that have identical the metadata can be coalesced.
> >
> > As a bonus, there's also a bit of framework to easily create kunit
> > tests for resource tree functions (beyond just resource_coalesce()).
> >
> > Ilpo Järvinen (3):
> >   PCI: Refactor host bridge window coalescing loop to use prev
> >   PCI: Do not coalesce host bridge resource structs in place
> >   resource, kunit: add test case for resource_coalesce()
> 
> Thanks for your series!
> 
> I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> gave it a a try on Koelsch (R-Car M2-W).

So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
No coalescing would be attempted without that change.

With the pci_bus_add_resource() patch, the resource name is different, I 
think, so even then it should abort coalescing the ranges with this 
series.

> Impact on dmesg (for the first PCI/USB) instance:
> 
>      pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
>      pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff
> -> 0x00ee080000
>      pci-rcar-gen2 ee090000.pci: PCI: revision 11
>      pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
>      pci_bus 0000:00: root bus resource [bus 00]
>      pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
>      pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional
> PCI endpoint
>     -pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]: no initial
> claim (no window)
>      pci 0000:00:00.0: BAR 0 [mem size 0x00000400]
>     -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no
> initial claim (no window)
>      pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
>      pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310 conventional
> PCI endpoint
>     -pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]: no initial
> claim (no window)
>      pci 0000:00:01.0: BAR 0 [mem size 0x00001000]
>      pci 0000:00:01.0: supports D1 D2
>      pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
>      pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320 conventional
> PCI endpoint
>     -pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]: no initial
> claim (no window)
>      pci 0000:00:02.0: BAR 0 [mem size 0x00000100]
>      pci 0000:00:02.0: supports D1 D2
>      pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
>      PCI: bus0: Fast back to back transfers disabled
>      pci 0000:00:01.0: BAR 0 [mem 0xee080000-0xee080fff]: assigned
>      pci 0000:00:02.0: BAR 0 [mem 0xee081000-0xee0810ff]: assigned
>      pci_bus 0000:00: resource 4 [mem 0xee080000-0xee08ffff]
>      pci 0000:00:01.0: enabling device (0140 -> 0142)
>      pci 0000:00:02.0: enabling device (0140 -> 0142)
> 
> I.e. the "no initial claim (no window)" messages introduced by commit
> 06b77d5647a4d6a7 are no longer seen.

Is that perhaps again because of pci_dbg() vs pci_info()?

> The BARs still show "mem size <n>" instead of the "mem <start>-<end>"
> before commit 06b77d5647a4d6a7, though.

To me this looks like UNSET was still set for these resources. Missing the 
pci_bus_add_resource() patch would explain that and if the pci_dbg() 
wasn't printed, it would explain both symptoms.

Once these questions are resolved, I'll ask Rob what is the preferred 
solution here, a) driver doing pci_bus_add_resource() or b) including it 
into the DT ranges (if we could fix the ranges).

We likely need to go with a) to preserve backwards compatibility but I 
also want to understand how those ranges are supposed to be used if we 
wouldn't have historical baggage.


(I appreciate following through even if the original series is now 
reverted!)

> This series has not impact on /proc/iomem, or on the output of
> "lspci -v" (commit 06b77d5647a4d6a7 also had no impact here).
> I.e. the part of /proc/iomem related to the first PCI/USB instance
> still looks like:
> 
>     ee080000-ee08ffff : pci@ee090000
>       ee080000-ee080fff : 0000:00:01.0
>         ee080000-ee080fff : ohci_hcd
>       ee081000-ee0810ff : 0000:00:02.0
>         ee081000-ee0810ff : ehci_hcd
>     ee090000-ee090bff : ee090000.pci pci@ee090000
>
> I hope this matches your expectation.s
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> 

-- 
 i.

Re: [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 6772 bytes --]

On Wed, 15 Oct 2025, Andy Shevchenko wrote:

> On Fri, Oct 10, 2025 at 05:42:30PM +0300, Ilpo Järvinen wrote:
> > The resource coalescing for host bridge windows in
> > pci_register_host_bridge() can alter the underlying struct resource
> > which is inherently dangerous as this code has no knowledge of who else
> > holds a pointer the same resource.
> > 
> > Merge the struct resource inside a newly added
> > resource_list_merge_entries() which uses the internal __res member of
> > the struct resource_entry to store the merged resource, thus preserving
> > the original resource structs.
> 
> ...
> 
> > +static void resource_clear_tree_links(struct resource *res)
> > +{
> > +	res->parent = NULL;
> > +	res->child = NULL;
> > +	res->sibling = NULL;
> > +}
> 
> Not sure if this is the best location to inject a new helper to in the code
> (I mean the position in the file). But I leave it to you just to give another
> look in case something more suitable can be found.

This placement was far from random. I'd also want to start clearing links 
of any childs  resources (direct or grand) on a release of a resource 
(when called with  __release_resource(..., release_child=true). It's what 
lead to placing this helper here right above __release_resource().

Currently, released child resources have no way of knowing they've been 
removed from the resource tree as the resource tree links are all left in 
place (only old->parent is set to NULL by __release_resource(), strictly 
speaking even that wouldn't be necessary if we don't care for stale 
links).

My goal is to make res->parent invariant that unambiguously tells whether 
the resource is within the resource tree or not (sans the root "anchor" 
resources that are parentless).

(But as you could see, it's not part of this series.)

I initially tried to also change old->parent = NULL in 
__release_resource() to use this new helper but then realized there can be 
children too which will have stale links so to make all resource links 
coherent, a bigger change would have been needed so I left it to a later 
patch as this series was to fix PCI host bridge resource coalescing 
algorithm.

Clearing stale links from the children will come with potential 
performance penalty as the entire subtree have to be walked so it might 
result in discussion and perhaps some even opposing the idea. But I'd 
assume it to be small and likely not measurable in practice, and how 
much resource are removed from the resource tree anyway, not much I 
think except perhaps in some hotplug stress test.

I've not yet investigated how often there are unreleased children still 
remaining in first place when calling __release_resource(). It could be 
that the calling code has released those before calling release of the 
resource itself (making the performance impact nil in practice).

> >  static int __release_resource(struct resource *old, bool release_child)
> 
> ...
> 
> > +/**
> > + * resource_mergeable - Test if resources are contiguous and can be merged
> > + * @r1: first resource
> > + * @r2: second resource
> > + *
> > + * Tests @r1 is followed by @r2 contiguously and share the metadata.
> 
> This needs an additional explanation about name equivalence that's not only by
> pointers, but by a content.

Okay. The point was to check names are the same, the pointer check was 
just an optimization as these resources are expected to carry the same 
name even on the pointer level.

> > + * Return: %true if resources are mergeable non-destructively.
> > + */
> > +static bool resource_mergeable(struct resource *r1, struct resource *r2)
> > +{
> > +	if ((r1->flags != r2->flags) ||
> > +	    (r1->desc != r2->desc) ||
> > +	    (r1->parent != r2->parent) ||
> > +	    (r1->end + 1 != r2->start))
> > +		return false;
> 
> > +	if (r1->name == r2->name)
> > +		return true;
> > +
> > +	if (r1->name && r2->name && !strcmp(r1->name, r2->name))
> > +		return true;
> > +
> > +	return false;
> 
> Hmm... Can we keep the logic more straight as in returning false cases as soon
> as possible?
> 
> I think of something like this:
> 
> 	if (r1->name && r2->name)
> 		return strcmp(r1->name, r2->name) == 0;
> 
> 	return r1->name == r2->name;

But the point the order above was to avoid strcmp() when the pointer 
itself is same which I think is quite common case. I don't think strcmp() 
itself checks whether the pointer is the same.

Thanks for the review.

-- 
 i.

> > +}
> 
> ...
> 
> > +	new_res->start = res->start;
> > +	new_res->end = next_res->end;
> > +	new_res->name = res->name;
> > +	new_res->flags = res->flags;
> > +	new_res->desc = res->desc;
> 
> Hmm... IIRC I saw similar code lines a few times in the kernel in resource.c
> and might be elsewhere. Perhaps a time for another helper?
> 
> 
> ...
> 
> > +	/* prepare for step 2), find res & next_res from child/sibling chain. */
> > +	p = &parent->child;
> > +	while (1) {
> > +		tmp = *p;
> > +		if (tmp == res)
> > +			break;
> > +
> > +		/*  No res in child/sibling, the resource tree is corrupted! */
> 
> Extra space which is not needed.
> 
> > +		if (WARN_ON_ONCE(!tmp))
> > +			return -EINVAL;
> > +
> > +		p = &tmp->sibling;
> > +	}
> 
> ...
> 
> > static bool system_ram_resources_mergeable(struct resource *r1,
> >  					   struct resource *r2)
> >  {
> >  	/* We assume either r1 or r2 is IORESOURCE_SYSRAM_MERGEABLE. */
> > -	return r1->flags == r2->flags && r1->end + 1 == r2->start &&
> > -	       r1->name == r2->name && r1->desc == r2->desc &&
> > +	return resource_mergeable(r1, r2) &&
> >  	       !r1->child && !r2->child;
> 
> Now one line?
> 
> >  }
> 
> > +struct resource_entry *resource_list_merge_entries(struct resource_entry *entry,
> > +						   struct resource_entry *next)
> > +{
> > +	struct resource *res = entry->res, *next_res = next->res, *new_res;
> > +	int ret;
> > +
> > +	if ((entry->offset != next->offset) ||
> > +	    !resource_mergeable(res, next_res))
> 
> One line? (It's only 82 characters long)
> 
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	/* Use the internal __res to not mutate the input resources. */
> > +	struct resource_entry __free(kfree) *new = resource_list_create_entry(NULL, 0);
> > +	if (!new)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	new->offset = next->offset;
> > +	new_res = new->res;
> > +
> > +	ret = resource_coalesce(res, next_res, new_res);
> > +	if (ret)
> > +		return ERR_PTR(ret);
> > +
> > +	resource_list_add_tail(new, &entry->node);
> > +	resource_list_destroy_entry(entry);
> > +	resource_list_destroy_entry(next);
> > +
> > +	return no_free_ptr(new);
> > +}
> 
> 

Re: [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place

[Andy Shevchenko]

On Mon, Oct 20, 2025 at 08:21:50PM +0300, Ilpo Järvinen wrote:
> On Wed, 15 Oct 2025, Andy Shevchenko wrote:
> > On Fri, Oct 10, 2025 at 05:42:30PM +0300, Ilpo Järvinen wrote:

...

> > > +/**
> > > + * resource_mergeable - Test if resources are contiguous and can be merged
> > > + * @r1: first resource
> > > + * @r2: second resource
> > > + *
> > > + * Tests @r1 is followed by @r2 contiguously and share the metadata.
> > 
> > This needs an additional explanation about name equivalence that's not only by
> > pointers, but by a content.
> 
> Okay. The point was to check names are the same, the pointer check was 
> just an optimization as these resources are expected to carry the same 
> name even on the pointer level.
> 
> > > + * Return: %true if resources are mergeable non-destructively.
> > > + */
> > > +static bool resource_mergeable(struct resource *r1, struct resource *r2)
> > > +{
> > > +	if ((r1->flags != r2->flags) ||
> > > +	    (r1->desc != r2->desc) ||
> > > +	    (r1->parent != r2->parent) ||
> > > +	    (r1->end + 1 != r2->start))
> > > +		return false;
> > 
> > > +	if (r1->name == r2->name)
> > > +		return true;
> > > +
> > > +	if (r1->name && r2->name && !strcmp(r1->name, r2->name))
> > > +		return true;
> > > +
> > > +	return false;
> > 
> > Hmm... Can we keep the logic more straight as in returning false cases as soon
> > as possible?
> > 
> > I think of something like this:
> > 
> > 	if (r1->name && r2->name)
> > 		return strcmp(r1->name, r2->name) == 0;
> > 
> > 	return r1->name == r2->name;
> 
> But the point the order above was to avoid strcmp() when the pointer 
> itself is same which I think is quite common case. I don't think strcmp() 
> itself checks whether the pointer is the same.

On the second thought I think comparing by the content is quite a behavioural
change here. Perhaps we may start without doing that first? Theoretically it
might be the case when the content of names is different, but resources are
the same. The case when name is the same (by content, but pointers) with the
idea of having different resources sounds to me quite an awkward case. TL;
DR: What are the cases that we have in practice now?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4059 bytes --]

On Mon, 20 Oct 2025, Andy Shevchenko wrote:

> On Mon, Oct 20, 2025 at 08:21:50PM +0300, Ilpo Järvinen wrote:
> > On Wed, 15 Oct 2025, Andy Shevchenko wrote:
> > > On Fri, Oct 10, 2025 at 05:42:30PM +0300, Ilpo Järvinen wrote:
> 
> ...
> 
> > > > +/**
> > > > + * resource_mergeable - Test if resources are contiguous and can be merged
> > > > + * @r1: first resource
> > > > + * @r2: second resource
> > > > + *
> > > > + * Tests @r1 is followed by @r2 contiguously and share the metadata.
> > > 
> > > This needs an additional explanation about name equivalence that's not only by
> > > pointers, but by a content.
> > 
> > Okay. The point was to check names are the same, the pointer check was 
> > just an optimization as these resources are expected to carry the same 
> > name even on the pointer level.
> > 
> > > > + * Return: %true if resources are mergeable non-destructively.
> > > > + */
> > > > +static bool resource_mergeable(struct resource *r1, struct resource *r2)
> > > > +{
> > > > +	if ((r1->flags != r2->flags) ||
> > > > +	    (r1->desc != r2->desc) ||
> > > > +	    (r1->parent != r2->parent) ||
> > > > +	    (r1->end + 1 != r2->start))
> > > > +		return false;
> > > 
> > > > +	if (r1->name == r2->name)
> > > > +		return true;
> > > > +
> > > > +	if (r1->name && r2->name && !strcmp(r1->name, r2->name))
> > > > +		return true;
> > > > +
> > > > +	return false;
> > > 
> > > Hmm... Can we keep the logic more straight as in returning false cases as soon
> > > as possible?
> > > 
> > > I think of something like this:
> > > 
> > > 	if (r1->name && r2->name)
> > > 		return strcmp(r1->name, r2->name) == 0;
> > > 
> > > 	return r1->name == r2->name;
> > 
> > But the point the order above was to avoid strcmp() when the pointer 
> > itself is same which I think is quite common case. I don't think strcmp() 
> > itself checks whether the pointer is the same.
> 
> On the second thought I think comparing by the content is quite a behavioural
> change here.

Compared to what?

This code was previously only used for merging contiguous "System RAM" 
resources (AFAICT, I don't have way to check what the names in all those
resources truly were but in any case, the check was even stricter earlier, 
comparing pointer equality only so definitely the names were not different 
before this).

> Perhaps we may start without doing that first? Theoretically it
> might be the case when the content of names is different, but resources are
> the same.

Resources are NOT same, they're two contiguous memory regions and may 
originate from different source, and thus have different names.

Not caring about the names will lose one of them from /proc/iomem.

> The case when name is the same (by content, but pointers) with the
> idea of having different resources sounds to me quite an awkward case. TL;
> DR: What are the cases that we have in practice now?

In the original thread [1], PCI side resource coalescing did break the 
resources by merging without caring what the resource internals were. That 
problem was found after trying to fix another problem, thus it might not 
happen in practice except after fixing the other problem with root bus 
resources.

In the common case when merging PCI root bus resources, the resources 
typically have the same name - this happens all the time (e.g. io port 
ranges are split to many small ranges which form a contiguous region 
when coalesced). But that's not always the case, why do you think these 
two names should be merged losing some information:

     ee080000-ee08ffff : pci@ee090000
       ...
     ee090000-ee090bff : ee090000.pci pci@ee090000

?

(Also, the careless change in the underlying resource by the code this 
series tries to fix would have likely broken also devres release of the 
mangled resource, which admittedly, is not related to name at all).

[1] https://lore.kernel.org/linux-pci/CAMuHMdVgCHU80mRm1Vwo6GFgNAtQcf50yHBz_oAk4TrtjcMpYg@mail.gmail.com/


-- 
 i.

Re: [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place

[Andy Shevchenko]

On Mon, Oct 20, 2025 at 09:15:08PM +0300, Ilpo Järvinen wrote:
> On Mon, 20 Oct 2025, Andy Shevchenko wrote:
> > On Mon, Oct 20, 2025 at 08:21:50PM +0300, Ilpo Järvinen wrote:
> > > On Wed, 15 Oct 2025, Andy Shevchenko wrote:
> > > > On Fri, Oct 10, 2025 at 05:42:30PM +0300, Ilpo Järvinen wrote:

...

> > > > > +/**
> > > > > + * resource_mergeable - Test if resources are contiguous and can be merged
> > > > > + * @r1: first resource
> > > > > + * @r2: second resource
> > > > > + *
> > > > > + * Tests @r1 is followed by @r2 contiguously and share the metadata.
> > > > 
> > > > This needs an additional explanation about name equivalence that's not only by
> > > > pointers, but by a content.
> > > 
> > > Okay. The point was to check names are the same, the pointer check was 
> > > just an optimization as these resources are expected to carry the same 
> > > name even on the pointer level.
> > > 
> > > > > + * Return: %true if resources are mergeable non-destructively.
> > > > > + */
> > > > > +static bool resource_mergeable(struct resource *r1, struct resource *r2)
> > > > > +{
> > > > > +	if ((r1->flags != r2->flags) ||
> > > > > +	    (r1->desc != r2->desc) ||
> > > > > +	    (r1->parent != r2->parent) ||
> > > > > +	    (r1->end + 1 != r2->start))
> > > > > +		return false;
> > > > 
> > > > > +	if (r1->name == r2->name)
> > > > > +		return true;
> > > > > +
> > > > > +	if (r1->name && r2->name && !strcmp(r1->name, r2->name))
> > > > > +		return true;
> > > > > +
> > > > > +	return false;
> > > > 
> > > > Hmm... Can we keep the logic more straight as in returning false cases as soon
> > > > as possible?
> > > > 
> > > > I think of something like this:
> > > > 
> > > > 	if (r1->name && r2->name)
> > > > 		return strcmp(r1->name, r2->name) == 0;
> > > > 
> > > > 	return r1->name == r2->name;
> > > 
> > > But the point the order above was to avoid strcmp() when the pointer 
> > > itself is same which I think is quite common case. I don't think strcmp() 
> > > itself checks whether the pointer is the same.
> > 
> > On the second thought I think comparing by the content is quite a behavioural
> > change here.
> 
> Compared to what?
> 
> This code was previously only used for merging contiguous "System RAM" 
> resources (AFAICT, I don't have way to check what the names in all those
> resources truly were but in any case, the check was even stricter earlier, 
> comparing pointer equality only so definitely the names were not different 
> before this).
> 
> > Perhaps we may start without doing that first? Theoretically it
> > might be the case when the content of names is different, but resources are
> > the same.
> 
> Resources are NOT same, they're two contiguous memory regions and may 
> originate from different source, and thus have different names.
> 
> Not caring about the names will lose one of them from /proc/iomem.
> 
> > The case when name is the same (by content, but pointers) with the
> > idea of having different resources sounds to me quite an awkward case. TL;
> > DR: What are the cases that we have in practice now?
> 
> In the original thread [1], PCI side resource coalescing did break the 
> resources by merging without caring what the resource internals were. That 
> problem was found after trying to fix another problem, thus it might not 
> happen in practice except after fixing the other problem with root bus 
> resources.
> 
> In the common case when merging PCI root bus resources, the resources 
> typically have the same name - this happens all the time (e.g. io port 
> ranges are split to many small ranges which form a contiguous region 
> when coalesced). But that's not always the case, why do you think these 
> two names should be merged losing some information:
> 
>      ee080000-ee08ffff : pci@ee090000
>        ...
>      ee090000-ee090bff : ee090000.pci pci@ee090000
> 
> ?

I don't think it's a good idea (after reading the nice elaboration from you).
It seems I misunderstood the use case(s). That's why I asked for some elaboration
about the (new?) requirement to test the content of the names and not only pointer
equivalency.

> (Also, the careless change in the underlying resource by the code this 
> series tries to fix would have likely broken also devres release of the 
> mangled resource, which admittedly, is not related to name at all).
> 
> [1] https://lore.kernel.org/linux-pci/CAMuHMdVgCHU80mRm1Vwo6GFgNAtQcf50yHBz_oAk4TrtjcMpYg@mail.gmail.com/


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Geert Uytterhoeven]

Hi Ilpo,

On Mon, 20 Oct 2025 at 18:20, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
> On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> > On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > > Here's a series for Geert to test if this fixes the improper coalescing
> > > of resources as was experienced with the pci_add_resource() change (I
> > > know the breaking change was pulled before 6.18 main PR but I'd want to
> > > retry it later once the known issues have been addressed). The expected
> > > result is there'll be two adjacent host bridge resources in the
> > > resource tree as the different name should disallow coalescing them
> > > together, and therefore BAR0 has a window into which it belongs to.
> > >
> > > Generic info for the series:
> > >
> > > PCI host bridge windows were coalesced in place into one of the structs
> > > on the resources list. The host bridge window coalescing code does not
> > > know who holds references and still needs the struct resource it's
> > > coalescing from/to so it is safer to perform coalescing into entirely
> > > a new struct resource instead and leave the old resource addresses as
> > > they were.
> > >
> > > The checks when coalescing is allowed are also made stricter so that
> > > only resources that have identical the metadata can be coalesced.
> > >
> > > As a bonus, there's also a bit of framework to easily create kunit
> > > tests for resource tree functions (beyond just resource_coalesce()).
> > >
> > > Ilpo Järvinen (3):
> > >   PCI: Refactor host bridge window coalescing loop to use prev
> > >   PCI: Do not coalesce host bridge resource structs in place
> > >   resource, kunit: add test case for resource_coalesce()
> >
> > Thanks for your series!
> >
> > I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> > Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> > gave it a a try on Koelsch (R-Car M2-W).
>
> So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
> No coalescing would be attempted without that change.

Sorry, I didn't realize you wanted that (and anything else) to be
included, too.  Please tell me the exact base I should use for testing,
and I will give it another run.
Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3959 bytes --]

On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:
> On Mon, 20 Oct 2025 at 18:20, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> > On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> > > On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > Here's a series for Geert to test if this fixes the improper coalescing
> > > > of resources as was experienced with the pci_add_resource() change (I
> > > > know the breaking change was pulled before 6.18 main PR but I'd want to
> > > > retry it later once the known issues have been addressed). The expected
> > > > result is there'll be two adjacent host bridge resources in the
> > > > resource tree as the different name should disallow coalescing them
> > > > together, and therefore BAR0 has a window into which it belongs to.
> > > >
> > > > Generic info for the series:
> > > >
> > > > PCI host bridge windows were coalesced in place into one of the structs
> > > > on the resources list. The host bridge window coalescing code does not
> > > > know who holds references and still needs the struct resource it's
> > > > coalescing from/to so it is safer to perform coalescing into entirely
> > > > a new struct resource instead and leave the old resource addresses as
> > > > they were.
> > > >
> > > > The checks when coalescing is allowed are also made stricter so that
> > > > only resources that have identical the metadata can be coalesced.
> > > >
> > > > As a bonus, there's also a bit of framework to easily create kunit
> > > > tests for resource tree functions (beyond just resource_coalesce()).
> > > >
> > > > Ilpo Järvinen (3):
> > > >   PCI: Refactor host bridge window coalescing loop to use prev
> > > >   PCI: Do not coalesce host bridge resource structs in place
> > > >   resource, kunit: add test case for resource_coalesce()
> > >
> > > Thanks for your series!
> > >
> > > I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> > > Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> > > gave it a a try on Koelsch (R-Car M2-W).
> >
> > So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
> > No coalescing would be attempted without that change.
> 
> Sorry, I didn't realize you wanted that (and anything else) to be
> included, too.  Please tell me the exact base I should use for testing,
> and I will give it another run.

I'm sorry, it's indeed a bit confusing as some of these patches never 
have been in Linus' tree.

So I'm interested on what's the result with these changes/series together:

[PATCH 1/2] PCI: Setup bridge resources earlier 
[PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET 
[PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
[PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev 
[PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
[PATCH 3/3] resource, kunit: add test case for resource_coalesce()

You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch 
to pci_info() (as otherwise dyndbg is necessary to make it visible).

Lore links to these series/patches:

https://lore.kernel.org/linux-pci/20250924134228.1663-1-ilpo.jarvinen@linux.intel.com/
https://lore.kernel.org/linux-pci/7640a03e-dfea-db9c-80f5-d80fa2c505b7@linux.intel.com/
https://lore.kernel.org/linux-pci/20251010144231.15773-1-ilpo.jarvinen@linux.intel.com/

The expected result is that those usb resources are properly parented and 
the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as 
that would destroy information). So something along the lines of:

    ee080000-ee08ffff : pci@ee090000
      ee080000-ee080fff : 0000:00:01.0
        ee080000-ee080fff : ohci_hcd
      ee081000-ee0810ff : 0000:00:02.0
        ee081000-ee0810ff : ehci_hcd
    ee090000-ee090bff : ee090000.pci pci@ee090000

-- 
 i.

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Andy Shevchenko]

On Tue, Oct 21, 2025 at 02:54:03PM +0300, Ilpo Järvinen wrote:
> On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:

...

> I'm sorry, it's indeed a bit confusing as some of these patches never 
> have been in Linus' tree.
> 
> So I'm interested on what's the result with these changes/series together:
> 
> [PATCH 1/2] PCI: Setup bridge resources earlier 
> [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET 
> [PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
> [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev 
> [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
> [PATCH 3/3] resource, kunit: add test case for resource_coalesce()
> 
> You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch 
> to pci_info() (as otherwise dyndbg is necessary to make it visible).
> 
> Lore links to these series/patches:
> 
> https://lore.kernel.org/linux-pci/20250924134228.1663-1-ilpo.jarvinen@linux.intel.com/
> https://lore.kernel.org/linux-pci/7640a03e-dfea-db9c-80f5-d80fa2c505b7@linux.intel.com/
> https://lore.kernel.org/linux-pci/20251010144231.15773-1-ilpo.jarvinen@linux.intel.com/
> 
> The expected result is that those usb resources are properly parented and 
> the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as 
> that would destroy information). So something along the lines of:
> 
>     ee080000-ee08ffff : pci@ee090000

For my pedantic eye, the naming is a bit confusing here. Is this a mistake in
the code or in the example?

>       ee080000-ee080fff : 0000:00:01.0
>         ee080000-ee080fff : ohci_hcd
>       ee081000-ee0810ff : 0000:00:02.0
>         ee081000-ee0810ff : ehci_hcd
>     ee090000-ee090bff : ee090000.pci pci@ee090000


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2303 bytes --]

On Tue, 21 Oct 2025, Andy Shevchenko wrote:

> On Tue, Oct 21, 2025 at 02:54:03PM +0300, Ilpo Järvinen wrote:
> > On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:
> 
> ...
> 
> > I'm sorry, it's indeed a bit confusing as some of these patches never 
> > have been in Linus' tree.
> > 
> > So I'm interested on what's the result with these changes/series together:
> > 
> > [PATCH 1/2] PCI: Setup bridge resources earlier 
> > [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET 
> > [PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
> > [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev 
> > [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
> > [PATCH 3/3] resource, kunit: add test case for resource_coalesce()
> > 
> > You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch 
> > to pci_info() (as otherwise dyndbg is necessary to make it visible).
> > 
> > Lore links to these series/patches:
> > 
> > https://lore.kernel.org/linux-pci/20250924134228.1663-1-ilpo.jarvinen@linux.intel.com/
> > https://lore.kernel.org/linux-pci/7640a03e-dfea-db9c-80f5-d80fa2c505b7@linux.intel.com/
> > https://lore.kernel.org/linux-pci/20251010144231.15773-1-ilpo.jarvinen@linux.intel.com/
> > 
> > The expected result is that those usb resources are properly parented and 
> > the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as 
> > that would destroy information). So something along the lines of:
> > 
> >     ee080000-ee08ffff : pci@ee090000
> 
> For my pedantic eye, the naming is a bit confusing here. Is this a mistake in
> the code or in the example?
> 
> >       ee080000-ee080fff : 0000:00:01.0
> >         ee080000-ee080fff : ohci_hcd
> >       ee081000-ee0810ff : 0000:00:02.0
> >         ee081000-ee0810ff : ehci_hcd
> >     ee090000-ee090bff : ee090000.pci pci@ee090000

I tried to copy them from here:

https://lore.kernel.org/linux-pci/CAMuHMdUbaQDXsowZETimLJ-=gLCofeP+LnJp_txetuBQ0hmcPQ@mail.gmail.com/

So my answer is, from code.

I'm not trying to change the names here, they are what they are.

Why things work that way in DT platform (ee08 vs @ee09), don't ask me as I 
unfortunately don't know the answers.

-- 
 i.

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Geert Uytterhoeven]

Hi Andy,

On Tue, 21 Oct 2025 at 17:49, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Tue, Oct 21, 2025 at 02:54:03PM +0300, Ilpo Järvinen wrote:
> > The expected result is that those usb resources are properly parented and
> > the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as
> > that would destroy information). So something along the lines of:
> >
> >     ee080000-ee08ffff : pci@ee090000
>
> For my pedantic eye, the naming is a bit confusing here. Is this a mistake in
> the code or in the example?
>
> >       ee080000-ee080fff : 0000:00:01.0
> >         ee080000-ee080fff : ohci_hcd
> >       ee081000-ee0810ff : 0000:00:02.0
> >         ee081000-ee0810ff : ehci_hcd
> >     ee090000-ee090bff : ee090000.pci pci@ee090000

A platform device instantiated from DT is named after the node name
and unit address of the corresponding DT node.  If the device has
multiple register banks, all its register banks are still claimed by
the same device, so all but the first (in DT) register bank show a
non-matching address in the device name.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Geert Uytterhoeven]

Hi Ilpo,

On Tue, 21 Oct 2025 at 13:54, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
> On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:
> > On Mon, 20 Oct 2025 at 18:20, Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > > On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> > > > On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> > > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > > Here's a series for Geert to test if this fixes the improper coalescing
> > > > > of resources as was experienced with the pci_add_resource() change (I
> > > > > know the breaking change was pulled before 6.18 main PR but I'd want to
> > > > > retry it later once the known issues have been addressed). The expected
> > > > > result is there'll be two adjacent host bridge resources in the
> > > > > resource tree as the different name should disallow coalescing them
> > > > > together, and therefore BAR0 has a window into which it belongs to.
> > > > >
> > > > > Generic info for the series:
> > > > >
> > > > > PCI host bridge windows were coalesced in place into one of the structs
> > > > > on the resources list. The host bridge window coalescing code does not
> > > > > know who holds references and still needs the struct resource it's
> > > > > coalescing from/to so it is safer to perform coalescing into entirely
> > > > > a new struct resource instead and leave the old resource addresses as
> > > > > they were.
> > > > >
> > > > > The checks when coalescing is allowed are also made stricter so that
> > > > > only resources that have identical the metadata can be coalesced.
> > > > >
> > > > > As a bonus, there's also a bit of framework to easily create kunit
> > > > > tests for resource tree functions (beyond just resource_coalesce()).
> > > > >
> > > > > Ilpo Järvinen (3):
> > > > >   PCI: Refactor host bridge window coalescing loop to use prev
> > > > >   PCI: Do not coalesce host bridge resource structs in place
> > > > >   resource, kunit: add test case for resource_coalesce()
> > > >
> > > > Thanks for your series!
> > > >
> > > > I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> > > > Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> > > > gave it a a try on Koelsch (R-Car M2-W).
> > >
> > > So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
> > > No coalescing would be attempted without that change.
> >
> > Sorry, I didn't realize you wanted that (and anything else) to be
> > included, too.  Please tell me the exact base I should use for testing,
> > and I will give it another run.
>
> I'm sorry, it's indeed a bit confusing as some of these patches never
> have been in Linus' tree.
>
> So I'm interested on what's the result with these changes/series together:
>
> [PATCH 1/2] PCI: Setup bridge resources earlier
> [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET
> [PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
> [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev
> [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
> [PATCH 3/3] resource, kunit: add test case for resource_coalesce()
>
> You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch
> to pci_info() (as otherwise dyndbg is necessary to make it visible).

Thanks, all done:

    $ git cherry -v --abbrev=1 v6.18-rc2^
    + 211ddde0 Linux 6.18-rc2
    + 3fdaf2 PCI: Setup bridge resources earlier
    + 5be02e5 PCI: Resources outside their window must set IORESOURCE_UNSET
    + adf6f11 PCI: rcar-gen2: Add BAR0 into host bridge resources
    + eecb500 PCI: Refactor host bridge window coalescing loop to use prev
    + 60470b3 PCI: Do not coalesce host bridge resource structs in place
    + afe3ec resource, kunit: add test case for resource_coalesce()
    + 487c98 Use dev_info() in drivers/pci/probe.c:__pci_read_base()
IORESOURCE_UNSET path

Compared to v6.18-rc2, dmesg changed (for the first PCI/USB instance)
like:

     pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
     pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff
-> 0x00ee080000
     pci-rcar-gen2 ee090000.pci: PCI: revision 11
     pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
     pci_bus 0000:00: root bus resource [bus 00]
     pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
    +pci_bus 0000:00: root bus resource [mem 0xee090000-0xee090bff]
     pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional
PCI endpoint
     pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
    -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
    +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no
initial claim (no window)
    +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
     pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310 conventional
PCI endpoint
    -pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]
    +pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]: no initial
claim (no window)
    +pci 0000:00:01.0: BAR 0 [mem size 0x00001000]
     pci 0000:00:01.0: supports D1 D2
     pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
     pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320 conventional
PCI endpoint
    -pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]
    +pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]: no initial
claim (no window)
    +pci 0000:00:02.0: BAR 0 [mem size 0x00000100]
     pci 0000:00:02.0: supports D1 D2
     pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
     PCI: bus0: Fast back to back transfers disabled
     pci 0000:00:01.0: BAR 0 [mem 0xee080000-0xee080fff]: assigned
     pci 0000:00:02.0: BAR 0 [mem 0xee081000-0xee0810ff]: assigned
     pci_bus 0000:00: resource 4 [mem 0xee080000-0xee08ffff]
    +pci_bus 0000:00: resource 5 [mem 0xee090000-0xee090bff]
     pci 0000:00:01.0: enabling device (0140 -> 0142)
     pci 0000:00:02.0: enabling device (0140 -> 0142)

> The expected result is that those usb resources are properly parented and
> the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as
> that would destroy information). So something along the lines of:
>
>     ee080000-ee08ffff : pci@ee090000
>       ee080000-ee080fff : 0000:00:01.0
>         ee080000-ee080fff : ohci_hcd
>       ee081000-ee0810ff : 0000:00:02.0
>         ee081000-ee0810ff : ehci_hcd
>     ee090000-ee090bff : ee090000.pci pci@ee090000

Compared to v6.18-rc2, the output of "lspci -v" or "cat /proc/iomem"
did not change.  Hence for the two PCI/USB instances:

    ee080000-ee08ffff : pci@ee090000
      ee080000-ee080fff : 0000:00:01.0
        ee080000-ee080fff : ohci_hcd
      ee081000-ee0810ff : 0000:00:02.0
        ee081000-ee0810ff : ehci_hcd
    ee090000-ee090bff : ee090000.pci pci@ee090000
    ee0c0000-ee0cffff : pci@ee0d0000
      ee0c0000-ee0c0fff : 0001:01:01.0
        ee0c0000-ee0c0fff : ohci_hcd
      ee0c1000-ee0c10ff : 0001:01:02.0
        ee0c1000-ee0c10ff : ehci_hcd
    ee0d0000-ee0d0bff : ee0d0000.pci pci@ee0d0000

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 7688 bytes --]

On Wed, 22 Oct 2025, Geert Uytterhoeven wrote:
> On Tue, 21 Oct 2025 at 13:54, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> > On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:
> > > On Mon, 20 Oct 2025 at 18:20, Ilpo Järvinen
> > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> > > > > On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> > > > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > > > Here's a series for Geert to test if this fixes the improper coalescing
> > > > > > of resources as was experienced with the pci_add_resource() change (I
> > > > > > know the breaking change was pulled before 6.18 main PR but I'd want to
> > > > > > retry it later once the known issues have been addressed). The expected
> > > > > > result is there'll be two adjacent host bridge resources in the
> > > > > > resource tree as the different name should disallow coalescing them
> > > > > > together, and therefore BAR0 has a window into which it belongs to.
> > > > > >
> > > > > > Generic info for the series:
> > > > > >
> > > > > > PCI host bridge windows were coalesced in place into one of the structs
> > > > > > on the resources list. The host bridge window coalescing code does not
> > > > > > know who holds references and still needs the struct resource it's
> > > > > > coalescing from/to so it is safer to perform coalescing into entirely
> > > > > > a new struct resource instead and leave the old resource addresses as
> > > > > > they were.
> > > > > >
> > > > > > The checks when coalescing is allowed are also made stricter so that
> > > > > > only resources that have identical the metadata can be coalesced.
> > > > > >
> > > > > > As a bonus, there's also a bit of framework to easily create kunit
> > > > > > tests for resource tree functions (beyond just resource_coalesce()).
> > > > > >
> > > > > > Ilpo Järvinen (3):
> > > > > >   PCI: Refactor host bridge window coalescing loop to use prev
> > > > > >   PCI: Do not coalesce host bridge resource structs in place
> > > > > >   resource, kunit: add test case for resource_coalesce()
> > > > >
> > > > > Thanks for your series!
> > > > >
> > > > > I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> > > > > Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> > > > > gave it a a try on Koelsch (R-Car M2-W).
> > > >
> > > > So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
> > > > No coalescing would be attempted without that change.
> > >
> > > Sorry, I didn't realize you wanted that (and anything else) to be
> > > included, too.  Please tell me the exact base I should use for testing,
> > > and I will give it another run.
> >
> > I'm sorry, it's indeed a bit confusing as some of these patches never
> > have been in Linus' tree.
> >
> > So I'm interested on what's the result with these changes/series together:
> >
> > [PATCH 1/2] PCI: Setup bridge resources earlier
> > [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET
> > [PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
> > [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev
> > [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
> > [PATCH 3/3] resource, kunit: add test case for resource_coalesce()
> >
> > You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch
> > to pci_info() (as otherwise dyndbg is necessary to make it visible).
> 
> Thanks, all done:
> 
>     $ git cherry -v --abbrev=1 v6.18-rc2^
>     + 211ddde0 Linux 6.18-rc2
>     + 3fdaf2 PCI: Setup bridge resources earlier
>     + 5be02e5 PCI: Resources outside their window must set IORESOURCE_UNSET
>     + adf6f11 PCI: rcar-gen2: Add BAR0 into host bridge resources
>     + eecb500 PCI: Refactor host bridge window coalescing loop to use prev
>     + 60470b3 PCI: Do not coalesce host bridge resource structs in place
>     + afe3ec resource, kunit: add test case for resource_coalesce()
>     + 487c98 Use dev_info() in drivers/pci/probe.c:__pci_read_base()
> IORESOURCE_UNSET path
> 
> Compared to v6.18-rc2, dmesg changed (for the first PCI/USB instance)
> like:
> 
>      pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
>      pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff
> -> 0x00ee080000
>      pci-rcar-gen2 ee090000.pci: PCI: revision 11
>      pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
>      pci_bus 0000:00: root bus resource [bus 00]
>      pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
>     +pci_bus 0000:00: root bus resource [mem 0xee090000-0xee090bff]
>      pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional
> PCI endpoint
>      pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
>     -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
>     +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no
> initial claim (no window)
>     +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
>      pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310 conventional
> PCI endpoint
>     -pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]
>     +pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]: no initial
> claim (no window)
>     +pci 0000:00:01.0: BAR 0 [mem size 0x00001000]
>      pci 0000:00:01.0: supports D1 D2
>      pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
>      pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320 conventional
> PCI endpoint
>     -pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]
>     +pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]: no initial
> claim (no window)
>     +pci 0000:00:02.0: BAR 0 [mem size 0x00000100]
>      pci 0000:00:02.0: supports D1 D2
>      pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
>      PCI: bus0: Fast back to back transfers disabled
>      pci 0000:00:01.0: BAR 0 [mem 0xee080000-0xee080fff]: assigned
>      pci 0000:00:02.0: BAR 0 [mem 0xee081000-0xee0810ff]: assigned
>      pci_bus 0000:00: resource 4 [mem 0xee080000-0xee08ffff]
>     +pci_bus 0000:00: resource 5 [mem 0xee090000-0xee090bff]
>      pci 0000:00:01.0: enabling device (0140 -> 0142)
>      pci 0000:00:02.0: enabling device (0140 -> 0142)
> 
> > The expected result is that those usb resources are properly parented and
> > the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as
> > that would destroy information). So something along the lines of:
> >
> >     ee080000-ee08ffff : pci@ee090000
> >       ee080000-ee080fff : 0000:00:01.0
> >         ee080000-ee080fff : ohci_hcd
> >       ee081000-ee0810ff : 0000:00:02.0
> >         ee081000-ee0810ff : ehci_hcd
> >     ee090000-ee090bff : ee090000.pci pci@ee090000
> 
> Compared to v6.18-rc2, the output of "lspci -v" or "cat /proc/iomem"
> did not change.  Hence for the two PCI/USB instances:
> 
>     ee080000-ee08ffff : pci@ee090000
>       ee080000-ee080fff : 0000:00:01.0
>         ee080000-ee080fff : ohci_hcd
>       ee081000-ee0810ff : 0000:00:02.0
>         ee081000-ee0810ff : ehci_hcd
>     ee090000-ee090bff : ee090000.pci pci@ee090000
>     ee0c0000-ee0cffff : pci@ee0d0000
>       ee0c0000-ee0c0fff : 0001:01:01.0
>         ee0c0000-ee0c0fff : ohci_hcd
>       ee0c1000-ee0c10ff : 0001:01:02.0
>         ee0c1000-ee0c10ff : ehci_hcd
>     ee0d0000-ee0d0bff : ee0d0000.pci pci@ee0d0000

Looks it works now when it comes to what PCI: Resources outside their 
window must set IORESOURCE_UNSET tried to achieve.

Thanks a lot for all the testing!

-- 
 i.

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 10861 bytes --]

On Wed, 22 Oct 2025, Geert Uytterhoeven wrote:
> On Tue, 21 Oct 2025 at 13:54, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> > On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:
> > > On Mon, 20 Oct 2025 at 18:20, Ilpo Järvinen
> > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> > > > > On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> > > > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > > > Here's a series for Geert to test if this fixes the improper coalescing
> > > > > > of resources as was experienced with the pci_add_resource() change (I
> > > > > > know the breaking change was pulled before 6.18 main PR but I'd want to
> > > > > > retry it later once the known issues have been addressed). The expected
> > > > > > result is there'll be two adjacent host bridge resources in the
> > > > > > resource tree as the different name should disallow coalescing them
> > > > > > together, and therefore BAR0 has a window into which it belongs to.
> > > > > >
> > > > > > Generic info for the series:
> > > > > >
> > > > > > PCI host bridge windows were coalesced in place into one of the structs
> > > > > > on the resources list. The host bridge window coalescing code does not
> > > > > > know who holds references and still needs the struct resource it's
> > > > > > coalescing from/to so it is safer to perform coalescing into entirely
> > > > > > a new struct resource instead and leave the old resource addresses as
> > > > > > they were.
> > > > > >
> > > > > > The checks when coalescing is allowed are also made stricter so that
> > > > > > only resources that have identical the metadata can be coalesced.
> > > > > >
> > > > > > As a bonus, there's also a bit of framework to easily create kunit
> > > > > > tests for resource tree functions (beyond just resource_coalesce()).
> > > > > >
> > > > > > Ilpo Järvinen (3):
> > > > > >   PCI: Refactor host bridge window coalescing loop to use prev
> > > > > >   PCI: Do not coalesce host bridge resource structs in place
> > > > > >   resource, kunit: add test case for resource_coalesce()
> > > > >
> > > > > Thanks for your series!
> > > > >
> > > > > I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> > > > > Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> > > > > gave it a a try on Koelsch (R-Car M2-W).
> > > >
> > > > So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
> > > > No coalescing would be attempted without that change.
> > >
> > > Sorry, I didn't realize you wanted that (and anything else) to be
> > > included, too.  Please tell me the exact base I should use for testing,
> > > and I will give it another run.
> >
> > I'm sorry, it's indeed a bit confusing as some of these patches never
> > have been in Linus' tree.
> >
> > So I'm interested on what's the result with these changes/series together:
> >
> > [PATCH 1/2] PCI: Setup bridge resources earlier
> > [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET
> > [PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
> > [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev
> > [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
> > [PATCH 3/3] resource, kunit: add test case for resource_coalesce()
> >
> > You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch
> > to pci_info() (as otherwise dyndbg is necessary to make it visible).
> 
> Thanks, all done:
> 
>     $ git cherry -v --abbrev=1 v6.18-rc2^
>     + 211ddde0 Linux 6.18-rc2
>     + 3fdaf2 PCI: Setup bridge resources earlier
>     + 5be02e5 PCI: Resources outside their window must set IORESOURCE_UNSET
>     + adf6f11 PCI: rcar-gen2: Add BAR0 into host bridge resources
>     + eecb500 PCI: Refactor host bridge window coalescing loop to use prev
>     + 60470b3 PCI: Do not coalesce host bridge resource structs in place
>     + afe3ec resource, kunit: add test case for resource_coalesce()
>     + 487c98 Use dev_info() in drivers/pci/probe.c:__pci_read_base()
> IORESOURCE_UNSET path
> 
> Compared to v6.18-rc2, dmesg changed (for the first PCI/USB instance)
> like:
> 
>      pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
>      pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff
> -> 0x00ee080000
>      pci-rcar-gen2 ee090000.pci: PCI: revision 11
>      pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
>      pci_bus 0000:00: root bus resource [bus 00]
>      pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
>     +pci_bus 0000:00: root bus resource [mem 0xee090000-0xee090bff]
>      pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional
> PCI endpoint
>      pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
>     -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
>     +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no
> initial claim (no window)
>     +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
>      pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310 conventional
> PCI endpoint
>     -pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]
>     +pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]: no initial
> claim (no window)
>     +pci 0000:00:01.0: BAR 0 [mem size 0x00001000]
>      pci 0000:00:01.0: supports D1 D2
>      pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
>      pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320 conventional
> PCI endpoint
>     -pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]
>     +pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]: no initial
> claim (no window)
>     +pci 0000:00:02.0: BAR 0 [mem size 0x00000100]
>      pci 0000:00:02.0: supports D1 D2
>      pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
>      PCI: bus0: Fast back to back transfers disabled
>      pci 0000:00:01.0: BAR 0 [mem 0xee080000-0xee080fff]: assigned
>      pci 0000:00:02.0: BAR 0 [mem 0xee081000-0xee0810ff]: assigned
>      pci_bus 0000:00: resource 4 [mem 0xee080000-0xee08ffff]
>     +pci_bus 0000:00: resource 5 [mem 0xee090000-0xee090bff]
>      pci 0000:00:01.0: enabling device (0140 -> 0142)
>      pci 0000:00:02.0: enabling device (0140 -> 0142)
> 
> > The expected result is that those usb resources are properly parented and
> > the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as
> > that would destroy information). So something along the lines of:
> >
> >     ee080000-ee08ffff : pci@ee090000
> >       ee080000-ee080fff : 0000:00:01.0
> >         ee080000-ee080fff : ohci_hcd
> >       ee081000-ee0810ff : 0000:00:02.0
> >         ee081000-ee0810ff : ehci_hcd
> >     ee090000-ee090bff : ee090000.pci pci@ee090000
> 
> Compared to v6.18-rc2, the output of "lspci -v" or "cat /proc/iomem"
> did not change.  Hence for the two PCI/USB instances:
> 
>     ee080000-ee08ffff : pci@ee090000
>       ee080000-ee080fff : 0000:00:01.0
>         ee080000-ee080fff : ohci_hcd
>       ee081000-ee0810ff : 0000:00:02.0
>         ee081000-ee0810ff : ehci_hcd
>     ee090000-ee090bff : ee090000.pci pci@ee090000
>     ee0c0000-ee0cffff : pci@ee0d0000
>       ee0c0000-ee0c0fff : 0001:01:01.0
>         ee0c0000-ee0c0fff : ohci_hcd
>       ee0c1000-ee0c10ff : 0001:01:02.0
>         ee0c1000-ee0c10ff : ehci_hcd
>     ee0d0000-ee0d0bff : ee0d0000.pci pci@ee0d0000

Hi Rob,

I'd want to hear your opinion on the solutions me and Geert tried and
discussed in the subthread starting from this:

https://lore.kernel.org/linux-pci/CAMuHMdVtVzcL3AX0uetNhKr-gLij37Ww+fcWXxnYpO3xRAOthA@mail.gmail.com/
   
A short history/summary of the problem and solution space:

I made "PCI: Resources outside their window must set IORESOURCE_UNSET"
change that checks at the init time whether BARs belong to an upstream
window or not. If not, the resource is marked wit IORESOURCE_UNSET to 
indicate FW/platform didn't provide working addressing for those BARs.

On Geert's R-Car M2-W, it caused some BARs to be detected as not having a
an upstream window where they belong to:

     pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
     pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff -> 0x00ee080000
     pci-rcar-gen2 ee090000.pci: PCI: revision 11
     pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
     pci_bus 0000:00: root bus resource [bus 00]
     pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
     pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional PCI endpoint
    -pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
    -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
    +pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]: no initial claim (no window)
    +pci 0000:00:00.0: BAR 0 [mem size 0x00000400]
    +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no initial claim (no window)
    +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]

...In the log above, there's no root bus resource that covers
BAR0's 0xee090800-0xee090bff address range (which itself comes from DT 
"reg"), and thus it got marked IORESOURCE_UNSET with as it does not 
have window where it belongs to.

It's unclear to me whether DT ranges should have included BAR0 so that 
the root bus resources would cover that range?

I was then told the updaing ranges now will not be enough due to DT 
backwards compatibility requirements so it looks we have to resort to a 
change like this:

https://lore.kernel.org/linux-pci/7640a03e-dfea-db9c-80f5-d80fa2c505b7@linux.intel.com/

(+ a few supporting changes as that change exposed brokenness in PCI core.)

Does that look correct solution? That is, should these be added on 
case-by-case basis as additional root bus resources or should there be 
something more generic in the OF PCI code to do it?

There's also that BAR1 which seems to be related to dma_ranges and I don't 
know what to make of it. This resource comes with the added complication 
that this same address appears more than once (in the full log there's 
more than one PCI/USB instance). Again, this BAR1 is not covered by any 
root bus resource and thus gets flagged with IORESOURCE_UNSET. 

So I'm interested what is the "correct" solution for these resources that 
appear as BARs but do not have a backing root bus resource, is it having 
DT "ranges" cover them (I'm ignoring backwards compatibility aspect here) 
or something else?


In addition, is there something special/non-ordinary with these BARs and 
PCI core should treat them somehow differently? If that's the case, how 
can I identify such BARs from "normal" ones to avoid messing with them?


-- 
 i.

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Rob Herring]

On Wed, Oct 22, 2025 at 03:14:12PM +0300, Ilpo Järvinen wrote:
> On Wed, 22 Oct 2025, Geert Uytterhoeven wrote:
> > On Tue, 21 Oct 2025 at 13:54, Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > > On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:
> > > > On Mon, 20 Oct 2025 at 18:20, Ilpo Järvinen
> > > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > > On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> > > > > > On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> > > > > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > > > > Here's a series for Geert to test if this fixes the improper coalescing
> > > > > > > of resources as was experienced with the pci_add_resource() change (I
> > > > > > > know the breaking change was pulled before 6.18 main PR but I'd want to
> > > > > > > retry it later once the known issues have been addressed). The expected
> > > > > > > result is there'll be two adjacent host bridge resources in the
> > > > > > > resource tree as the different name should disallow coalescing them
> > > > > > > together, and therefore BAR0 has a window into which it belongs to.
> > > > > > >
> > > > > > > Generic info for the series:
> > > > > > >
> > > > > > > PCI host bridge windows were coalesced in place into one of the structs
> > > > > > > on the resources list. The host bridge window coalescing code does not
> > > > > > > know who holds references and still needs the struct resource it's
> > > > > > > coalescing from/to so it is safer to perform coalescing into entirely
> > > > > > > a new struct resource instead and leave the old resource addresses as
> > > > > > > they were.
> > > > > > >
> > > > > > > The checks when coalescing is allowed are also made stricter so that
> > > > > > > only resources that have identical the metadata can be coalesced.
> > > > > > >
> > > > > > > As a bonus, there's also a bit of framework to easily create kunit
> > > > > > > tests for resource tree functions (beyond just resource_coalesce()).
> > > > > > >
> > > > > > > Ilpo Järvinen (3):
> > > > > > >   PCI: Refactor host bridge window coalescing loop to use prev
> > > > > > >   PCI: Do not coalesce host bridge resource structs in place
> > > > > > >   resource, kunit: add test case for resource_coalesce()
> > > > > >
> > > > > > Thanks for your series!
> > > > > >
> > > > > > I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> > > > > > Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> > > > > > gave it a a try on Koelsch (R-Car M2-W).
> > > > >
> > > > > So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
> > > > > No coalescing would be attempted without that change.
> > > >
> > > > Sorry, I didn't realize you wanted that (and anything else) to be
> > > > included, too.  Please tell me the exact base I should use for testing,
> > > > and I will give it another run.
> > >
> > > I'm sorry, it's indeed a bit confusing as some of these patches never
> > > have been in Linus' tree.
> > >
> > > So I'm interested on what's the result with these changes/series together:
> > >
> > > [PATCH 1/2] PCI: Setup bridge resources earlier
> > > [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET
> > > [PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
> > > [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev
> > > [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
> > > [PATCH 3/3] resource, kunit: add test case for resource_coalesce()
> > >
> > > You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch
> > > to pci_info() (as otherwise dyndbg is necessary to make it visible).
> > 
> > Thanks, all done:
> > 
> >     $ git cherry -v --abbrev=1 v6.18-rc2^
> >     + 211ddde0 Linux 6.18-rc2
> >     + 3fdaf2 PCI: Setup bridge resources earlier
> >     + 5be02e5 PCI: Resources outside their window must set IORESOURCE_UNSET
> >     + adf6f11 PCI: rcar-gen2: Add BAR0 into host bridge resources
> >     + eecb500 PCI: Refactor host bridge window coalescing loop to use prev
> >     + 60470b3 PCI: Do not coalesce host bridge resource structs in place
> >     + afe3ec resource, kunit: add test case for resource_coalesce()
> >     + 487c98 Use dev_info() in drivers/pci/probe.c:__pci_read_base()
> > IORESOURCE_UNSET path
> > 
> > Compared to v6.18-rc2, dmesg changed (for the first PCI/USB instance)
> > like:
> > 
> >      pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
> >      pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff
> > -> 0x00ee080000
> >      pci-rcar-gen2 ee090000.pci: PCI: revision 11
> >      pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
> >      pci_bus 0000:00: root bus resource [bus 00]
> >      pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
> >     +pci_bus 0000:00: root bus resource [mem 0xee090000-0xee090bff]
> >      pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional
> > PCI endpoint
> >      pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
> >     -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
> >     +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no
> > initial claim (no window)
> >     +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
> >      pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310 conventional
> > PCI endpoint
> >     -pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]
> >     +pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]: no initial
> > claim (no window)
> >     +pci 0000:00:01.0: BAR 0 [mem size 0x00001000]
> >      pci 0000:00:01.0: supports D1 D2
> >      pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
> >      pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320 conventional
> > PCI endpoint
> >     -pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]
> >     +pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]: no initial
> > claim (no window)
> >     +pci 0000:00:02.0: BAR 0 [mem size 0x00000100]
> >      pci 0000:00:02.0: supports D1 D2
> >      pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
> >      PCI: bus0: Fast back to back transfers disabled
> >      pci 0000:00:01.0: BAR 0 [mem 0xee080000-0xee080fff]: assigned
> >      pci 0000:00:02.0: BAR 0 [mem 0xee081000-0xee0810ff]: assigned
> >      pci_bus 0000:00: resource 4 [mem 0xee080000-0xee08ffff]
> >     +pci_bus 0000:00: resource 5 [mem 0xee090000-0xee090bff]
> >      pci 0000:00:01.0: enabling device (0140 -> 0142)
> >      pci 0000:00:02.0: enabling device (0140 -> 0142)
> > 
> > > The expected result is that those usb resources are properly parented and
> > > the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as
> > > that would destroy information). So something along the lines of:
> > >
> > >     ee080000-ee08ffff : pci@ee090000
> > >       ee080000-ee080fff : 0000:00:01.0
> > >         ee080000-ee080fff : ohci_hcd
> > >       ee081000-ee0810ff : 0000:00:02.0
> > >         ee081000-ee0810ff : ehci_hcd
> > >     ee090000-ee090bff : ee090000.pci pci@ee090000
> > 
> > Compared to v6.18-rc2, the output of "lspci -v" or "cat /proc/iomem"
> > did not change.  Hence for the two PCI/USB instances:
> > 
> >     ee080000-ee08ffff : pci@ee090000
> >       ee080000-ee080fff : 0000:00:01.0
> >         ee080000-ee080fff : ohci_hcd
> >       ee081000-ee0810ff : 0000:00:02.0
> >         ee081000-ee0810ff : ehci_hcd
> >     ee090000-ee090bff : ee090000.pci pci@ee090000
> >     ee0c0000-ee0cffff : pci@ee0d0000
> >       ee0c0000-ee0c0fff : 0001:01:01.0
> >         ee0c0000-ee0c0fff : ohci_hcd
> >       ee0c1000-ee0c10ff : 0001:01:02.0
> >         ee0c1000-ee0c10ff : ehci_hcd
> >     ee0d0000-ee0d0bff : ee0d0000.pci pci@ee0d0000
> 
> Hi Rob,
> 
> I'd want to hear your opinion on the solutions me and Geert tried and
> discussed in the subthread starting from this:
> 
> https://lore.kernel.org/linux-pci/CAMuHMdVtVzcL3AX0uetNhKr-gLij37Ww+fcWXxnYpO3xRAOthA@mail.gmail.com/
>    
> A short history/summary of the problem and solution space:
> 
> I made "PCI: Resources outside their window must set IORESOURCE_UNSET"
> change that checks at the init time whether BARs belong to an upstream
> window or not. If not, the resource is marked wit IORESOURCE_UNSET to 
> indicate FW/platform didn't provide working addressing for those BARs.
> 
> On Geert's R-Car M2-W, it caused some BARs to be detected as not having a
> an upstream window where they belong to:
> 
>      pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
>      pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff -> 0x00ee080000
>      pci-rcar-gen2 ee090000.pci: PCI: revision 11
>      pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
>      pci_bus 0000:00: root bus resource [bus 00]
>      pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
>      pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional PCI endpoint
>     -pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
>     -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
>     +pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]: no initial claim (no window)
>     +pci 0000:00:00.0: BAR 0 [mem size 0x00000400]
>     +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no initial claim (no window)
>     +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
> 
> ...In the log above, there's no root bus resource that covers
> BAR0's 0xee090800-0xee090bff address range (which itself comes from DT 
> "reg"), and thus it got marked IORESOURCE_UNSET with as it does not 
> have window where it belongs to.
> 
> It's unclear to me whether DT ranges should have included BAR0 so that 
> the root bus resources would cover that range?

I think it should. Otherwise, how do you know if the region is 32 or 64 
bit or prefetchable or not?

> 
> I was then told the updaing ranges now will not be enough due to DT 
> backwards compatibility requirements so it looks we have to resort to a 
> change like this:
> 
> https://lore.kernel.org/linux-pci/7640a03e-dfea-db9c-80f5-d80fa2c505b7@linux.intel.com/

You aren't adding 'BAR0 region', but something else. Based on the name, 
cfg_res, that is config space? That seems wrong both in requiring it to 
be registered and that you would assign BAR0 to config space. How would 
that device operate when config space is not memory mapped?

For compatibility, the change itself doesn't look so bad. However, we 
could also fixup the DT itself. That's not done too much on Arm systems, 
but there's all sorts of fixups in powerpc. I have some initial 
infrastructure to support that if we need to go down that path.

> 
> (+ a few supporting changes as that change exposed brokenness in PCI core.)
> 
> Does that look correct solution? That is, should these be added on 
> case-by-case basis as additional root bus resources or should there be 
> something more generic in the OF PCI code to do it?
> 
> There's also that BAR1 which seems to be related to dma_ranges and I don't 
> know what to make of it. This resource comes with the added complication 
> that this same address appears more than once (in the full log there's 
> more than one PCI/USB instance). Again, this BAR1 is not covered by any 
> root bus resource and thus gets flagged with IORESOURCE_UNSET. 
> 
> So I'm interested what is the "correct" solution for these resources that 
> appear as BARs but do not have a backing root bus resource, is it having 
> DT "ranges" cover them (I'm ignoring backwards compatibility aspect here) 
> or something else?

If it is config space, no, I don't think ranges should cover that. 
'ranges' should have all the memory and io spaces.

> In addition, is there something special/non-ordinary with these BARs and 
> PCI core should treat them somehow differently? If that's the case, how 
> can I identify such BARs from "normal" ones to avoid messing with them?

You've exceeded my PCI knowledge on this...

Rob

Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer

[Bjorn Helgaas]

[+cc Marek, Yoshihiro for rcar-gen2]

On Wed, Oct 22, 2025 at 03:14:12PM +0300, Ilpo Järvinen wrote:
> On Wed, 22 Oct 2025, Geert Uytterhoeven wrote:
> > On Tue, 21 Oct 2025 at 13:54, Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > > On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:
> > > > On Mon, 20 Oct 2025 at 18:20, Ilpo Järvinen
> > > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > > On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> > > > > > On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> > > > > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > > > > Here's a series for Geert to test if this fixes the improper coalescing
> > > > > > > of resources as was experienced with the pci_add_resource() change (I
> > > > > > > know the breaking change was pulled before 6.18 main PR but I'd want to
> > > > > > > retry it later once the known issues have been addressed). The expected
> > > > > > > result is there'll be two adjacent host bridge resources in the
> > > > > > > resource tree as the different name should disallow coalescing them
> > > > > > > together, and therefore BAR0 has a window into which it belongs to.
> > > > > > >
> > > > > > > Generic info for the series:
> > > > > > >
> > > > > > > PCI host bridge windows were coalesced in place into one of the structs
> > > > > > > on the resources list. The host bridge window coalescing code does not
> > > > > > > know who holds references and still needs the struct resource it's
> > > > > > > coalescing from/to so it is safer to perform coalescing into entirely
> > > > > > > a new struct resource instead and leave the old resource addresses as
> > > > > > > they were.
> > > > > > >
> > > > > > > The checks when coalescing is allowed are also made stricter so that
> > > > > > > only resources that have identical the metadata can be coalesced.
> > > > > > >
> > > > > > > As a bonus, there's also a bit of framework to easily create kunit
> > > > > > > tests for resource tree functions (beyond just resource_coalesce()).
> > > > > > >
> > > > > > > Ilpo Järvinen (3):
> > > > > > >   PCI: Refactor host bridge window coalescing loop to use prev
> > > > > > >   PCI: Do not coalesce host bridge resource structs in place
> > > > > > >   resource, kunit: add test case for resource_coalesce()
> > > > > >
> > > > > > Thanks for your series!
> > > > > >
> > > > > > I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> > > > > > Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> > > > > > gave it a a try on Koelsch (R-Car M2-W).
> > > > >
> > > > > So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
> > > > > No coalescing would be attempted without that change.
> > > >
> > > > Sorry, I didn't realize you wanted that (and anything else) to be
> > > > included, too.  Please tell me the exact base I should use for testing,
> > > > and I will give it another run.
> > >
> > > I'm sorry, it's indeed a bit confusing as some of these patches never
> > > have been in Linus' tree.
> > >
> > > So I'm interested on what's the result with these changes/series together:
> > >
> > > [PATCH 1/2] PCI: Setup bridge resources earlier
> > > [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET
> > > [PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
> > > [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev
> > > [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
> > > [PATCH 3/3] resource, kunit: add test case for resource_coalesce()
> > >
> > > You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch
> > > to pci_info() (as otherwise dyndbg is necessary to make it visible).
> > 
> > Thanks, all done:
> > 
> >     $ git cherry -v --abbrev=1 v6.18-rc2^
> >     + 211ddde0 Linux 6.18-rc2
> >     + 3fdaf2 PCI: Setup bridge resources earlier
> >     + 5be02e5 PCI: Resources outside their window must set IORESOURCE_UNSET
> >     + adf6f11 PCI: rcar-gen2: Add BAR0 into host bridge resources
> >     + eecb500 PCI: Refactor host bridge window coalescing loop to use prev
> >     + 60470b3 PCI: Do not coalesce host bridge resource structs in place
> >     + afe3ec resource, kunit: add test case for resource_coalesce()
> >     + 487c98 Use dev_info() in drivers/pci/probe.c:__pci_read_base()
> > IORESOURCE_UNSET path
> > 
> > Compared to v6.18-rc2, dmesg changed (for the first PCI/USB instance)
> > like:
> > 
> >      pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
> >      pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff
> > -> 0x00ee080000
> >      pci-rcar-gen2 ee090000.pci: PCI: revision 11
> >      pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
> >      pci_bus 0000:00: root bus resource [bus 00]
> >      pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
> >     +pci_bus 0000:00: root bus resource [mem 0xee090000-0xee090bff]
> >      pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional
> > PCI endpoint
> >      pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
> >     -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
> >     +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no
> > initial claim (no window)
> >     +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
> >      pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310 conventional
> > PCI endpoint
> >     -pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]
> >     +pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]: no initial
> > claim (no window)
> >     +pci 0000:00:01.0: BAR 0 [mem size 0x00001000]
> >      pci 0000:00:01.0: supports D1 D2
> >      pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
> >      pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320 conventional
> > PCI endpoint
> >     -pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]
> >     +pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]: no initial
> > claim (no window)
> >     +pci 0000:00:02.0: BAR 0 [mem size 0x00000100]
> >      pci 0000:00:02.0: supports D1 D2
> >      pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
> >      PCI: bus0: Fast back to back transfers disabled
> >      pci 0000:00:01.0: BAR 0 [mem 0xee080000-0xee080fff]: assigned
> >      pci 0000:00:02.0: BAR 0 [mem 0xee081000-0xee0810ff]: assigned
> >      pci_bus 0000:00: resource 4 [mem 0xee080000-0xee08ffff]
> >     +pci_bus 0000:00: resource 5 [mem 0xee090000-0xee090bff]
> >      pci 0000:00:01.0: enabling device (0140 -> 0142)
> >      pci 0000:00:02.0: enabling device (0140 -> 0142)
> > 
> > > The expected result is that those usb resources are properly parented and
> > > the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as
> > > that would destroy information). So something along the lines of:
> > >
> > >     ee080000-ee08ffff : pci@ee090000
> > >       ee080000-ee080fff : 0000:00:01.0
> > >         ee080000-ee080fff : ohci_hcd
> > >       ee081000-ee0810ff : 0000:00:02.0
> > >         ee081000-ee0810ff : ehci_hcd
> > >     ee090000-ee090bff : ee090000.pci pci@ee090000
> > 
> > Compared to v6.18-rc2, the output of "lspci -v" or "cat /proc/iomem"
> > did not change.  Hence for the two PCI/USB instances:
> > 
> >     ee080000-ee08ffff : pci@ee090000
> >       ee080000-ee080fff : 0000:00:01.0
> >         ee080000-ee080fff : ohci_hcd
> >       ee081000-ee0810ff : 0000:00:02.0
> >         ee081000-ee0810ff : ehci_hcd
> >     ee090000-ee090bff : ee090000.pci pci@ee090000
> >     ee0c0000-ee0cffff : pci@ee0d0000
> >       ee0c0000-ee0c0fff : 0001:01:01.0
> >         ee0c0000-ee0c0fff : ohci_hcd
> >       ee0c1000-ee0c10ff : 0001:01:02.0
> >         ee0c1000-ee0c10ff : ehci_hcd
> >     ee0d0000-ee0d0bff : ee0d0000.pci pci@ee0d0000
> 
> Hi Rob,
> 
> I'd want to hear your opinion on the solutions me and Geert tried and
> discussed in the subthread starting from this:
> 
> https://lore.kernel.org/linux-pci/CAMuHMdVtVzcL3AX0uetNhKr-gLij37Ww+fcWXxnYpO3xRAOthA@mail.gmail.com/
>    
> A short history/summary of the problem and solution space:
> 
> I made "PCI: Resources outside their window must set IORESOURCE_UNSET"
> change that checks at the init time whether BARs belong to an upstream
> window or not. If not, the resource is marked wit IORESOURCE_UNSET to 
> indicate FW/platform didn't provide working addressing for those BARs.
> 
> On Geert's R-Car M2-W, it caused some BARs to be detected as not having a
> an upstream window where they belong to:
> 
>      pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
>      pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff -> 0x00ee080000
>      pci-rcar-gen2 ee090000.pci: PCI: revision 11
>      pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
>      pci_bus 0000:00: root bus resource [bus 00]
>      pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
>      pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional PCI endpoint
>     -pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
>     -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
>     +pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]: no initial claim (no window)
>     +pci 0000:00:00.0: BAR 0 [mem size 0x00000400]
>     +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no initial claim (no window)
>     +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
> 
> ...In the log above, there's no root bus resource that covers
> BAR0's 0xee090800-0xee090bff address range (which itself comes from DT 
> "reg"), and thus it got marked IORESOURCE_UNSET with as it does not 
> have window where it belongs to.

I guess this came from arch/arm/boot/dts/renesas/r8a7791.dtsi:

                pci0: pci@ee090000 {
                        compatible = "renesas,pci-r8a7791",
                                     "renesas,pci-rcar-gen2";
                        reg = <0 0xee090000 0 0xc00>,
                              <0 0xee080000 0 0x1100>;
                        ranges = <0x02000000 0 0xee080000 0 0xee080000 0 0x00010000>;
                        bus-range = <0 0>;

                pciec: pcie@fe000000 {
                        compatible = "renesas,pcie-r8a7791",
                                     "renesas,pcie-rcar-gen2";
                        dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000>,
                        bus-range = <0x00 0xff>;

"reg" describes address space on a device's parent side, e.g.,
memory-mapped registers consumed by the device itself.  Since this is
a host bridge, it also has "ranges" to describe address space on its
child side, including any address translation from the parent side.

PCI devices are on the child side and should consume space from the
"ranges" child side, [mem 0xee080000-0xee08ffff] in this case.

00:00.0: BAR 0 [mem 0xee090800-0xee090bff] isn't in that child address
space so I think it's invalid per spec.  Does 00:00.0 actually respond
at whatever address is in BAR 0?  Do we care?  I don't see any driver
that claims [1033:0000].

00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref] looks like it came
from the pcie@fe000000 "dma-ranges", which describes valid child
address space for DMA.  We need to know the DMA address space, of
course, but we get it from "dma-ranges", not from a BAR.

I think it would be nicest if pci-rcar-gen2.c could hide both BAR 0
and 1 somehow so they wouldn't confuse us.

(BTW, I don't understand how both pci@ee090000 and pcie@fe000000 say
they are bridges to bus 0.)

> It's unclear to me whether DT ranges should have included BAR0 so that 
> the root bus resources would cover that range?
> 
> I was then told the updating ranges now will not be enough due to DT 
> backwards compatibility requirements so it looks we have to resort to a 
> change like this:
> 
> https://lore.kernel.org/linux-pci/7640a03e-dfea-db9c-80f5-d80fa2c505b7@linux.intel.com/
> 
> (+ a few supporting changes as that change exposed brokenness in PCI core.)
> 
> Does that look correct solution? That is, should these be added on 
> case-by-case basis as additional root bus resources or should there be 
> something more generic in the OF PCI code to do it?
> 
> There's also that BAR1 which seems to be related to dma_ranges and I don't 
> know what to make of it. This resource comes with the added complication 
> that this same address appears more than once (in the full log there's 
> more than one PCI/USB instance). Again, this BAR1 is not covered by any 
> root bus resource and thus gets flagged with IORESOURCE_UNSET. 
> 
> So I'm interested what is the "correct" solution for these resources that 
> appear as BARs but do not have a backing root bus resource, is it having 
> DT "ranges" cover them (I'm ignoring backwards compatibility aspect here) 
> or something else?
> 
> 
> In addition, is there something special/non-ordinary with these BARs and 
> PCI core should treat them somehow differently? If that's the case, how 
> can I identify such BARs from "normal" ones to avoid messing with them?
> 
> 
> -- 
>  i.