Hi Marek,

Thank you for the patch.

On Sat, Mar 02, 2024 at 11:54:02PM +0100, Marek Vasut wrote:

According to README CFG_SYS_BOOTMAPSZ section, in case both "bootm_low" and "bootm_size" variables are defined, "bootm_mapsize" variable is not defined and CFG_SYS_BOOTMAPSZ macro is not defined, all data for the Linux kernel must be between "bootm_low" and "bootm_low" + "bootm_size".

Currently, for systems with DRAM between 0x4000_0000..0x7fff_ffff and with e.g. bootm_low=0x60000000 and bootm_size=0x10000000, the code will attempt to reserve memory from 0x4000_0000..0x4fff_ffff, which is incorrect. This is because "bootm_low" is not taken into consideration correctly.

The last parameter of lmb_alloc_base() is the maximum physical address of the to be reserved LMB area. Currently this is the start of DRAM bank that is considered for LMB area reservation + min(DRAM bank size, bootm_size). In case bootm_low is set to non-zero, this maximum physical address has to be shifted upward, to min(DRAM bank start + size, bootm_low + bootm_size), otherwise the reserved memory may be below bootm_low address.

In case of multiple DRAM banks, the current change reserves top part of the first bank, and reserves the rest of memory in the follow up banks.

Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org

Cc: Geert Uytterhoeven geert@linux-m68k.org Cc: Hans Verkuil hverkuil-cisco@xs4all.nl Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Niklas Söderlund niklas.soderlund@ragnatech.se Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Cc: Wolfram Sang wsa@kernel.org

---

boot/image-fdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 75bdd55f326..5e4aa9de0d2 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -217,14 +217,14 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size) if (start + size < low) continue;

• 	usable = min(size, (u64)mapsize);
  

• 	usable = min(start + size, (u64)(low + mapsize));
  

low and mapsize are ulong, is there a risk this could overflow on 32-bit platforms ?

I would rename usable to usable_end to make it clearer.

	/*
	 * At least part of this DRAM bank is usable, try
	 * using it for LMB allocation.
	 */
	of_start = map_sysmem((ulong)lmb_alloc_base(lmb,

• 		    of_len, 0x1000, start + usable), of_len);
  

• 		    of_len, 0x1000, usable), of_len);
  /* Allocation succeeded, use this block. */
  if (of_start != NULL)
  	break;
  

The code continue with

/* * Reduce the mapping size in the next bank * by the size of attempt in current bank. */ mapsize -= usable - max(start, (u64)low); if (!mapsize) break;

which seems to be correct now, but interestingly was not before :-)