Re: [PATCH] of: Fix comparison of reserved memory regions

4 messages, 4 authors, 2015-12-06 · open the first message on its own page

Re: [PATCH] of: Fix comparison of reserved memory regions

From: Mitchel Humpherys <hidden>
Date: 2015-12-04 17:07:44

On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman [off-list ref] wrote:
In order to check for overlapping reserved memory regions, we first need
to sort the array of memory regions. This is implemented using sort(),
and a custom comparison function __rmem_cmp().

Unfortunatley __rmem_cmp() doesn't work in all cases. Because the two
base values are phys_addr_t, they may be u64 on some platforms, in which
case subtracting one from the other and then (implicitly) casting to int
does not give us the -ve/0/+ve value we need.

This leads to incorrect reports about overlaps, eg:

  ibm,slw-image@1ffe600000 (0x0000001ffe600000--0x0000001ffe700000) overlaps with
  ibm,firmware-allocs-memory@1000000000 (0x0000001000000000--0x0000001000dc0200)

Fix it by just doing the standard double if and return 0 logic.

Fixes: ae1add247bf8 ("of: Check for overlap in reserved memory regions")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 drivers/of/of_reserved_mem.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Woops, thanks.

Tested-by: Mitchel Humpherys <redacted>

-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] of: Fix comparison of reserved memory regions

From: Michael Ellerman <hidden>
Date: 2015-12-05 11:44:06


On 5 December 2015 04:07:39 GMT+11:00, Mitchel Humpherys [off-list ref] wrote:
On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman
[off-list ref] wrote:
quoted
In order to check for overlapping reserved memory regions, we first
need
quoted
to sort the array of memory regions. This is implemented using
sort(),
quoted
and a custom comparison function __rmem_cmp().

Unfortunatley __rmem_cmp() doesn't work in all cases. Because the two
base values are phys_addr_t, they may be u64 on some platforms, in
which
quoted
case subtracting one from the other and then (implicitly) casting to
int
quoted
does not give us the -ve/0/+ve value we need.

This leads to incorrect reports about overlaps, eg:

  ibm,slw-image@1ffe600000 (0x0000001ffe600000--0x0000001ffe700000)
overlaps with
quoted
  ibm,firmware-allocs-memory@1000000000
(0x0000001000000000--0x0000001000dc0200)
quoted
Fix it by just doing the standard double if and return 0 logic.

Fixes: ae1add247bf8 ("of: Check for overlap in reserved memory
regions")
quoted
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 drivers/of/of_reserved_mem.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Woops, thanks.

Tested-by: Mitchel Humpherys <redacted>
Thanks for testing.

Rob, can we get this merged for 4.4 please?

cheers 

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: [PATCH] of: Fix comparison of reserved memory regions

From: Rob Herring <robh+dt@kernel.org>
Date: 2015-12-06 20:31:37

On Sat, Dec 5, 2015 at 5:43 AM, Michael Ellerman [off-list ref] wrote:

On 5 December 2015 04:07:39 GMT+11:00, Mitchel Humpherys [off-list ref] wrote:
quoted
On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman
[off-list ref] wrote:
quoted
In order to check for overlapping reserved memory regions, we first
need
quoted
to sort the array of memory regions. This is implemented using
sort(),
quoted
and a custom comparison function __rmem_cmp().

Unfortunatley __rmem_cmp() doesn't work in all cases. Because the two
base values are phys_addr_t, they may be u64 on some platforms, in
which
quoted
case subtracting one from the other and then (implicitly) casting to
int
quoted
does not give us the -ve/0/+ve value we need.

This leads to incorrect reports about overlaps, eg:

  ibm,slw-image@1ffe600000 (0x0000001ffe600000--0x0000001ffe700000)
overlaps with
quoted
  ibm,firmware-allocs-memory@1000000000
(0x0000001000000000--0x0000001000dc0200)
quoted
Fix it by just doing the standard double if and return 0 logic.

Fixes: ae1add247bf8 ("of: Check for overlap in reserved memory
regions")
quoted
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 drivers/of/of_reserved_mem.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Woops, thanks.

Tested-by: Mitchel Humpherys <redacted>
Thanks for testing.

Rob, can we get this merged for 4.4 please?
Yes. I meant to last week, but was waiting on getting another issue
sorted out. I should get it to Linus in the next couple of days.

Rob

Re: [PATCH] of: Fix comparison of reserved memory regions

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2015-12-06 23:33:49

On Sun, 2015-12-06 at 14:31 -0600, Rob Herring wrote:
On Sat, Dec 5, 2015 at 5:43 AM, Michael Ellerman [off-list ref] wrote:
quoted
On 5 December 2015 04:07:39 GMT+11:00, Mitchel Humpherys [off-list ref] wrote:
quoted
On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman
[off-list ref] wrote:
quoted
Fix it by just doing the standard double if and return 0 logic.

Fixes: ae1add247bf8 ("of: Check for overlap in reserved memory
regions")

Woops, thanks.

Tested-by: Mitchel Humpherys <redacted>
Thanks for testing.

Rob, can we get this merged for 4.4 please?
Yes. I meant to last week, but was waiting on getting another issue
sorted out. I should get it to Linus in the next couple of days.
Sure thing. No great rush but would be nice for it to be fixed before 4.4
releases.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help