| Message ID | 1447843598-9355-1-git-send-email-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman <mpe@ellerman.id.au> 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 <mitchelh@codeaurora.org> -Mitch
On 5 December 2015 04:07:39 GMT+11:00, Mitchel Humpherys <mitchelh@codeaurora.org> wrote: >On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman ><mpe@ellerman.id.au> 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 <mitchelh@codeaurora.org> Thanks for testing. Rob, can we get this merged for 4.4 please? cheers
On Sat, Dec 5, 2015 at 5:43 AM, Michael Ellerman <michael@ellerman.id.au> wrote: > > > On 5 December 2015 04:07:39 GMT+11:00, Mitchel Humpherys <mitchelh@codeaurora.org> wrote: >>On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman >><mpe@ellerman.id.au> 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 <mitchelh@codeaurora.org> > > 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
On Sun, 2015-12-06 at 14:31 -0600, Rob Herring wrote: > On Sat, Dec 5, 2015 at 5:43 AM, Michael Ellerman <michael@ellerman.id.au> wrote: > > On 5 December 2015 04:07:39 GMT+11:00, Mitchel Humpherys <mitchelh@codeaurora.org> wrote: > > > On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman > > > <mpe@ellerman.id.au> wrote: > > > > 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 <mitchelh@codeaurora.org> > > > > 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
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index be77e75c587d..1a3556a9e9ea 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -206,7 +206,13 @@ static int __init __rmem_cmp(const void *a, const void *b) { const struct reserved_mem *ra = a, *rb = b; - return ra->base - rb->base; + if (ra->base < rb->base) + return -1; + + if (ra->base > rb->base) + return 1; + + return 0; } static void __init __rmem_check_for_overlap(void)
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(-)