[PATCHv5 2/6] boot_fdt_add_mem_rsv_regions: free old dtb reservations

[Ilias Apalodimas]

Simon,

This looks ok to me, but I haven't been involved with the bootm code
much. Can you taker a quick look?

On Wed, 22 Apr 2026 at 20:10, <rs at ti.com> wrote:
>
> From: Randolph Sapp <rs at ti.com>
>
> Add a free flag and an initial call to free allocations covered by the
> global FDT. This assumes that all calls to boot_fdt_add_mem_rsv_regions
> occur before the transition to the new device tree, thus we can access
> the currently active device tree through the global data pointer.
>
> This allows us to clearly indicate to the user when a device tree
> reservation fails. How we handle this can still use some improvement.
> Right now we'll keep the default behavior and try to boot anyway.
>
> This functionality was broken in:
> 5a6aa7d ("boot: fdt: Handle already reserved memory in boot_fdt_reserve_region()")
>

[...]

either operation.
> + */
> +static void boot_fdt_reserve_region(u64 addr, u64 size, u32 flags, bool free)
>  {

The function name is not accurate anymore. Perhaps call it
boot_fdt_handle_region()?

>         long ret;
>         phys_addr_t rsv_addr;
>
>         rsv_addr = (phys_addr_t)addr;
> -       ret = lmb_alloc_mem(LMB_MEM_ALLOC_ADDR, 0, &rsv_addr, size, flags);
> +       if (free)
> +               ret = lmb_free(rsv_addr, size, flags);
> +       else
> +               ret = lmb_alloc_mem(LMB_MEM_ALLOC_ADDR, 0, &rsv_addr, size,
> +                                   flags);
> +
>         if (!ret) {
> -               debug("   reserving fdt memory region: addr=%llx size=%llx flags=%x\n",
> -                     (unsigned long long)addr,
> +               debug("   %s fdt memory region: addr=%llx size=%llx flags=%x\n",
> +                     free ? "freed" : "reserved", (unsigned long long)addr,
>                       (unsigned long long)size, flags);
> -       } else if (ret != -EEXIST && ret != -EINVAL) {
> -               puts("ERROR: reserving fdt memory region failed ");
> -               printf("(addr=%llx size=%llx flags=%x)\n",
> -                      (unsigned long long)addr,
> -                      (unsigned long long)size, flags);
> +       } else {
> +               printf("ERROR: %s fdt memory region failed (addr=%llx size=%llx flags=%x): %ld\n",
> +                      free ? "freeing" : "reserving", (unsigned long long)addr,
> +                      (unsigned long long)size, flags, ret);
>         }
>  }
>
>  /**
> - * boot_fdt_add_mem_rsv_regions - Mark the memreserve and reserved-memory
> - * sections as unusable
> + * boot_fdt_add_mem_rsv_regions - Handle FDT memreserve and reserved-memory
> + * sections
>   * @fdt_blob: pointer to fdt blob base address
> + * @free: indicate if regions are being freed
>   *
> - * Adds the and reserved-memorymemreserve regions in the dtb to the lmb block.
> - * Adding the memreserve regions prevents u-boot from using them to store the
> - * initrd or the fdt blob.
> + * Adds or removes reserved-memory and memreserve regions in the dtb to the lmb
> + * block. Adding the memreserve regions prevents u-boot from using them to store
> + * the initrd or the fdt blob. This function will attempt to clean the currently
> + * active reservations if a new device tree blob is given.
>   */
> -void boot_fdt_add_mem_rsv_regions(void *fdt_blob)
> +void boot_fdt_add_mem_rsv_regions(void *fdt_blob, bool free)
>  {
>         uint64_t addr, size;
>         int i, total, ret;
> @@ -108,12 +124,16 @@ void boot_fdt_add_mem_rsv_regions(void *fdt_blob)
>         if (fdt_check_header(fdt_blob) != 0)
>                 return;
>
> +       /* Remove old regions */
> +       if (gd->fdt_blob != fdt_blob)
> +               boot_fdt_add_mem_rsv_regions((void *)gd->fdt_blob, true);
> +
>         /* process memreserve sections */
>         total = fdt_num_mem_rsv(fdt_blob);
>         for (i = 0; i < total; i++) {
>                 if (fdt_get_mem_rsv(fdt_blob, i, &addr, &size) != 0)
>                         continue;
> -               boot_fdt_reserve_region(addr, size, LMB_NOOVERWRITE);
> +               boot_fdt_reserve_region(addr, size, LMB_NOOVERWRITE, free);
>         }
>
>         /* process reserved-memory */
> @@ -131,7 +151,8 @@ void boot_fdt_add_mem_rsv_regions(void *fdt_blob)
>                                         flags = LMB_NOMAP;
>                                 addr = res.start;
>                                 size = res.end - res.start + 1;
> -                               boot_fdt_reserve_region(addr, size, flags);
> +                               boot_fdt_reserve_region(addr, size, flags,
> +                                                       free);
>                         }
>
>                         subnode = fdt_next_subnode(fdt_blob, subnode);
> diff --git a/include/image.h b/include/image.h
> index 34efac6056d..68dfa4716ab 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -827,7 +827,7 @@ int boot_get_fdt(void *buf, const char *select, uint arch,
>                  struct bootm_headers *images, char **of_flat_tree,
>                  ulong *of_size);
>
> -void boot_fdt_add_mem_rsv_regions(void *fdt_blob);
> +void boot_fdt_add_mem_rsv_regions(void *fdt_blob, bool free);
>  int boot_relocate_fdt(char **of_flat_tree, ulong *of_size);
>
>  int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start,
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 8f12c6ad8e5..9a8c70b778a 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -581,7 +581,7 @@ static void lmb_reserve_common(void *fdt_blob)
>         lmb_reserve_uboot_region();
>
>         if (CONFIG_IS_ENABLED(OF_LIBFDT) && fdt_blob)
> -               boot_fdt_add_mem_rsv_regions(fdt_blob);
> +               boot_fdt_add_mem_rsv_regions(fdt_blob, false);
>  }
>
>  static __maybe_unused void lmb_reserve_common_spl(void)
> --
> 2.53.0
>

With the change above
Acked-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>