[PATCH 6/6] boot: fdt: Handle already reserved memory in boot_fdt_reserve_region()

[Ilias Apalodimas]

On Sun, 8 Dec 2024 at 02:21, Sam Protsenko <semen.protsenko at linaro.org> wrote:
>
> The boot_fdt_add_mem_rsv_regions() function can be called twice, e.g.
> first time during the board init (as a part of LMB init), and then when
> booting the OS with 'booti' command:
>
>     lmb_add_region_flags
>     lmb_reserve_flags
>     boot_fdt_reserve_region
>     boot_fdt_add_mem_rsv_regions
>                ^
>                |
>                +-----------------------+
>                | (1)                   | (2)
>     lmb_reserve_common        image_setup_linux
>     lmb_init                  ...
>     initr_lmb                 do_booti
>     board_init_r              'booti'
>
> That consequently leads to the attempt of reserving the same memory
> areas (described in the 'reserved-memory' dts node) in LMB. The
> lmb_add_region_flags() returns -EEXIST error code in such cases, but
> boot_fdt_reserve_region() handles all negative error codes as a failure
> to reserve fdt memory region, printing corresponding error messages,
> which are essentially harmless, but misleading. For example, this is the
> output of 'booti' command on E850-96 board:
>
>     => booti $loadaddr - $fdtaddr
>     ...
>     ERROR: reserving fdt memory region failed
>            (addr=bab00000 size=5500000 flags=2)
>     ERROR: reserving fdt memory region failed
>            (addr=f0000000 size=200000 flags=4)
>     ...
>     Starting kernel ...
>
> The mentioned false positive error messages are observed starting with
> commit 1d9aa4a283da ("lmb: Fix the allocation of overlapping memory
> areas with !LMB_NONE"), which removes the check for the already added
> memory regions in lmb_add_region_flags(), making it return -1 for
> !LMB_NONE cases. Another commit 827dee587b75 ("fdt: lmb: add reserved
> regions as no-overwrite") changes flags used for reserving memory in
> boot_fdt_add_mem_rsv_regions() from LMB_NONE to LMB_NOOVERWRITE. So
> together with the patch mentioned earlier, it makes
> lmb_add_region_flags() return -1 when called from
> boot_fdt_reserve_region().
>
> Since then, the different patch was implemented, adding the check back
> and returning -EEXIST error code in described cases, which is:
>
>    lmb: Return -EEXIST in lmb_add_region_flags() if region already added
>
> Handle -EEXIST error code as a normal (successful) case in
> lmb_reserve_flags() and don't print any messages.
>
> Fixes: 1d9aa4a283da ("lmb: Fix the allocation of overlapping memory areas with !LMB_NONE")
> Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> ---
>  boot/image-fdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index 3d5b6f9e2dc7..73c43c30684f 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -77,7 +77,7 @@ static void boot_fdt_reserve_region(u64 addr, u64 size, enum lmb_flags flags)
>                 debug("   reserving fdt memory region: addr=%llx size=%llx flags=%x\n",
>                       (unsigned long long)addr,
>                       (unsigned long long)size, flags);
> -       } else {
> +       } else if (ret != -EEXIST) {
>                 puts("ERROR: reserving fdt memory region failed ");
>                 printf("(addr=%llx size=%llx flags=%x)\n",
>                        (unsigned long long)addr,
> --
> 2.39.5
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>