diff mbox series

ARM: bootm: take into account gd->ram_top

Message ID 20200213182950.10744-1-patrick.delaunay@st.com
State New
Headers show
Series ARM: bootm: take into account gd->ram_top | expand

Commit Message

Patrick Delaunay Feb. 13, 2020, 6:29 p.m. UTC 
From: Patrice Chotard <patrice.chotard at st.com>

If gd->ram_top has been tuned using board_get_usable_ram_top(),
it must be taken into account when reserving arch lmb.

Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
Reviewed-by: Patrick DELAUNAY <patrick.delaunay at st.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
---

 arch/arm/lib/bootm.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Simon Glass Feb. 14, 2020, 7:16 p.m. UTC | #1 
Hi Patrick,

On Thu, 13 Feb 2020 at 11:30, Patrick Delaunay <patrick.delaunay at st.com> wrote:
>
> From: Patrice Chotard <patrice.chotard at st.com>
>
> If gd->ram_top has been tuned using board_get_usable_ram_top(),
> it must be taken into account when reserving arch lmb.
>
> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
> Reviewed-by: Patrick DELAUNAY <patrick.delaunay at st.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> ---
>
>  arch/arm/lib/bootm.c | 3 +++
>  1 file changed, 3 insertions(+)

Is this something we can test in test/lib/lmb.c ?

Regards,
Simon
Patrick Delaunay Feb. 18, 2020, 10:32 a.m. UTC | #2 
Hi Simon,

> From: Simon Glass <sjg at chromium.org>
> Sent: vendredi 14 f?vrier 2020 20:17
> 
> Hi Patrick,
> 
> On Thu, 13 Feb 2020 at 11:30, Patrick Delaunay <patrick.delaunay at st.com>
> wrote:
> >
> > From: Patrice Chotard <patrice.chotard at st.com>
> >
> > If gd->ram_top has been tuned using board_get_usable_ram_top(), it
> > must be taken into account when reserving arch lmb.
> >
> > Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
> > Reviewed-by: Patrick DELAUNAY <patrick.delaunay at st.com>
> > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> > ---
> >
> >  arch/arm/lib/bootm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> Is this something we can test in test/lib/lmb.c ?

I check these tests, and for me it is not possible, as I change the ARM part for bootm lib 
(limit the reserved size by u-boot to avoid conflict wxith other reserved memory) 
and not the lmb generic code...

I don't see how to test it in sandbox.

 
> Regards,
> Simon

Regards
Patrick
Tom Rini April 21, 2020, 12:25 p.m. UTC | #3 
On Thu, Feb 13, 2020 at 07:29:50PM +0100, Patrick Delaunay wrote:

> From: Patrice Chotard <patrice.chotard at st.com>
> 
> If gd->ram_top has been tuned using board_get_usable_ram_top(),
> it must be taken into account when reserving arch lmb.
> 
> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
> Reviewed-by: Patrick DELAUNAY <patrick.delaunay at st.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>

Applied to u-boot/master, thanks!
Baruch Siach Aug. 17, 2020, 4:46 a.m. UTC | #4 
Hi Patrick, all,

On Thu, Feb 13 2020, Patrick Delaunay wrote:
> From: Patrice Chotard <patrice.chotard@st.com>

>

> If gd->ram_top has been tuned using board_get_usable_ram_top(),

> it must be taken into account when reserving arch lmb.

>

> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

> Reviewed-by: Patrick DELAUNAY <patrick.delaunay@st.com>

> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

[snip]
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c

> index a135bcfc7b..f4b5ca6de0 100644

> --- a/arch/arm/lib/bootm.c

> +++ b/arch/arm/lib/bootm.c

> @@ -75,6 +75,9 @@ void arch_lmb_reserve(struct lmb *lmb)

>  			gd->bd->bi_dram[bank].size - 1;

>  		if (sp > bank_end)

>  			continue;

> +		if (bank_end > gd->ram_top)

> +			bank_end = gd->ram_top - 1;

> +


This patch (now committed as 8ce1f10cf2b1) breaks kernel boot on Armada
8040 based Clearfog GT-8K with 16GB RAM. See below the console output of
v2020.10-rc2 with a few added prints.

