[PATCH] checks: Warn about overlapping reserved-memory ranges

[PATCH] checks: Warn about overlapping reserved-memory ranges

[Bjorn Andersson]

Static /reserved-memory entries associate physical address ranges with
particular purposes and properties. While not explicitly prohibited,
overlapping ranges commonly indicate an incorrect address or size and
may assign the same memory conflicting attributes. Dtc currently
provides no diagnostic for this type of mistake.

Check every non-empty reg range under available reserved-memory children
and report overlaps. Add tests covering overlapping and adjacent ranges,
as well as unavailable, zero-sized, and dynamically allocated regions.

Assisted-by: pi:gpt-5.6-sol
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
---
 checks.c                             | 131 +++++++++++++++++++++++++++++++++++
 tests/reserved-memory-no-overlap.dts |  43 ++++++++++++
 tests/reserved-memory-overlap.dts    |  21 ++++++
 tests/run_tests.sh                   |   2 +
 4 files changed, 197 insertions(+)

diff --git a/checks.c b/checks.c
index a6037ed08000..baa0914e37a5 100644
--- a/checks.c
+++ b/checks.c
@@ -1266,6 +1266,136 @@ static bool node_is_disabled(struct node *node)
 	return false;
 }
 
+static bool node_is_available(struct node *node)
+{
+	struct property *prop;
+	const char *status;
+
+	prop = get_property(node, "status");
+	if (!prop)
+		return true;
+
+	if (!data_is_one_string(prop->val))
+		return false;
+
+	status = prop->val.val;
+	return streq(status, "okay") || streq(status, "ok");
+}
+
+static bool get_reg_value(struct property *reg, unsigned int cell,
+			  int cell_count, uint64_t *value)
+{
+	uint64_t val = 0;
+	int i;
+
+	if (cell_count < 0 || cell_count > 2)
+		return false;
+
+	for (i = 0; i < cell_count; i++)
+		val = (val << 32) | propval_cell_n(reg, cell + i);
+
+	*value = val;
+	return true;
+}
+
+static bool ranges_overlap(uint64_t addr_a, uint64_t size_a,
+			   uint64_t addr_b, uint64_t size_b)
+{
+	if (!size_a || !size_b)
+		return false;
+
+	if (addr_a <= addr_b)
+		return addr_b - addr_a < size_a;
+
+	return addr_a - addr_b < size_b;
+}
+
+static void format_range_end(char end_str[20], uint64_t addr, uint64_t size)
+{
+	uint64_t end = addr + size;
+
+	if (end < addr)
+		snprintf(end_str, 20, "0x1%016" PRIx64, end);
+	else
+		snprintf(end_str, 20, "0x%" PRIx64, end);
+}
+
+static void check_reserved_memory_overlap(struct check *c,
+					  struct dt_info *dti,
+					  struct node *node)
+{
+	struct reserve_range {
+		struct node *node;
+		uint64_t addr;
+		uint64_t size;
+	} *ranges = NULL;
+	struct node *child;
+	unsigned int range_count = 0;
+	int addr_cells, size_cells, entry_cells;
+
+	if (!streq(node->fullpath, "/reserved-memory"))
+		return;
+
+	addr_cells = node_addr_cells(node);
+	size_cells = node_size_cells(node);
+	entry_cells = addr_cells + size_cells;
+
+	/* Physical addresses and sizes wider than 64 bits cannot be compared. */
+	if (addr_cells > 2 || size_cells > 2 || entry_cells <= 0)
+		return;
+
+	for_each_child(node, child) {
+		struct property *reg;
+		unsigned int cell;
+
+		if (!node_is_available(child))
+			continue;
+
+		reg = get_property(child, "reg");
+		if (!reg)
+			continue;
+
+		for (cell = 0; cell < reg->val.len / sizeof(cell_t);
+		     cell += entry_cells) {
+			uint64_t addr, size;
+			unsigned int i;
+
+			if (!get_reg_value(reg, cell, addr_cells, &addr) ||
+			    !get_reg_value(reg, cell + addr_cells,
+					   size_cells, &size))
+				continue;
+
+			for (i = 0; i < range_count; i++) {
+				char end[20], other_end[20];
+
+				if (!ranges_overlap(addr, size, ranges[i].addr,
+						    ranges[i].size))
+					continue;
+
+				format_range_end(end, addr, size);
+				format_range_end(other_end, ranges[i].addr,
+						 ranges[i].size);
+				FAIL(c, dti, child,
+				     "reg range [0x%" PRIx64 ", %s) overlaps with %s "
+				     "[0x%" PRIx64 ", %s)",
+				     addr, end, ranges[i].node->fullpath,
+				     ranges[i].addr, other_end);
+			}
+
+			ranges = xrealloc(ranges,
+					  (range_count + 1) * sizeof(*ranges));
+			ranges[range_count].node = child;
+			ranges[range_count].addr = addr;
+			ranges[range_count].size = size;
+			range_count++;
+		}
+	}
+
+	free(ranges);
+}
+WARNING(reserved_memory_overlap, check_reserved_memory_overlap, NULL,
+	&addr_size_cells, &reg_format);
+
 static void check_unique_unit_address_common(struct check *c,
 						struct dt_info *dti,
 						struct node *node,
@@ -1945,6 +2075,7 @@ static struct check *check_table[] = {
 
 	&avoid_default_addr_size,
 	&avoid_unnecessary_addr_size,
+	&reserved_memory_overlap,
 	&unique_unit_address,
 	&unique_unit_address_if_enabled,
 	&obsolete_chosen_interrupt_controller,
diff --git a/tests/reserved-memory-no-overlap.dts b/tests/reserved-memory-no-overlap.dts
new file mode 100644
index 000000000000..0293986ef746
--- /dev/null
+++ b/tests/reserved-memory-no-overlap.dts
@@ -0,0 +1,43 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		first@100000000 {
+			reg = <0x1 0x00000000 0x0 0x00100000>;
+		};
+
+		adjacent@100100000 {
+			reg = <0x1 0x00100000 0x0 0x00100000>;
+		};
+
+		disabled@100020000 {
+			reg = <0x1 0x00020000 0x0 0x00100000>;
+			status = "disabled";
+		};
+
+		reserved@100040000 {
+			reg = <0x1 0x00040000 0x0 0x00100000>;
+			status = "reserved";
+		};
+
+		failed@100060000 {
+			reg = <0x1 0x00060000 0x0 0x00100000>;
+			status = "fail";
+		};
+
+		zero-size@100080000 {
+			reg = <0x1 0x00080000 0x0 0x0>;
+		};
+
+		dynamic {
+			size = <0x0 0x00100000>;
+		};
+	};
+};
diff --git a/tests/reserved-memory-overlap.dts b/tests/reserved-memory-overlap.dts
new file mode 100644
index 000000000000..fec7c3a33b1f
--- /dev/null
+++ b/tests/reserved-memory-overlap.dts
@@ -0,0 +1,21 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		first@100000000 {
+			reg = <0x1 0x00000000 0x0 0x00100000>,
+			      <0x2 0x00000000 0x0 0x00200000>;
+		};
+
+		second@200100000 {
+			reg = <0x2 0x00100000 0x0 0x00100000>;
+		};
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 842b543059bb..27c3ebfed338 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -746,6 +746,8 @@ dtc_tests () {
     check_tests "$SRCDIR/reg-ranges-root.dts" reg_format ranges_format
     check_tests "$SRCDIR/bad-dma-ranges.dts" dma_ranges_format
     check_tests "$SRCDIR/default-addr-size.dts" avoid_default_addr_size
+    check_tests "$SRCDIR/reserved-memory-overlap.dts" reserved_memory_overlap
+    check_tests "$SRCDIR/reserved-memory-no-overlap.dts" -n reserved_memory_overlap
     check_tests "$SRCDIR/obsolete-chosen-interrupt-controller.dts" obsolete_chosen_interrupt_controller
     check_tests "$SRCDIR/reg-without-unit-addr.dts" unit_address_vs_reg
     check_tests "$SRCDIR/unit-addr-without-reg.dts" unit_address_vs_reg

---
base-commit: 7a1e017926004ecff5fce62d62d42ce9f3e00082
change-id: 20260923-check-reserved-overlap-6c5fc2214ea4

Best regards,
--  
Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>

Re: [PATCH] checks: Warn about overlapping reserved-memory ranges

[Herve Codina]

Hi Bjorn,


On Wed, 23 Sep 2026 17:50:18 +0000
Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> wrote:

> Static /reserved-memory entries associate physical address ranges with
> particular purposes and properties. While not explicitly prohibited,
> overlapping ranges commonly indicate an incorrect address or size and
> may assign the same memory conflicting attributes. Dtc currently
> provides no diagnostic for this type of mistake.
> 
> Check every non-empty reg range under available reserved-memory children
> and report overlaps. Add tests covering overlapping and adjacent ranges,
> as well as unavailable, zero-sized, and dynamically allocated regions.

I am not so sure that this has to checked by dtc.

Should dtc have the knowledge of /reserved-memory nodes and sub-nodes meaning?

Overlaps on regs and size can appear in many places in the devicetree which
are not correct from a functional point of view. Should dtc check this kind
of issue? I am not so sure.

I'm not in a position to judge the validity of the test but, anyway, I can
review the code.

> 
> Assisted-by: pi:gpt-5.6-sol
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> ---
>  checks.c                             | 131 +++++++++++++++++++++++++++++++++++
>  tests/reserved-memory-no-overlap.dts |  43 ++++++++++++
>  tests/reserved-memory-overlap.dts    |  21 ++++++
>  tests/run_tests.sh                   |   2 +
>  4 files changed, 197 insertions(+)
> 
> diff --git a/checks.c b/checks.c
> index a6037ed08000..baa0914e37a5 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1266,6 +1266,136 @@ static bool node_is_disabled(struct node *node)
>  	return false;
>  }
>  
> +static bool node_is_available(struct node *node)
> +{
> +	struct property *prop;
> +	const char *status;
> +
> +	prop = get_property(node, "status");
> +	if (!prop)
> +		return true;
> +
> +	if (!data_is_one_string(prop->val))
> +		return false;
> +
> +	status = prop->val.val;
> +	return streq(status, "okay") || streq(status, "ok");
> +}
> +
> +static bool get_reg_value(struct property *reg, unsigned int cell,
> +			  int cell_count, uint64_t *value)
> +{
> +	uint64_t val = 0;
> +	int i;
> +
> +	if (cell_count < 0 || cell_count > 2)
> +		return false;
> +
> +	for (i = 0; i < cell_count; i++)
> +		val = (val << 32) | propval_cell_n(reg, cell + i);
> +
> +	*value = val;
> +	return true;
> +}
> +
> +static bool ranges_overlap(uint64_t addr_a, uint64_t size_a,
> +			   uint64_t addr_b, uint64_t size_b)
> +{
> +	if (!size_a || !size_b)
> +		return false;
> +
> +	if (addr_a <= addr_b)
> +		return addr_b - addr_a < size_a;
> +
> +	return addr_a - addr_b < size_b;
> +}
> +
> +static void format_range_end(char end_str[20], uint64_t addr, uint64_t size)
> +{
> +	uint64_t end = addr + size;
> +
> +	if (end < addr)
> +		snprintf(end_str, 20, "0x1%016" PRIx64, end);
> +	else
> +		snprintf(end_str, 20, "0x%" PRIx64, end);
> +}
> +
> +static void check_reserved_memory_overlap(struct check *c,
> +					  struct dt_info *dti,
> +					  struct node *node)
> +{
> +	struct reserve_range {
> +		struct node *node;
> +		uint64_t addr;
> +		uint64_t size;
> +	} *ranges = NULL;
> +	struct node *child;
> +	unsigned int range_count = 0;
> +	int addr_cells, size_cells, entry_cells;
> +
> +	if (!streq(node->fullpath, "/reserved-memory"))
> +		return;
> +
> +	addr_cells = node_addr_cells(node);
> +	size_cells = node_size_cells(node);
> +	entry_cells = addr_cells + size_cells;
> +
> +	/* Physical addresses and sizes wider than 64 bits cannot be compared. */
> +	if (addr_cells > 2 || size_cells > 2 || entry_cells <= 0)
> +		return;
> +
> +	for_each_child(node, child) {
> +		struct property *reg;
> +		unsigned int cell;
> +
> +		if (!node_is_available(child))
> +			continue;

Could a "status" property be present in a /reserved-memory subnode?

> +
> +		reg = get_property(child, "reg");
> +		if (!reg)
> +			continue;
> +
> +		for (cell = 0; cell < reg->val.len / sizeof(cell_t);
> +		     cell += entry_cells) {
> +			uint64_t addr, size;
> +			unsigned int i;
> +
> +			if (!get_reg_value(reg, cell, addr_cells, &addr) ||
> +			    !get_reg_value(reg, cell + addr_cells,
> +					   size_cells, &size))
> +				continue;
> +
> +			for (i = 0; i < range_count; i++) {
> +				char end[20], other_end[20];
> +
> +				if (!ranges_overlap(addr, size, ranges[i].addr,
> +						    ranges[i].size))
> +					continue;
> +
> +				format_range_end(end, addr, size);
> +				format_range_end(other_end, ranges[i].addr,
> +						 ranges[i].size);

Why do you need to convert to a string ?


> +				FAIL(c, dti, child,
> +				     "reg range [0x%" PRIx64 ", %s) overlaps with %s "
> +				     "[0x%" PRIx64 ", %s)",
> +				     addr, end, ranges[i].node->fullpath,
> +				     ranges[i].addr, other_end);

Weird to have '[' and ')'.

Also, IHMO, end and other_end don't bring any benefit in the message.

I think the following message is enough:
--- 8< ---
  "reg 0x%PRIx64 size 0x%PRIx64 overlaps with reg 0x%PRIx64 size 0x%PRIx64 from %s",
     addr, size, ranges[i].addr, ranges[i].size, ranges[i].node->fullpath
--- 8< ---

or in a more compact way:
 "reg (0x%PRIx64, 0x%PRIx64) overlaps with reg (0x%PRIx64, 0x%PRIx64) from %s",

> +			}
> +
> +			ranges = xrealloc(ranges,
> +					  (range_count + 1) * sizeof(*ranges));
> +			ranges[range_count].node = child;
> +			ranges[range_count].addr = addr;
> +			ranges[range_count].size = size;
> +			range_count++;
> +		}
> +	}
> +
> +	free(ranges);
> +}
> +WARNING(reserved_memory_overlap, check_reserved_memory_overlap, NULL,
> +	&addr_size_cells, &reg_format);
> +
>  static void check_unique_unit_address_common(struct check *c,
>  						struct dt_info *dti,
>  						struct node *node,
> @@ -1945,6 +2075,7 @@ static struct check *check_table[] = {
>  
>  	&avoid_default_addr_size,
>  	&avoid_unnecessary_addr_size,
> +	&reserved_memory_overlap,
>  	&unique_unit_address,
>  	&unique_unit_address_if_enabled,
>  	&obsolete_chosen_interrupt_controller,
> diff --git a/tests/reserved-memory-no-overlap.dts b/tests/reserved-memory-no-overlap.dts
> new file mode 100644
> index 000000000000..0293986ef746
> --- /dev/null
> +++ b/tests/reserved-memory-no-overlap.dts
> @@ -0,0 +1,43 @@
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		first@100000000 {
> +			reg = <0x1 0x00000000 0x0 0x00100000>;
> +		};
> +
> +		adjacent@100100000 {
> +			reg = <0x1 0x00100000 0x0 0x00100000>;
> +		};
> +
> +		disabled@100020000 {
> +			reg = <0x1 0x00020000 0x0 0x00100000>;
> +			status = "disabled";
> +		};
> +
> +		reserved@100040000 {
> +			reg = <0x1 0x00040000 0x0 0x00100000>;
> +			status = "reserved";
> +		};
> +
> +		failed@100060000 {
> +			reg = <0x1 0x00060000 0x0 0x00100000>;
> +			status = "fail";
> +		};
> +
> +		zero-size@100080000 {
> +			reg = <0x1 0x00080000 0x0 0x0>;
> +		};
> +
> +		dynamic {
> +			size = <0x0 0x00100000>;
> +		};
> +	};
> +};
> diff --git a/tests/reserved-memory-overlap.dts b/tests/reserved-memory-overlap.dts
> new file mode 100644
> index 000000000000..fec7c3a33b1f
> --- /dev/null
> +++ b/tests/reserved-memory-overlap.dts
> @@ -0,0 +1,21 @@
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		first@100000000 {
> +			reg = <0x1 0x00000000 0x0 0x00100000>,
> +			      <0x2 0x00000000 0x0 0x00200000>;
> +		};
> +
> +		second@200100000 {
> +			reg = <0x2 0x00100000 0x0 0x00100000>;
> +		};

