Hi Folks,

I hope you are all doing well! Is it possible this patch to get some attention?

Thanks!

Regards, Aleksandar

-----Original Message----- From: Tom Rini trini@konsulko.com Sent: Freitag, 30. April 2021 13:53 To: Aleksandar Gerasimovski aleksandar.gerasimovski@hitachi-powergrids.com; Patrick Delaunay patrick.delaunay@foss.st.com; Michal Simek michal.simek@xilinx.com; . Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Cc: u-boot@lists.denx.de Subject: Re: [PATCH] arm: bootm: wrong lmb region reservation when PRAM is used

On Fri, Feb 19, 2021 at 09:46:49PM +0000, Aleksandar Gerasimovski wrote:

This is a draft patch to describe the problem and to initiate a discussion for solution.

PRAM usage is not taken into account when reserving lmb for ARM architecture, this means that predefined PRAM region is reserved by the u-boot and cannot be used by the u-boot users.

In our case this bug leads to non functional ramfs boot, as the PRAM and ram rootfs address ranges are getting reserved by the u-boot.

It is obvious that here PRAM region is ignored, but my question is is this clear to everyone and expected?

Taking  board_f.c as reference, when calculating relocation address PRAM area is taken into account so I would expect that to be case here. Here the intention is to reserve unused space at the end of the effective RAM but PRAM is not part of that.

Possible solution would be to introduce something like get_effective_memsize here e.g powerpc/lib/bootm.c but then also NR_DRAM_BANKS has to be considered?

Signed-off-by: Aleksandar Gerasimovski

aleksandar.gerasimovski@hitachi-powergrids.com

arch/arm/lib/bootm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 11af9e2..4b06d25 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -54,7 +54,7 @@ static ulong get_sp(void)

