diff mbox series

[4.19,48/71] of: Fix reserved-memory overlap detection

Message ID 20201109125022.156835188@linuxfoundation.org
State Superseded
Headers show
Series None | expand

Commit Message

Greg Kroah-Hartman Nov. 9, 2020, 12:55 p.m. UTC 
From: Vincent Whitchurch <vincent.whitchurch@axis.com>

[ Upstream commit ca05f33316559a04867295dd49f85aeedbfd6bfd ]

The reserved-memory overlap detection code fails to detect overlaps if
either of the regions starts at address 0x0.  The code explicitly checks
for and ignores such regions, apparently in order to ignore dynamically
allocated regions which have an address of 0x0 at this point.  These
dynamically allocated regions also have a size of 0x0 at this point, so
fix this by removing the check and sorting the dynamically allocated
regions ahead of any static regions at address 0x0.

For example, there are two overlaps in this case but they are not
currently reported:

	foo@0 {
	        reg = <0x0 0x2000>;
	};

	bar@0 {
	        reg = <0x0 0x1000>;
	};

	baz@1000 {
	        reg = <0x1000 0x1000>;
	};

	quux {
	        size = <0x1000>;
	};

but they are after this patch:

 OF: reserved mem: OVERLAP DETECTED!
 bar@0 (0x00000000--0x00001000) overlaps with foo@0 (0x00000000--0x00002000)
 OF: reserved mem: OVERLAP DETECTED!
 foo@0 (0x00000000--0x00002000) overlaps with baz@1000 (0x00001000--0x00002000)

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
Link: https://lore.kernel.org/r/ded6fd6b47b58741aabdcc6967f73eca6a3f311e.1603273666.git-series.vincent.whitchurch@axis.com
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/of/of_reserved_mem.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Pavel Machek Nov. 11, 2020, 12:53 p.m. UTC | #1 
Hi!

> For example, there are two overlaps in this case but they are not

> currently reported:


...

> but they are after this patch:

> 

>  OF: reserved mem: OVERLAP DETECTED!

>  bar@0 (0x00000000--0x00001000) overlaps with foo@0 (0x00000000--0x00002000)

>  OF: reserved mem: OVERLAP DETECTED!

>  foo@0 (0x00000000--0x00002000) overlaps with baz@1000 (0x00001000--0x00002000)


Is it good idea to push this into 4.19 so early? It does not fix
anything, it just causes warnings.

Such overlap currently exists in 4.19:

arch/arm/boot/dts/s5pv210.dtsi and can not be fixed easily, see:

> > > > >               clocks: clock-controller@e0100000 {

> > > > > +                     compatible = "samsung,s5pv210-clock";

> > > > >                       reg = <0xe0100000 0x10000>;

> > > > ...

> > > > > +             pmu_syscon: syscon@e0108000 {

> > > > > +                     reg = <0xe0108000 0x8000>;

> > > > >               };


Date: Fri, 6 Nov 2020 22:10:38 +0100
From: Krzysztof Kozlowski <krzk@kernel.org>

Subject: Re: [PATCH 4.19 107/191] ARM: dts: s5pv210: move PMU node out of clock controller
				
Best regards,
									Pavel

-- 
http://www.livejournal.com/~pavelmachek
Vincent Whitchurch Nov. 11, 2020, 2:34 p.m. UTC | #2 
On Wed, Nov 11, 2020 at 01:53:59PM +0100, Pavel Machek wrote:
> >  OF: reserved mem: OVERLAP DETECTED!

> >  bar@0 (0x00000000--0x00001000) overlaps with foo@0 (0x00000000--0x00002000)

> >  OF: reserved mem: OVERLAP DETECTED!

> >  foo@0 (0x00000000--0x00002000) overlaps with baz@1000 (0x00001000--0x00002000)

> 

> Is it good idea to push this into 4.19 so early? It does not fix

> anything, it just causes warnings.

> 

> Such overlap currently exists in 4.19:

> 

> arch/arm/boot/dts/s5pv210.dtsi and can not be fixed easily, see:

> 

> > > > > >               clocks: clock-controller@e0100000 {

> > > > > > +                     compatible = "samsung,s5pv210-clock";

> > > > > >                       reg = <0xe0100000 0x10000>;

> > > > > ...

> > > > > > +             pmu_syscon: syscon@e0108000 {

> > > > > > +                     reg = <0xe0108000 0x8000>;

> > > > > >               };


The patch only concerns detection of overlaps in reserved-memory nodes,
and the above does not look like reserved-memory so it will not be
affected.

That being said, I already questioned the need for backporting this
patch:

 https://lore.kernel.org/lkml/20201103111110.lvapcdf4nndunsie@axis.com/
diff mbox series

Patch

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 895c83e0c7b6c..19f95552da4d8 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -218,6 +218,16 @@  static int __init __rmem_cmp(const void *a, const void *b)
 	if (ra->base > rb->base)
 		return 1;
 
+	/*
+	 * Put the dynamic allocations (address == 0, size == 0) before static
+	 * allocations at address 0x0 so that overlap detection works
+	 * correctly.
+	 */
+	if (ra->size < rb->size)
+		return -1;
+	if (ra->size > rb->size)
+		return 1;
+
 	return 0;
 }
 
@@ -235,8 +245,7 @@  static void __init __rmem_check_for_overlap(void)
 
 		this = &reserved_mem[i];
 		next = &reserved_mem[i + 1];
-		if (!(this->base && next->base))
-			continue;
+
 		if (this->base + this->size > next->base) {
 			phys_addr_t this_end, next_end;