The first memory bank (bi_dram[0]) goes from 0 to 3GB. The rest
(4GB-17GB) is on bi_dram[1] (see a8k_dram_init_banksize()). ram_top is
set to 2GB on
arch/arm/mach-mvebu/arm64-common.c:board_get_usable_ram_top().

Reverting commit 8ce1f10cf2b1 on top of v2020.10-rc2 fixes boot.

Any Idea?

Found /extlinux/extlinux.conf
Retrieving file: /extlinux/extlinux.conf
## Current stack ends at 0x7fb24300
arch_lmb_reserve: bank_end: bfffffff ram_top: 80000000
62 bytes read in 21 ms (2 KiB/s)
1:	linux
Retrieving file: /extlinux/Image
## Current stack ends at 0x7fb23960
arch_lmb_reserve: bank_end: bfffffff ram_top: 80000000
13740544 bytes read in 1266 ms (10.4 MiB/s)
Retrieving file: /extlinux/armada-8040-clearfog-gt-8k.dtb
## Current stack ends at 0x7fb23960
arch_lmb_reserve: bank_end: bfffffff ram_top: 80000000
33368 bytes read in 31 ms (1 MiB/s)
## Current stack ends at 0x7fb23cd0
arch_lmb_reserve: bank_end: bfffffff ram_top: 80000000
## Flattened Device Tree blob at 04f00000
   Booting using the fdt blob at 0x4f00000
   Loading Device Tree to 00000000bfff4000, end 00000000bffff257 ... "Synchronous Abort" handler, esr 0x96000045
elr: 000000000006e1cc lr : 0000000000068fd8 (reloc)
elr: 000000007ffa91cc lr : 000000007ffa3fd8
x0 : ffffffffffffffff x1 : 00000000bfffc258
x2 : 0000000000000000 x3 : ffffffffffff7da7
x4 : 0000000004f08258 x5 : 00000000bfff4000
x6 : 00000000bfff4000 x7 : 000000000000000f
x8 : 000000007fb23bf8 x9 : 0000000000000008
x10: 00000000bffff257 x11: 00000000bffff257
x12: 0000000000000000 x13: fffffffffffff000
x14: 00000000bfff4000 x15: 0000000000000021
x16: 000000007ff7bc38 x17: 0000000000000000
x18: 000000007fb2add0 x19: 00000000bfff4000
x20: 0000000004f00000 x21: 000000000000b258
x22: 0000000058820000 x23: 0000000000000010
x24: 000000007ffe3c40 x25: 000000007fb23cb8
x26: 00000000c0000000 x27: 0000000000000000
x28: 000000007fc3fd50 x29: 000000007fb23bd0

Code: 54000061 aa0603e0 d65f03c0 38606882 (38206822) 
Resetting CPU ...

Thanks,
baruch

>  		lmb_reserve(lmb, sp, bank_end - sp + 1);

>  		break;

>  	}


-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Patrice CHOTARD Aug. 17, 2020, 4:29 p.m. UTC | #5 
Hi Baruch

On 8/17/20 6:46 AM, Baruch Siach wrote:
> Hi Patrick, all,

>

> On Thu, Feb 13 2020, Patrick Delaunay wrote:

>> From: Patrice Chotard <patrice.chotard@st.com>

>>

>> If gd->ram_top has been tuned using board_get_usable_ram_top(),

>> it must be taken into account when reserving arch lmb.

>>

>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

>> Reviewed-by: Patrick DELAUNAY <patrick.delaunay@st.com>

>> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

> [snip]

>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c

>> index a135bcfc7b..f4b5ca6de0 100644

>> --- a/arch/arm/lib/bootm.c

>> +++ b/arch/arm/lib/bootm.c

>> @@ -75,6 +75,9 @@ void arch_lmb_reserve(struct lmb *lmb)

>>  			gd->bd->bi_dram[bank].size - 1;

>>  		if (sp > bank_end)

>>  			continue;

>> +		if (bank_end > gd->ram_top)

>> +			bank_end = gd->ram_top - 1;

>> +

