[PATCH v3 17/27] lmb: do away with arch_lmb_reserve()

[Simon Glass]

Hi Sughosh,

On Wed, 21 Aug 2024 at 05:00, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> All of the current definitions of arch_lmb_reserve() are doing the
> same thing -- reserve the region of memory occupied by U-Boot,
> starting from the current stack address to the ram_top. Introduce a
> function lmb_reserve_uboot_region() which does this, and do away with
> the arch_lmb_reserve() function.
>
> Instead of using the current value of stack pointer for starting the
> reserved region, have a fixed value, considering the stack size config
> value.nit
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
> Changes since V2:
> * Remove the definitions of arch_lmb_reserve() and
>   arch_lmb_reserve_generic() functions.
> * Introduce a new function lmb_reserve_uboot_region() and call it from
>   lmb_reserve_common().
>
>  arch/arc/lib/cache.c        | 14 -------
>  arch/arm/lib/stack.c        | 14 -------
>  arch/m68k/lib/bootm.c       | 17 --------
>  arch/microblaze/lib/bootm.c | 14 -------
>  arch/mips/lib/bootm.c       | 15 -------
>  arch/nios2/lib/bootm.c      | 13 ------
>  arch/powerpc/lib/bootm.c    |  9 ----
>  arch/riscv/lib/bootm.c      | 13 ------
>  arch/sh/lib/bootm.c         | 13 ------
>  arch/x86/lib/bootm.c        | 18 --------
>  arch/xtensa/lib/bootm.c     | 13 ------
>  include/lmb.h               |  2 -
>  lib/lmb.c                   | 83 ++++++++++++++++++-------------------
>  13 files changed, 41 insertions(+), 197 deletions(-)

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

question below

[..]

> diff --git a/include/lmb.h b/include/lmb.h
> index e6c6f1cdcd..6e435bd35c 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -108,8 +108,6 @@ void lmb_dump_all(void);
>  void lmb_dump_all_force(void);
>
>  void board_lmb_reserve(void);
> -void arch_lmb_reserve(void);
> -void arch_lmb_reserve_generic(ulong sp, ulong end, ulong align);
>
>  #if CONFIG_IS_ENABLED(UNIT_TEST)

There is no need to do this in the header file, BTW. It will cause a
link error if someone uses it by mistake.

>  struct lmb *lmb_get(void);
> diff --git a/lib/lmb.c b/lib/lmb.c
> index e26e329248..1116b2b868 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
[..]
> @@ -215,10 +179,50 @@ static __maybe_unused int efi_lmb_reserve(void)
>         return 0;
>  }
>
> +static void lmb_reserve_uboot_region(void)
> +{
> +       int bank;
> +       ulong end, bank_end;
> +       phys_addr_t rsv_start;
> +
> +       rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE;
> +       end = gd->ram_top;
> +
> +       /*
> +        * Reserve memory from aligned address below the bottom of U-Boot stack
> +        * until end of RAM area to prevent LMB from overwriting that memory.
> +        */
> +       debug("## Current stack ends at 0x%08lx ", (ulong)rsv_start);
> +
> +       /* adjust sp by 4K to be safe */
> +       rsv_start -= 16384;

Should that be SZ_4K ?

> +       for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +               if (!gd->bd->bi_dram[bank].size ||
> +                   rsv_start < gd->bd->bi_dram[bank].start)
> +                       continue;
> +               /* Watch out for RAM at end of address space! */
> +               bank_end = gd->bd->bi_dram[bank].start +
> +                       gd->bd->bi_dram[bank].size - 1;
> +               if (rsv_start > bank_end)
> +                       continue;
> +               if (bank_end > end)
> +                       bank_end = end - 1;
> +
> +               lmb_reserve_flags(rsv_start, bank_end - rsv_start + 1,
> +                                 LMB_NOOVERWRITE);
> +
> +               if (gd->flags & GD_FLG_SKIP_RELOC)
> +                       lmb_reserve_flags((phys_addr_t)(uintptr_t)_start,
> +                                   gd->mon_len, LMB_NOOVERWRITE);
> +
> +               break;
> +       }
> +}
> +
>  static void lmb_reserve_common(void *fdt_blob)
>  {
> -       arch_lmb_reserve();
>         board_lmb_reserve();
> +       lmb_reserve_uboot_region();
>
>         if (CONFIG_IS_ENABLED(OF_LIBFDT) && fdt_blob)
>                 boot_fdt_add_mem_rsv_regions(fdt_blob);
> @@ -689,11 +693,6 @@ __weak void board_lmb_reserve(void)
>         /* please define platform specific board_lmb_reserve() */
>  }
>
> -__weak void arch_lmb_reserve(void)
> -{
> -       /* please define platform specific arch_lmb_reserve() */
> -}
> -
>  static int lmb_setup(void)
>  {
>         bool ret;
> --
> 2.34.1
>

Regards,
Simon