[PATCH][RFC] image: fdt: Fix DT relocation handling with multiple DRAM banks with gap

[Simon Glass]

Hi Marek,

On Wed, 21 Jul 2021 at 18:05, Marek Vasut <marex at denx.de> wrote:
>
> The current implementation of boot_relocate_fdt() places DT at the
> highest usable DRAM address, which is calculated as:
>   env_get_bootm_low() + env_get_bootm_mapsize()
> which by default becomes gd->ram_base + gd->ram_size.
>
> Systems like i.MX53 can have multiple DRAM banks with gap between them,
> e.g. have DRAM at 0x70000000-0x8fffffff and 0xb0000000-0xcfffffff , so
> for them the calculated highest DRAM address is 0xafffffff, which is
> exactly in the gap and thus not usable.
>
> Fix this by iterating over all DRAM banks and tracking the remaining
> amount of the total mapping size obtained from env_get_bootm_mapsize().
> Limit the maximum LMB area size to each bank, to avoid using nonexistent
> DRAM.
>
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Tom Rini <trini at konsulko.com>
> ---
>  common/image-fdt.c | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

Should we put this behind a Kconfig option to reduce code size?

>
> diff --git a/common/image-fdt.c b/common/image-fdt.c
> index 327a8c4c395..2b199ffc2b1 100644
> --- a/common/image-fdt.c
> +++ b/common/image-fdt.c
> @@ -157,8 +157,11 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size)
>  {
>         void    *fdt_blob = *of_flat_tree;
>         void    *of_start = NULL;
> +       u64     start, size, usable;
>         char    *fdt_high;
> +       ulong   mapsize, low;
>         ulong   of_len = 0;
> +       int     bank;
>         int     err;
>         int     disable_relocation = 0;
>
> @@ -198,10 +201,39 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size)
>                             (void *)(ulong) lmb_alloc(lmb, of_len, 0x1000);
>                 }
>         } else {
> -               of_start =
> -                   (void *)(ulong) lmb_alloc_base(lmb, of_len, 0x1000,
> -                                                  env_get_bootm_mapsize()
> -                                                  + env_get_bootm_low());
> +               mapsize = env_get_bootm_mapsize();
> +               low = env_get_bootm_low();
> +               of_start = NULL;
> +
> +               for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +                       start = gd->bd->bi_dram[bank].start;
> +                       size = gd->bd->bi_dram[bank].size;
> +
> +                       /* DRAM bank addresses are too low, skip it. */
> +                       if (start + size < low)
> +                               continue;
> +
> +                       usable = min(size, (u64)mapsize);
> +
> +                       /*
> +                        * At least part of this DRAM bank is usable, try
> +                        * using it for LMB allocation.
> +                        */
> +                       of_start =
> +                           (void *)(ulong) lmb_alloc_base(lmb, of_len, 0x1000,
> +                                                          start + usable);
> +                       /* Allocation succeeded, use this block. */
> +                       if (of_start != NULL)
> +                               break;
> +
> +                       /*
> +                        * 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;
> +               }
>         }
>
>         if (of_start == NULL) {
> --
> 2.30.2
>

Regards,
Simon