[PATCH] fdt: Fix bootm_low handling

[Laurent Pinchart]

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

-- 
Regards,

Laurent Pinchart