void arch_lmb_reserve(struct lmb *lmb) {

• ulong sp, bank_end;

• ulong sp, bank_end, pram = 0; int bank;

  /*

@@ -69,6 +69,11 @@ void arch_lmb_reserve(struct lmb *lmb) sp = get_sp(); debug("## Current stack ends at 0x%08lx ", sp);

+#ifdef CONFIG_PRAM

• pram = env_get_ulong("pram", 10, CONFIG_PRAM);
• pram = pram << 10; /* size is in kB */

+#endif

• /* adjust sp by 4K to be safe */ sp -= 4096; for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { @@ -76,8 +81,9

@@ void arch_lmb_reserve(struct lmb *lmb) sp < gd->bd->bi_dram[bank].start) continue; /* Watch out for RAM at end of address space! */

• /* @todo: pram obviously wrong if NR_DRAM_BANKS > 1 */
  

  bank_end = gd->bd->bi_dram[bank].start +

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

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

  if (sp > bank_end) continue; if (bank_end > gd->ram_top)

Adding a few folks who have touched lmb code relatively recently for their thoughts. Thanks!

-- Tom

Hi Patrick,

Many thanks for your reply!

-----Original Message----- From: Patrick DELAUNAY patrick.delaunay@foss.st.com Sent: Freitag, 4. Juni 2021 17:41 To: Aleksandar Gerasimovski aleksandar.gerasimovski@hitachi-powergrids.com; Tom Rini trini@konsulko.com; Michal Simek michal.simek@xilinx.com; . Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Cc: u-boot@lists.denx.de Subject: Re: [PATCH] arm: bootm: wrong lmb region reservation when PRAM is used

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Hi Aleksandar,

On 6/3/21 6:40 AM, Aleksandar Gerasimovski wrote:

Hi Folks,

I hope you are all doing well! Is it possible this patch to get some attention?

Thanks!

Regards, Aleksandar

-----Original Message----- From: Tom Rini trini@konsulko.com Sent: Freitag, 30. April 2021 13:53 To: Aleksandar Gerasimovski aleksandar.gerasimovski@hitachi-powergrids.com; Patrick Delaunay patrick.delaunay@foss.st.com; Michal Simek michal.simek@xilinx.com; . Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Cc: u-boot@lists.denx.de Subject: Re: [PATCH] arm: bootm: wrong lmb region reservation when PRAM is used

On Fri, Feb 19, 2021 at 09:46:49PM +0000, Aleksandar Gerasimovski wrote:

...

This is a draft patch to describe the problem and to initiate a discussion for solution.

PRAM usage is not taken into account when reserving lmb for ARM architecture, this means that predefined PRAM region is reserved by the u-boot and cannot be used by the u-boot users.

In our case this bug leads to non functional ramfs boot, as the PRAM and ram rootfs address ranges are getting reserved by the u-boot.

It is obvious that here PRAM region is ignored, but my question is is this clear to everyone and expected?

Taking board_f.c as reference, when calculating relocation address PRAM area is taken into account so I would expect that to be case here. Here the intention is to reserve unused space at the end of the effective RAM but PRAM is not part of that.

Possible solution would be to introduce something like get_effective_memsize here e.g powerpc/lib/bootm.c but then also NR_DRAM_BANKS has to be considered?

Signed-off-by: Aleksandar Gerasimovski

aleksandar.gerasimovski@hitachi-powergrids.com

arch/arm/lib/bootm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 11af9e2..4b06d25 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -54,7 +54,7 @@ static ulong get_sp(void)

void arch_lmb_reserve(struct lmb *lmb) {

• ulong sp, bank_end;

• ulong sp, bank_end, pram = 0; int bank;

  /*

@@ -69,6 +69,11 @@ void arch_lmb_reserve(struct lmb *lmb) sp = get_sp(); debug("## Current stack ends at 0x%08lx ", sp);

+#ifdef CONFIG_PRAM

• pram = env_get_ulong("pram", 10, CONFIG_PRAM);
• pram = pram << 10; /* size is in kB */

+#endif

• /* adjust sp by 4K to be safe */ sp -= 4096; for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { @@ -76,8

+81,9 @@ void arch_lmb_reserve(struct lmb *lmb) sp < gd->bd->bi_dram[bank].start) continue; /* Watch out for RAM at end of address space! */

•        /* @todo: pram obviously wrong if NR_DRAM_BANKS > 1 */
         bank_end = gd->bd->bi_dram[bank].start +
  

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

•                   gd->bd->bi_dram[bank].size - pram - 1;
         if (sp > bank_end)
                 continue;
         if (bank_end > gd->ram_top)
  

Adding a few folks who have touched lmb code relatively recently for their thoughts. Thanks!

-- Tom

My first felling it is not a good location for the correction => it is not a ARM specific issue

And you already point the NR_DRAM_BANKS issue

Even if this option CONFIG_PRAM seems fewly used...

The expect behavior is described in README

- Protected RAM: CONFIG_PRAM

Define this variable to enable the reservation of "protected RAM", i. e. RAM which is not overwritten by U-Boot. Define CONFIG_PRAM to hold the number of kB you want to reserve for pRAM. You can overwrite this default value by defining an environment variable "pram" to the number of kB you want to reserve. Note that the board info structure will still show the full amount of RAM. If pRAM is reserved, a new environment variable "mem" will automatically be defined to hold the amount of remaining RAM in a form that can be passed as boot argument to Linux, for instance like that:

setenv bootargs ... mem=${mem} saveenv

This way you can tell Linux not to use this memory, either, which results in a memory region that will not be affected by reboots.

*WARNING* If your board configuration uses automatic detection of the RAM size, you must make sure that this memory test is non-destructive. So far, the following board configurations are known to be "pRAM-clean":

IVMS8, IVML24, SPD8xx, HERMES, IP860, RPXlite, LWMON,

FLAGADM

But I don't found the ${mem} usage....

I think you can directly add a lmb reserved memory region in a lmb_reserve_common()

just after boot_fdt_add_mem_rsv_regions()

I the propsal, I use gd->ram_top to avoid issue with NR_DRAM_BANKS > 1

it is alligned with board_f.c::setup_dest_addr()

staticvoidlmb_reserve_common(structlmb*lmb, void*fdt_blob) { arch_lmb_reserve(lmb); board_lmb_reserve(lmb); if(IMAGE_ENABLE_OF_LIBFDT && fdt_blob) boot_fdt_add_mem_rsv_regions(lmb, fdt_blob);

+ if(IS_ENABLED(CONFIG_PRAM)){

+ pram = env_get_ulong("pram", 10, CONFIG_PRAM);

+ lmb_reserve(lmb,gd->ram_top - pram, param);

+ }

}

but I see potential issue here if the lmb init function is called when ENV is not ready.

perhaps it should be done in your board_lmb_reserve() to avoid to impact other board /arch

PS: I think that is done in arch/arm/mach-imx/misc.c => reserved memory after the sp !?

For information, the "reserved-memory" can also be defined in device tree of U-Boot

and kernel (that avoid the CONFIG_PRAM in U-Boot and mem='' in kernel cmd line)

U-Boot need to use the device tree information to avoid the reserved memory for relocation.

it is done in stm32mp platform: using lmb to parse the DT and found the correct usable ram top

arch/arm/mach-stm32mp/dram_init.c::board_get_usable_ram_top()

ulong board_get_usable_ram_top(ulong total_size) { phys_size_t size; phys_addr_t reg; struct lmb lmb;

/* found enough not-reserved memory to relocated U-Boot */ lmb_init(&lmb); lmb_add(&lmb, gd->ram_base, gd->ram_size); boot_fdt_add_mem_rsv_regions(&lmb, (void *)gd->fdt_blob); size = ALIGN(CONFIG_SYS_MALLOC_LEN + total_size, MMU_SECTION_SIZE), reg = lmb_alloc(&lmb, size, MMU_SECTION_SIZE); ....

return reg + size; }

The OP-TEE reserved memory is dynamically added by the secure OS in U-Boot device tree.

Drawback: huge DT parsing cost if data cache is not activated then the function is called

Regards

Patrick

From my view this is indeed an ARM arch issue as ARM arch_lmb_reserve does not consider PRAM usage. Adding design specific board_lmb_reserve will not help as arch_lmb_reserve already reserves that region.

Now how to exactly solve this I still don't know.