> This patch (now committed as 8ce1f10cf2b1) breaks kernel boot on Armada

> 8040 based Clearfog GT-8K with 16GB RAM. See below the console output of

> v2020.10-rc2 with a few added prints.

>

> The first memory bank (bi_dram[0]) goes from 0 to 3GB. The rest

> (4GB-17GB) is on bi_dram[1] (see a8k_dram_init_banksize()). ram_top is

> set to 2GB on

> arch/arm/mach-mvebu/arm64-common.c:board_get_usable_ram_top().


If i have correctly understood, before this patch, on your platform U-boot was using a memory area over

the ram_top limit set to 2Gb ? which is not correct.

You probably have to rework the platform memory mapping in order to either

_ make all needed memory area fit inside the ram_top limit (2Gb)

or

_ increase the ram_top limit over 2Gb.

It's only assumption as i don't know your platform.

Patrice


>

> Reverting commit 8ce1f10cf2b1 on top of v2020.10-rc2 fixes boot.

>

> Any Idea?

>

> Found /extlinux/extlinux.conf

> Retrieving file: /extlinux/extlinux.conf

> ## Current stack ends at 0x7fb24300

> arch_lmb_reserve: bank_end: bfffffff ram_top: 80000000

> 62 bytes read in 21 ms (2 KiB/s)

> 1:	linux

> Retrieving file: /extlinux/Image

> ## Current stack ends at 0x7fb23960

> arch_lmb_reserve: bank_end: bfffffff ram_top: 80000000

> 13740544 bytes read in 1266 ms (10.4 MiB/s)

> Retrieving file: /extlinux/armada-8040-clearfog-gt-8k.dtb

> ## Current stack ends at 0x7fb23960

> arch_lmb_reserve: bank_end: bfffffff ram_top: 80000000

> 33368 bytes read in 31 ms (1 MiB/s)

> ## Current stack ends at 0x7fb23cd0

> arch_lmb_reserve: bank_end: bfffffff ram_top: 80000000

> ## Flattened Device Tree blob at 04f00000

>    Booting using the fdt blob at 0x4f00000

>    Loading Device Tree to 00000000bfff4000, end 00000000bffff257 ... "Synchronous Abort" handler, esr 0x96000045

> elr: 000000000006e1cc lr : 0000000000068fd8 (reloc)

> elr: 000000007ffa91cc lr : 000000007ffa3fd8

> x0 : ffffffffffffffff x1 : 00000000bfffc258

> x2 : 0000000000000000 x3 : ffffffffffff7da7

> x4 : 0000000004f08258 x5 : 00000000bfff4000

> x6 : 00000000bfff4000 x7 : 000000000000000f

> x8 : 000000007fb23bf8 x9 : 0000000000000008

> x10: 00000000bffff257 x11: 00000000bffff257

> x12: 0000000000000000 x13: fffffffffffff000

> x14: 00000000bfff4000 x15: 0000000000000021

> x16: 000000007ff7bc38 x17: 0000000000000000

> x18: 000000007fb2add0 x19: 00000000bfff4000

> x20: 0000000004f00000 x21: 000000000000b258

> x22: 0000000058820000 x23: 0000000000000010

> x24: 000000007ffe3c40 x25: 000000007fb23cb8

> x26: 00000000c0000000 x27: 0000000000000000

> x28: 000000007fc3fd50 x29: 000000007fb23bd0

>

> Code: 54000061 aa0603e0 d65f03c0 38606882 (38206822) 

> Resetting CPU ...

>

> Thanks,

> baruch

>

>>  		lmb_reserve(lmb, sp, bank_end - sp + 1);

>>  		break;

>>  	}
diff mbox series

Patch

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index a135bcfc7b..f4b5ca6de0 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -75,6 +75,9 @@  void arch_lmb_reserve(struct lmb *lmb)
 			gd->bd->bi_dram[bank].size - 1;
 		if (sp > bank_end)
 			continue;
+		if (bank_end > gd->ram_top)
+			bank_end = gd->ram_top - 1;
+
 		lmb_reserve(lmb, sp, bank_end - sp + 1);
 		break;
 	}