* [PATCH] of: Fix reserved-memory overlap detection
@ 2020-10-20 7:35 Vincent Whitchurch
2020-10-20 13:00 ` Rob Herring
0 siblings, 1 reply; 5+ messages in thread
From: Vincent Whitchurch @ 2020-10-20 7:35 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: kernel, mitchelh, Vincent Whitchurch, devicetree, linux-kernel
The reserved-memory overlap detection code fails to detect overlaps if
either of the regions starts at address 0x0. For some reason the code
explicitly checks for and ignores such regions, but this check looks
invalid. Remove the check and fix this detection.
For example, no overlap is currently reported for this case:
foo@0 {
reg = <0x0000 0x2000>;
};
bar@1000 {
reg = <0x1000 0x1000>;
};
but it is after this patch:
OF: reserved mem: OVERLAP DETECTED!
foo@0 (0x00000000--0x00002000) overlaps with bar@1000 (0x00001000--0x00002000)
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
drivers/of/of_reserved_mem.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 46b9371c8a33..1c5259e3e81f 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -217,8 +217,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;
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] of: Fix reserved-memory overlap detection 2020-10-20 7:35 [PATCH] of: Fix reserved-memory overlap detection Vincent Whitchurch @ 2020-10-20 13:00 ` Rob Herring 2020-10-20 13:46 ` Vincent Whitchurch 0 siblings, 1 reply; 5+ messages in thread From: Rob Herring @ 2020-10-20 13:00 UTC (permalink / raw) To: Vincent Whitchurch Cc: Frank Rowand, kernel, Mitchel Humpherys, devicetree, linux-kernel On Tue, Oct 20, 2020 at 2:36 AM Vincent Whitchurch <vincent.whitchurch@axis.com> wrote: > > The reserved-memory overlap detection code fails to detect overlaps if > either of the regions starts at address 0x0. For some reason the code > explicitly checks for and ignores such regions, but this check looks > invalid. Remove the check and fix this detection. Wouldn't 'base' be 0 for nodes that have a 'size' and no address? The base in those cases isn't set until later when __reserved_mem_alloc_size() is called. > > For example, no overlap is currently reported for this case: > > foo@0 { > reg = <0x0000 0x2000>; > }; > > bar@1000 { > reg = <0x1000 0x1000>; > }; > > but it is after this patch: > > OF: reserved mem: OVERLAP DETECTED! > foo@0 (0x00000000--0x00002000) overlaps with bar@1000 (0x00001000--0x00002000) > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > --- > drivers/of/of_reserved_mem.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c > index 46b9371c8a33..1c5259e3e81f 100644 > --- a/drivers/of/of_reserved_mem.c > +++ b/drivers/of/of_reserved_mem.c > @@ -217,8 +217,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; > > -- > 2.28.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] of: Fix reserved-memory overlap detection 2020-10-20 13:00 ` Rob Herring @ 2020-10-20 13:46 ` Vincent Whitchurch 2020-10-20 14:17 ` Rob Herring 0 siblings, 1 reply; 5+ messages in thread From: Vincent Whitchurch @ 2020-10-20 13:46 UTC (permalink / raw) To: Rob Herring; +Cc: Frank Rowand, kernel, devicetree, linux-kernel On Tue, Oct 20, 2020 at 03:00:14PM +0200, Rob Herring wrote: > On Tue, Oct 20, 2020 at 2:36 AM Vincent Whitchurch > <vincent.whitchurch@axis.com> wrote: > > > > The reserved-memory overlap detection code fails to detect overlaps if > > either of the regions starts at address 0x0. For some reason the code > > explicitly checks for and ignores such regions, but this check looks > > invalid. Remove the check and fix this detection. > > Wouldn't 'base' be 0 for nodes that have a 'size' and no address? The > base in those cases isn't set until later when > __reserved_mem_alloc_size() is called. Ah, yes, I guess that's why the check was there. I see that those entries have both a zero address and a zero size, so this seems to work: diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts index 623246f37448..6627e71c7283 100644 --- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts @@ -81,6 +81,18 @@ vram: vram@4c000000 { reg = <0x4c000000 0x00800000>; no-map; }; + + foo@0 { + reg = <0x0 0x2000>; + }; + + bar@1000 { + reg = <0x0 0x1000>; + }; + + baz { + size = <0x1000>; + }; }; clcd@10020000 { diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 46b9371c8a33..fea9433d942a 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -200,6 +200,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; } @@ -212,13 +222,19 @@ static void __init __rmem_check_for_overlap(void) sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]), __rmem_cmp, NULL); + + for (i = 0; i < reserved_mem_count - 1; i++) { + struct reserved_mem *this = &reserved_mem[i]; + + pr_info("i %d base %x size %x\n", i, this->base, this->size); + } + for (i = 0; i < reserved_mem_count - 1; i++) { struct reserved_mem *this, *next; 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; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] of: Fix reserved-memory overlap detection 2020-10-20 13:46 ` Vincent Whitchurch @ 2020-10-20 14:17 ` Rob Herring 2020-10-21 9:58 ` Vincent Whitchurch 0 siblings, 1 reply; 5+ messages in thread From: Rob Herring @ 2020-10-20 14:17 UTC (permalink / raw) To: Vincent Whitchurch; +Cc: Frank Rowand, kernel, devicetree, linux-kernel On Tue, Oct 20, 2020 at 8:46 AM Vincent Whitchurch <vincent.whitchurch@axis.com> wrote: > > On Tue, Oct 20, 2020 at 03:00:14PM +0200, Rob Herring wrote: > > On Tue, Oct 20, 2020 at 2:36 AM Vincent Whitchurch > > <vincent.whitchurch@axis.com> wrote: > > > > > > The reserved-memory overlap detection code fails to detect overlaps if > > > either of the regions starts at address 0x0. For some reason the code > > > explicitly checks for and ignores such regions, but this check looks > > > invalid. Remove the check and fix this detection. > > > > Wouldn't 'base' be 0 for nodes that have a 'size' and no address? The > > base in those cases isn't set until later when > > __reserved_mem_alloc_size() is called. > > Ah, yes, I guess that's why the check was there. I see that those > entries have both a zero address and a zero size, so this seems to work: Yes, I think it should work. > > diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts > index 623246f37448..6627e71c7283 100644 > --- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts > +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts > @@ -81,6 +81,18 @@ vram: vram@4c000000 { > reg = <0x4c000000 0x00800000>; > no-map; > }; > + > + foo@0 { > + reg = <0x0 0x2000>; > + }; > + > + bar@1000 { > + reg = <0x0 0x1000>; 0x1000 base? > + }; > + > + baz { > + size = <0x1000>; > + }; > }; > > clcd@10020000 { > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c > index 46b9371c8a33..fea9433d942a 100644 > --- a/drivers/of/of_reserved_mem.c > +++ b/drivers/of/of_reserved_mem.c > @@ -200,6 +200,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; > } > > @@ -212,13 +222,19 @@ static void __init __rmem_check_for_overlap(void) > > sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]), > __rmem_cmp, NULL); > + > + for (i = 0; i < reserved_mem_count - 1; i++) { > + struct reserved_mem *this = &reserved_mem[i]; > + > + pr_info("i %d base %x size %x\n", i, this->base, this->size); > + } > + > for (i = 0; i < reserved_mem_count - 1; i++) { > struct reserved_mem *this, *next; > > 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; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] of: Fix reserved-memory overlap detection 2020-10-20 14:17 ` Rob Herring @ 2020-10-21 9:58 ` Vincent Whitchurch 0 siblings, 0 replies; 5+ messages in thread From: Vincent Whitchurch @ 2020-10-21 9:58 UTC (permalink / raw) To: Rob Herring; +Cc: Frank Rowand, kernel, devicetree, linux-kernel On Tue, Oct 20, 2020 at 04:17:27PM +0200, Rob Herring wrote: > On Tue, Oct 20, 2020 at 8:46 AM Vincent Whitchurch > <vincent.whitchurch@axis.com> wrote: > > On Tue, Oct 20, 2020 at 03:00:14PM +0200, Rob Herring wrote: > > > On Tue, Oct 20, 2020 at 2:36 AM Vincent Whitchurch > > > <vincent.whitchurch@axis.com> wrote: > > > > > > > > The reserved-memory overlap detection code fails to detect overlaps if > > > > either of the regions starts at address 0x0. For some reason the code > > > > explicitly checks for and ignores such regions, but this check looks > > > > invalid. Remove the check and fix this detection. > > > > > > Wouldn't 'base' be 0 for nodes that have a 'size' and no address? The > > > base in those cases isn't set until later when > > > __reserved_mem_alloc_size() is called. > > > > Ah, yes, I guess that's why the check was there. I see that those > > entries have both a zero address and a zero size, so this seems to work: > > Yes, I think it should work. Thanks, I've tested it a bit more and sent it out as a v2 now. > > diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts > > index 623246f37448..6627e71c7283 100644 > > --- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts > > +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts > > @@ -81,6 +81,18 @@ vram: vram@4c000000 { > > reg = <0x4c000000 0x00800000>; > > no-map; > > }; > > + > > + foo@0 { > > + reg = <0x0 0x2000>; > > + }; > > + > > + bar@1000 { > > + reg = <0x0 0x1000>; > > 0x1000 base? I've corrected this in the example in the commit message for v2. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-21 9:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-20 7:35 [PATCH] of: Fix reserved-memory overlap detection Vincent Whitchurch 2020-10-20 13:00 ` Rob Herring 2020-10-20 13:46 ` Vincent Whitchurch 2020-10-20 14:17 ` Rob Herring 2020-10-21 9:58 ` Vincent Whitchurch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).