[PATCH 2/5] lmb: replace the lmb_alloc() and lmb_alloc_base() API's

[Ilias Apalodimas]

On Thu May 1, 2025 at 3:02 PM EEST, Sughosh Ganu wrote:
> There currently are two API's for requesting memory from the LMB
> module, lmb_alloc() and lmb_alloc_base(). The function which does the
> actual allocation is the same. Use the earlier introduced API
> lmb_allocate_mem() for both types of allocation requests.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>  arch/arm/mach-apple/board.c      | 27 ++++++++++++++-----
>  arch/arm/mach-snapdragon/board.c | 13 ++++++++-
>  boot/bootm.c                     |  6 +++--
>  boot/image-board.c               | 45 ++++++++++++++++++--------------
>  boot/image-fdt.c                 | 32 +++++++++++++++++------
>  include/lmb.h                    | 22 +++-------------
>  lib/efi_loader/efi_memory.c      | 14 +++++-----
>  lib/lmb.c                        | 30 ++++++++++-----------
>  test/lib/lmb.c                   | 26 ++++++++++++++++++
>  9 files changed, 138 insertions(+), 77 deletions(-)
>
> diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
> index 2644a04a622..f0eab5df1ef 100644
> --- a/arch/arm/mach-apple/board.c
> +++ b/arch/arm/mach-apple/board.c
> @@ -772,6 +772,19 @@ u64 get_page_table_size(void)
>
>  #define KERNEL_COMP_SIZE	SZ_128M
>
> +static phys_addr_t lmb_alloc(phys_size_t size)
> +{
> +	int ret;
> +	phys_addr_t addr;
> +
> +	/* All memory regions allocated with a 2MiB alignment */
> +	ret = lmb_allocate_mem(LMB_MEM_ALLOC_ANY, SZ_2M, &addr, size, LMB_NONE);
> +	if (ret)
> +		return 0;
> +
> +	return addr;
> +}
> +
>  int board_late_init(void)
>  {
>  	u32 status = 0;
> @@ -779,15 +792,15 @@ int board_late_init(void)
>  	/* somewhat based on the Linux Kernel boot requirements:
>  	 * align by 2M and maximal FDT size 2M
>  	 */
> -	status |= env_set_hex("loadaddr", lmb_alloc(SZ_1G, SZ_2M));
> -	status |= env_set_hex("fdt_addr_r", lmb_alloc(SZ_2M, SZ_2M));
> -	status |= env_set_hex("kernel_addr_r", lmb_alloc(SZ_128M, SZ_2M));
> -	status |= env_set_hex("ramdisk_addr_r", lmb_alloc(SZ_1G, SZ_2M));
> +	status |= env_set_hex("loadaddr", lmb_alloc(SZ_1G));

env_set_hex() expects a ulong, which might end up causing problems for some archs, but I don't think
that's a problem of this patchset. It's something that has to be fixed in a the wider codebase

> +	status |= env_set_hex("fdt_addr_r", lmb_alloc(SZ_2M));
> +	status |= env_set_hex("kernel_addr_r", lmb_alloc(SZ_128M));
>  					 rd_len, LMB_NONE);
>  		} else {
>  			if (initrd_high)
> -				*initrd_start =
> -					(ulong)lmb_alloc_base(rd_len,
> -								    0x1000,
> -								    initrd_high,
> -								    LMB_NONE);
> +				err = lmb_allocate_mem(LMB_MEM_ALLOC_MAX,
> +						       0x1000, &initrd_high,
> +						       rd_len, LMB_NONE);
>  			else
> -				*initrd_start = (ulong)lmb_alloc(rd_len,
> -								 0x1000);
> +				err = lmb_allocate_mem(LMB_MEM_ALLOC_ANY,
> +						       0x1000, &initrd_high,
> +						       rd_len, LMB_NONE);

You are now calling the same function, put LMB_MEM_ALLOC_ANY/LMB_MEM_ALLOC_MAX in a variable instead
and make the if smaller

[...]

> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index 6585813de00..b8e1b0f35bb 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -198,15 +198,27 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size)
>  			of_start = (void *)(uintptr_t)addr;
>  			disable_relocation = 1;
>  		} else if (desired_addr) {
> -			addr = lmb_alloc_base(of_len, 0x1000, desired_addr,
> -					      LMB_NONE);
> +			addr = desired_addr;
> +			err = lmb_allocate_mem(LMB_MEM_ALLOC_MAX, 0x1000, &addr,

Is this LMB_MEM_ALLOC_MAX or LMB_MEM_ALLOC_ADDR?

> +					       of_len, LMB_NONE);
> +
> +			if (err) {
> +				puts("Failed using fdt_high value for Device Tree");
> +				goto error;
> +			}
> +
>  			of_start = map_sysmem(addr, of_len);
>  	} else {
> @@ -228,11 +240,15 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size)
>
>  	switch (type) {
> +	case LMB_MEM_ALLOC_ANY:
> +		*addr = LMB_ALLOC_ANYWHERE;
> +		ret = _lmb_alloc_base(size, align, addr, flags);
> +		break;
> +	case LMB_MEM_ALLOC_MAX:
> +		ret = _lmb_alloc_base(size, align, addr, flags);
> +		break;

You can make this a fallthrough
case LMB_MEM_ALLOC_ANY:
    *addr = LMB_ALLOC_ANYWHERE;
case LMB_MEM_ALLOC_MAX:
  ret = _lmb_alloc_base(size, align, addr, flags);
  break;

>  	case LMB_MEM_ALLOC_ADDR:
>  		ret = _lmb_alloc_addr(*addr, size, flags);
>  		break;
> diff --git a/test/lib/lmb.c b/test/lib/lmb.c
> index f80115570e7..8ce19efc854 100644
> --- a/test/lib/lmb.c
> +++ b/test/lib/lmb.c
> @@ -82,6 +82,32 @@ static int lmb_reserve(phys_addr_t addr, phys_size_t size, u32 flags)
>  	return 0;
>  }
>
> +static phys_addr_t lmb_alloc(phys_size_t size, ulong align)
> +{
> +	int err;
> +	phys_addr_t addr;
> +
> +	err = lmb_allocate_mem(LMB_MEM_ALLOC_ANY, align, &addr, size, LMB_NONE);
> +	if (err)
> +		return 0;
> +
> +	return addr;
> +}
> +
> +static phys_addr_t lmb_alloc_base(phys_size_t size, ulong align,
> +				  phys_addr_t max_addr, u32 flags)
> +{
> +	int err;
> +	phys_addr_t addr;
> +
> +	addr = max_addr;
> +	err = lmb_allocate_mem(LMB_MEM_ALLOC_MAX, align, &addr, size, flags);
> +	if (err)
> +		return 0;
> +
> +	return addr;
> +}
> +
>  #define lmb_alloc_addr(addr, size, flags) lmb_reserve(addr, size, flags)
>
>  static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram,

Thanks
/Ilias