And other dts with only one node but with a reg composed to 2 items which
overlap could be a good test pattern.

> +	};
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 842b543059bb..27c3ebfed338 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -746,6 +746,8 @@ dtc_tests () {
>      check_tests "$SRCDIR/reg-ranges-root.dts" reg_format ranges_format
>      check_tests "$SRCDIR/bad-dma-ranges.dts" dma_ranges_format
>      check_tests "$SRCDIR/default-addr-size.dts" avoid_default_addr_size
> +    check_tests "$SRCDIR/reserved-memory-overlap.dts" reserved_memory_overlap
> +    check_tests "$SRCDIR/reserved-memory-no-overlap.dts" -n reserved_memory_overlap
>      check_tests "$SRCDIR/obsolete-chosen-interrupt-controller.dts" obsolete_chosen_interrupt_controller
>      check_tests "$SRCDIR/reg-without-unit-addr.dts" unit_address_vs_reg
>      check_tests "$SRCDIR/unit-addr-without-reg.dts" unit_address_vs_reg
> 
> ---
> base-commit: 7a1e017926004ecff5fce62d62d42ce9f3e00082
> change-id: 20260923-check-reserved-overlap-6c5fc2214ea4
> 
> Best regards,
> --  
> Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> 
> 

Best regards,
Hervé

Re: [PATCH] checks: Warn about overlapping reserved-memory ranges

[David Gibson]

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

On Fri, Sep 25, 2026 at 07:03:00PM +0200, Herve Codina wrote:
> Hi Bjorn,
> 
> 
> On Wed, 23 Sep 2026 17:50:18 +0000
> Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> wrote:
> 
> > Static /reserved-memory entries associate physical address ranges with
> > particular purposes and properties. While not explicitly prohibited,
> > overlapping ranges commonly indicate an incorrect address or size and
> > may assign the same memory conflicting attributes. Dtc currently
> > provides no diagnostic for this type of mistake.
> > 
> > Check every non-empty reg range under available reserved-memory children
> > and report overlaps. Add tests covering overlapping and adjacent ranges,
> > as well as unavailable, zero-sized, and dynamically allocated regions.
> 
> I am not so sure that this has to checked by dtc.

It doesn't have to be, but that doesn't mean it isn't useful to do so.

> Should dtc have the knowledge of /reserved-memory nodes and
> sub-nodes meaning?

For the core logic, no.  But checks.c is explicitly about making
semantic checks of the tree's contents.  This is at the very least a
reasonable candidate for a check.

> Overlaps on regs and size can appear in many places in the devicetree which
> are not correct from a functional point of view. Should dtc check this kind
> of issue? I am not so sure.

Again, it doesn't have to, but it could still be useful.  What we
check in checks.c is not that extensive, but that's because of policy,
it's just because no-one's implemented it.

[snip]
> > +	for_each_child(node, child) {
> > +		struct property *reg;
> > +		unsigned int cell;
> > +
> > +		if (!node_is_available(child))
> > +			continue;
> 
> Could a "status" property be present in a /reserved-memory subnode?

That's a good question, it certainly seems an odd thing to have in
something that's not an actual device, but I'm not familiar with the
/reserved-memory binding off the top of my head.

> > +
> > +		reg = get_property(child, "reg");
> > +		if (!reg)
> > +			continue;
> > +
> > +		for (cell = 0; cell < reg->val.len / sizeof(cell_t);
> > +		     cell += entry_cells) {
> > +			uint64_t addr, size;
> > +			unsigned int i;
> > +
> > +			if (!get_reg_value(reg, cell, addr_cells, &addr) ||
> > +			    !get_reg_value(reg, cell + addr_cells,
> > +					   size_cells, &size))
> > +				continue;
> > +
> > +			for (i = 0; i < range_count; i++) {
> > +				char end[20], other_end[20];
> > +
> > +				if (!ranges_overlap(addr, size, ranges[i].addr,
> > +						    ranges[i].size))
> > +					continue;
> > +
> > +				format_range_end(end, addr, size);
> > +				format_range_end(other_end, ranges[i].addr,
> > +						 ranges[i].size);
> 
> Why do you need to convert to a string ?
> 
> 
> > +				FAIL(c, dti, child,
> > +				     "reg range [0x%" PRIx64 ", %s) overlaps with %s "
> > +				     "[0x%" PRIx64 ", %s)",
> > +				     addr, end, ranges[i].node->fullpath,
> > +				     ranges[i].addr, other_end);
> 
> Weird to have '[' and ')'.

[x..y) is a standard notation in maths (it means x is included in the
range, but y is not).  But that doesn't mean it's generally known.


> Also, IHMO, end and other_end don't bring any benefit in the message.
> 
> I think the following message is enough:
> --- 8< ---
>   "reg 0x%PRIx64 size 0x%PRIx64 overlaps with reg 0x%PRIx64 size 0x%PRIx64 from %s",
>      addr, size, ranges[i].addr, ranges[i].size, ranges[i].node->fullpath
> --- 8< ---

This looks better to me.

> or in a more compact way:
>  "reg (0x%PRIx64, 0x%PRIx64) overlaps with reg (0x%PRIx64, 0x%PRIx64) from %s",

I don't really like this format, though.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] checks: Warn about overlapping reserved-memory ranges

[Herve Codina]

Hi,

On Wed, 30 Sep 2026 19:44:42 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

...

> > 
> > Weird to have '[' and ')'.  
> 
> [x..y) is a standard notation in maths (it means x is included in the
> range, but y is not).  But that doesn't mean it's generally known.
> 

I was surprised by the notation and I've looked deeper about it.

Two different math notations exist for an interval notation [0]. The one
you mention (with square brackets and parenthesis) but also the one I know
about (square brackets only).

Knowing now about both notation, having "[x, y)" is not weird anymore. It
matches the "[x, y[" syntax I know about.

[0] https://en.wikipedia.org/wiki/Interval_(mathematics)#Including_or_excluding_endpoints

Hervé

Re: [PATCH] checks: Warn about overlapping reserved-memory ranges

[David Gibson]

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

On Wed, Sep 30, 2026 at 01:45:22PM +0200, Herve Codina wrote:
> Hi,
> 
> On Wed, 30 Sep 2026 19:44:42 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> ...
> 
> > > 
> > > Weird to have '[' and ')'.  
> > 
> > [x..y) is a standard notation in maths (it means x is included in the
> > range, but y is not).  But that doesn't mean it's generally known.
> > 
> 
> I was surprised by the notation and I've looked deeper about it.
> 
> Two different math notations exist for an interval notation [0]. The one
> you mention (with square brackets and parenthesis) but also the one I know
> about (square brackets only).
> 
> Knowing now about both notation, having "[x, y)" is not weird anymore. It
> matches the "[x, y[" syntax I know about.

Oh, interesting. I hadn't encountered the "[x, y[" form.  I wonder if
that's (loosely) an Anglosphere/Francosphere thing (the [x, y[ form
coming from Bourbaki).


> [0] https://en.wikipedia.org/wiki/Interval_(mathematics)#Including_or_excluding_endpoints
> 
> Hervé
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] checks: Warn about overlapping reserved-memory ranges

[David Gibson]

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

On Wed, Sep 23, 2026 at 05:50:18PM +0000, Bjorn Andersson wrote:
> Static /reserved-memory entries associate physical address ranges with
> particular purposes and properties. While not explicitly prohibited,
> overlapping ranges commonly indicate an incorrect address or size and
> may assign the same memory conflicting attributes. Dtc currently
> provides no diagnostic for this type of mistake.
> 
> Check every non-empty reg range under available reserved-memory children
> and report overlaps. Add tests covering overlapping and adjacent ranges,
> as well as unavailable, zero-sized, and dynamically allocated regions.

Hm.  It occurs to me that overlapping reg values are generally bad on
*any* bus, not just /reserved-memory.  It might be worth generalising
that part of the test.

> Assisted-by: pi:gpt-5.6-sol
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> ---
>  checks.c                             | 131 +++++++++++++++++++++++++++++++++++
>  tests/reserved-memory-no-overlap.dts |  43 ++++++++++++
>  tests/reserved-memory-overlap.dts    |  21 ++++++
>  tests/run_tests.sh                   |   2 +
>  4 files changed, 197 insertions(+)
> 
> diff --git a/checks.c b/checks.c
> index a6037ed08000..baa0914e37a5 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1266,6 +1266,136 @@ static bool node_is_disabled(struct node *node)
>  	return false;
>  }
>  
> +static bool node_is_available(struct node *node)

Do we really need a new function, rather than just
!node_is_disabled()?

> +{
> +	struct property *prop;
> +	const char *status;
> +
> +	prop = get_property(node, "status");
> +	if (!prop)
> +		return true;
> +
> +	if (!data_is_one_string(prop->val))
> +		return false;
> +
> +	status = prop->val.val;
> +	return streq(status, "okay") || streq(status, "ok");

I'm pretty sure only "okay" is acceptable according to IEEE1275 - I'm
not sure if the flat tree specs expanded on that.

Which does suggest that a check for valid values of the 'status'
property (not just that it's a string) might be a good idea.

> +}
> +
> +static bool get_reg_value(struct property *reg, unsigned int cell,
> +			  int cell_count, uint64_t *value)
> +{
> +	uint64_t val = 0;
> +	int i;
> +
> +	if (cell_count < 0 || cell_count > 2)

cell_count < 0 is nonsensical, it should be an unsigned parameter.

> +		return false;
> +
> +	for (i = 0; i < cell_count; i++)
> +		val = (val << 32) | propval_cell_n(reg, cell + i);
> +
> +	*value = val;
> +	return true;

I don't love the fact that this is limited to cases with either 1 or 2
cells.  I'd kind of prefer a general 'reg' overlap checker, although I
recognize that's significantly more complex.


> +}
> +
> +static bool ranges_overlap(uint64_t addr_a, uint64_t size_a,
> +			   uint64_t addr_b, uint64_t size_b)
> +{
> +	if (!size_a || !size_b)
> +		return false;

I'm not sure if this makes sense.  If there's a zero-length range in
the middle of another range it's not technically an overlap, but it
still seems a dubious looking situation that we should probably warn
about.

> +	if (addr_a <= addr_b)
> +		return addr_b - addr_a < size_a;
> +
> +	return addr_a - addr_b < size_b;
> +}
> +
> +static void format_range_end(char end_str[20], uint64_t addr, uint64_t size)

Arbitrary open-coded string lengths are not ok.

> +{
> +	uint64_t end = addr + size;
> +
> +	if (end < addr)
> +		snprintf(end_str, 20, "0x1%016" PRIx64, end);
> +	else
> +		snprintf(end_str, 20, "0x%" PRIx64, end);

This seems a very convoluted way of going about things.  You could
instead express memory ranges in inclusive format
(e.g. 0x1000..0x1fff), to avoid the overflow occuring in the first
place.

> +}
> +
> +static void check_reserved_memory_overlap(struct check *c,
> +					  struct dt_info *dti,
> +					  struct node *node)
> +{
> +	struct reserve_range {
> +		struct node *node;
> +		uint64_t addr;
> +		uint64_t size;
> +	} *ranges = NULL;
> +	struct node *child;
> +	unsigned int range_count = 0;
> +	int addr_cells, size_cells, entry_cells;

These should be unsigned.

> +
> +	if (!streq(node->fullpath, "/reserved-memory"))
> +		return;
> +
> +	addr_cells = node_addr_cells(node);
> +	size_cells = node_size_cells(node);
> +	entry_cells = addr_cells + size_cells;

You try to check for overflow below, but *signed* integer overflow is
UB the moment it happens, it's too late to check it below.  But this
should be unsigned anyway.

> +
> +	/* Physical addresses and sizes wider than 64 bits cannot be compared. */
> +	if (addr_cells > 2 || size_cells > 2 || entry_cells <= 0)

Hmm.  If addr_cells + size_cells overflows we have much bigger
problems than this test.  Maybe we should expand sanity checks for
#address-cells and #size-cells and make that a prerequisite for this
check.

> +		return;
> +
> +	for_each_child(node, child) {
> +		struct property *reg;
> +		unsigned int cell;
> +
> +		if (!node_is_available(child))
> +			continue;

Is the 'status' property used in /reserved-memory?  That seems... weird.

> +
> +		reg = get_property(child, "reg");
> +		if (!reg)
> +			continue;
> +
> +		for (cell = 0; cell < reg->val.len / sizeof(cell_t);
> +		     cell += entry_cells) {
> +			uint64_t addr, size;
> +			unsigned int i;
> +
> +			if (!get_reg_value(reg, cell, addr_cells, &addr) ||
> +			    !get_reg_value(reg, cell + addr_cells,
> +					   size_cells, &size))
> +				continue;
> +
> +			for (i = 0; i < range_count; i++) {
> +				char end[20], other_end[20];
> +
> +				if (!ranges_overlap(addr, size, ranges[i].addr,
> +						    ranges[i].size))
> +					continue;
> +
> +				format_range_end(end, addr, size);
> +				format_range_end(other_end, ranges[i].addr,
> +						 ranges[i].size);
> +				FAIL(c, dti, child,
> +				     "reg range [0x%" PRIx64 ", %s) overlaps with %s "
> +				     "[0x%" PRIx64 ", %s)",
> +				     addr, end, ranges[i].node->fullpath,
> +				     ranges[i].addr, other_end);
> +			}
> +
> +			ranges = xrealloc(ranges,
> +					  (range_count + 1) * sizeof(*ranges));
> +			ranges[range_count].node = child;
> +			ranges[range_count].addr = addr;
> +			ranges[range_count].size = size;
> +			range_count++;
> +		}
> +	}
> +
> +	free(ranges);
> +}
> +WARNING(reserved_memory_overlap, check_reserved_memory_overlap, NULL,
> +	&addr_size_cells, &reg_format);
> +
>  static void check_unique_unit_address_common(struct check *c,
>  						struct dt_info *dti,
>  						struct node *node,
> @@ -1945,6 +2075,7 @@ static struct check *check_table[] = {
>  
>  	&avoid_default_addr_size,
>  	&avoid_unnecessary_addr_size,
> +	&reserved_memory_overlap,
>  	&unique_unit_address,
>  	&unique_unit_address_if_enabled,
>  	&obsolete_chosen_interrupt_controller,
> diff --git a/tests/reserved-memory-no-overlap.dts b/tests/reserved-memory-no-overlap.dts
> new file mode 100644
> index 000000000000..0293986ef746
> --- /dev/null
> +++ b/tests/reserved-memory-no-overlap.dts
> @@ -0,0 +1,43 @@
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +

Hrm.  Is 'ranges' really supposed to be there according to the
/reserved-memory binding?  That indicates this is a bridge with
identity mapping, which doesn't make a lot of sense for an
informational node like this.

Also, is there no 'compatible' that should be resent for /reserved-memory?

> +		first@100000000 {
> +			reg = <0x1 0x00000000 0x0 0x00100000>;
> +		};
> +
> +		adjacent@100100000 {
> +			reg = <0x1 0x00100000 0x0 0x00100000>;
> +		};
> +
> +		disabled@100020000 {
> +			reg = <0x1 0x00020000 0x0 0x00100000>;
> +			status = "disabled";
> +		};
> +
> +		reserved@100040000 {
> +			reg = <0x1 0x00040000 0x0 0x00100000>;
> +			status = "reserved";

"reserved" is not a valid value for 'status' according to IEEE 1275
(p188).

> +		};
> +
> +		failed@100060000 {
> +			reg = <0x1 0x00060000 0x0 0x00100000>;
> +			status = "fail";
> +		};
> +
> +		zero-size@100080000 {
> +			reg = <0x1 0x00080000 0x0 0x0>;
> +		};
> +
> +		dynamic {
> +			size = <0x0 0x00100000>;
> +		};
> +	};
> +};
> diff --git a/tests/reserved-memory-overlap.dts b/tests/reserved-memory-overlap.dts
> new file mode 100644
> index 000000000000..fec7c3a33b1f
> --- /dev/null
> +++ b/tests/reserved-memory-overlap.dts
> @@ -0,0 +1,21 @@
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		first@100000000 {
> +			reg = <0x1 0x00000000 0x0 0x00100000>,
> +			      <0x2 0x00000000 0x0 0x00200000>;
> +		};
> +
> +		second@200100000 {
> +			reg = <0x2 0x00100000 0x0 0x00100000>;
> +		};
> +	};
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 842b543059bb..27c3ebfed338 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -746,6 +746,8 @@ dtc_tests () {
>      check_tests "$SRCDIR/reg-ranges-root.dts" reg_format ranges_format
>      check_tests "$SRCDIR/bad-dma-ranges.dts" dma_ranges_format
>      check_tests "$SRCDIR/default-addr-size.dts" avoid_default_addr_size
> +    check_tests "$SRCDIR/reserved-memory-overlap.dts" reserved_memory_overlap
> +    check_tests "$SRCDIR/reserved-memory-no-overlap.dts" -n reserved_memory_overlap
>      check_tests "$SRCDIR/obsolete-chosen-interrupt-controller.dts" obsolete_chosen_interrupt_controller
>      check_tests "$SRCDIR/reg-without-unit-addr.dts" unit_address_vs_reg
>      check_tests "$SRCDIR/unit-addr-without-reg.dts" unit_address_vs_reg
> 
> ---
> base-commit: 7a1e017926004ecff5fce62d62d42ce9f3e00082
> change-id: 20260923-check-reserved-overlap-6c5fc2214ea4
> 
> Best regards,
> --  
> Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> 
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] checks: Warn about overlapping reserved-memory ranges

[Rob Herring]

On Wed, Sep 23, 2026 at 12:51 PM Bjorn Andersson
<bjorn.andersson@oss.qualcomm.com> wrote:
>
> Static /reserved-memory entries associate physical address ranges with
> particular purposes and properties. While not explicitly prohibited,
> overlapping ranges commonly indicate an incorrect address or size and
> may assign the same memory conflicting attributes. Dtc currently
> provides no diagnostic for this type of mistake.
>
> Check every non-empty reg range under available reserved-memory children
> and report overlaps. Add tests covering overlapping and adjacent ranges,
> as well as unavailable, zero-sized, and dynamically allocated regions.

This issue is not just reserved memory, but any address really. 2 h/w
nodes pointing to the same range points to a binding design problem.
That gets a bit more complex to find since it might not be at the same
level within the same subtree. A check for that has been on my todo
for a long time. I did have AI write something for me a while back
that seemed to work. It also functioned as a DT to memory map tool
given you check the entire address space. I'll try to find it...

Rob

Re: [PATCH] checks: Warn about overlapping reserved-memory ranges

[David Gibson]

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

On Thu, Oct 01, 2026 at 11:13:05AM -0500, Rob Herring wrote:
> On Wed, Sep 23, 2026 at 12:51 PM Bjorn Andersson
> <bjorn.andersson@oss.qualcomm.com> wrote:
> >
> > Static /reserved-memory entries associate physical address ranges with
> > particular purposes and properties. While not explicitly prohibited,
> > overlapping ranges commonly indicate an incorrect address or size and
> > may assign the same memory conflicting attributes. Dtc currently
> > provides no diagnostic for this type of mistake.
> >
> > Check every non-empty reg range under available reserved-memory children
> > and report overlaps. Add tests covering overlapping and adjacent ranges,
> > as well as unavailable, zero-sized, and dynamically allocated regions.
> 
> This issue is not just reserved memory, but any address really. 2 h/w
> nodes pointing to the same range points to a binding design problem.
> That gets a bit more complex to find since it might not be at the same
> level within the same subtree.

So.. that's kind of true, but I think we can examine it a bit more
closely.

One way to do the test would be to check for any overlaps between
either 'reg' properties or between the parent address parts of
'ranges' properties.  That works, and is sufficient when there are no
overlaps between either devices, or address ranges delegated to a
daughter bus via a bridge.  I suspect that's true in enough cases to
be useful as is.

Things get more complicated in cases where a large address range is
passed through a bridge - most often the entire bus address space
using an empty 'ranges' - but the range isn't exclusively delegated to
the subordinate bus.  That only makes sense with certain sorts of
bridge, but is also moderately common.

Perhaps this would be workable:

1st draft:
  * Check for overlaps between reg properties within a single parent
  * Check for overlaps in parent range between 'ranges' properties
    within a single parent (ignore empty 'ranges' nodes)
2nd draft:
  * Check for overlaps between 'reg' and 'ranges' parent range, still
    ignoring empty 'ranges' nodes
3rd draft:
  * When checking a single parent node, don't just check direct
    children, but also indirect descendents via nodes with empty
    'ranges'


> A check for that has been on my todo
> for a long time. I did have AI write something for me a while back
> that seemed to work. It also functioned as a DT to memory map tool
> given you check the entire address space. I'll try to find it...
> 
> Rob
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]