Re: [PATCHv3 2/6] lmb: add LMB_FDT for fdt reserved regions

[Randolph Sapp <rs@ti.com>]

On Thu Apr 16, 2026 at 3:39 AM CDT, Ilias Apalodimas wrote:
> Hi Randolph.
>
> On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>>
>> From: Randolph Sapp <rs@ti.com>
>>
>> Add an LMB_FDT bit for fdt reserved regions, so we can reclaim them when
>> parsing a new device tree and properly warn people when a reservation
>> overlaps with an existing allocation.
>
> The LMB has a set of regions that clearly describe memory in an
> abstact way, e.g reserved, no overwrite etc.
> I don't think adding we should open the door for treating reserved
> memory as special. Can't we apply the same fix without adding a new
> description?
>
> Thanks
> /Ilias

I did have an initial implementation using a separate allocation list for fdt
regions in LMB. It works, but boy it's much more messy. All allocations and
frees had to check both the regular allocation list and the fdt allocation list.

I thought about having the fdt helpers build their own local list separate from
LMB, but that's also a little messy in that it means we're either double
tracking regions or tying to reach into the guts of LMB to extract information
about that existing allocation region.

Maybe that's fine considering LMB already reaches out to kick
boot_fdt_add_mem_rsv_regions. Just trying to keep things compartmentalized at
the moment.

I'll revisit the localized reservation list for image-fdt. I'll just setup some
goofy linked list to track allocation start and size for the later free. I
assume we prefer a separate set of lmb_region structs for that list as it's a
little concerning to pass references to an active LMB region outside of the LMB
core.

>>
>> If we don't at least warn the user of these reservation failures,
>> there's a chance that this region could be freed and reallocated for
>> something important later.
>>
>> This useful warning mechanism was broken in:
>> 5a6aa7d5913 ("boot: fdt: Handle already reserved memory in boot_fdt_reserve_region()")
>>
>> Signed-off-by: Randolph Sapp <rs@ti.com>
>> ---
>>  boot/image-fdt.c |  5 ++++-
>>  include/lmb.h    | 14 ++++++++++++++
>>  lib/lmb.c        | 33 +++++++++++++++++++++++++++++----
>>  3 files changed, 47 insertions(+), 5 deletions(-)
>>
>> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
>> index a3a4fb8b558..0f5857f24d2 100644
>> --- a/boot/image-fdt.c
>> +++ b/boot/image-fdt.c
>> @@ -73,6 +73,7 @@ static void boot_fdt_reserve_region(u64 addr, u64 size, u32 flags)
>>  {
>>         long ret;
>>         phys_addr_t rsv_addr;
>> +       flags |= LMB_FDT;
>>
>>         rsv_addr = (phys_addr_t)addr;
>>         ret = lmb_alloc_mem(LMB_MEM_ALLOC_ADDR, 0, &rsv_addr, size, flags);
>> @@ -80,7 +81,7 @@ static void boot_fdt_reserve_region(u64 addr, u64 size, u32 flags)
>>                 debug("   reserving fdt memory region: addr=%llx size=%llx flags=%x\n",
>>                       (unsigned long long)addr,
>>                       (unsigned long long)size, flags);
>> -       } else if (ret != -EEXIST && ret != -EINVAL) {
>> +       } else if (ret != -EINVAL) {
>>                 puts("ERROR: reserving fdt memory region failed ");
>>                 printf("(addr=%llx size=%llx flags=%x)\n",
>>                        (unsigned long long)addr,
>> @@ -108,6 +109,8 @@ void boot_fdt_add_mem_rsv_regions(void *fdt_blob)
>>         if (fdt_check_header(fdt_blob) != 0)
>>                 return;
>>
>> +       lmb_free_fdt_regions();
>> +
>>         /* process memreserve sections */
>>         total = fdt_num_mem_rsv(fdt_blob);
>>         for (i = 0; i < total; i++) {
>> diff --git a/include/lmb.h b/include/lmb.h
>> index 427d701bc30..c6a1fc1ca47 100644
>> --- a/include/lmb.h
>> +++ b/include/lmb.h
>> @@ -51,6 +51,15 @@
>>   */
>>  #define LMB_NONOTIFY BIT(3)
>>
>> +/**
>> + * define LMB_FDT - reclaim this region with lmb_free_fdt_regions()
>> + *
>> + * LMB Memory region attribute flag to indicate that the region will be
>> + * reclaimed with lmb_free_fdt_regions(). This allows device tree reservations
>> + * to be cleaned up and tracked more granularly.
>> + */
>> +#define LMB_FDT BIT(4)
>> +
>>  /**
>>   * enum lmb_mem_type - type of memory allocation request
>>   * @LMB_MEM_ALLOC_ADDR:        request for a particular region of memory
>> @@ -235,6 +244,11 @@ phys_addr_t io_lmb_alloc(struct lmb *io_lmb, phys_size_t size, ulong align);
>>   */
>>  long io_lmb_free(struct lmb *io_lmb, phys_addr_t base, phys_size_t size);
>>
>> +/**
>> + * lmb_free_fdt_regions() - Reclaim all %LMB_FDT tagged reserved regions
>> + */
>> +void lmb_free_fdt_regions(void);
>> +
>>  #endif /* __KERNEL__ */
>>
>>  #endif /* _LINUX_LMB_H */
>> diff --git a/lib/lmb.c b/lib/lmb.c
>> index 8f12c6ad8e5..7ecc548d831 100644
>> --- a/lib/lmb.c
>> +++ b/lib/lmb.c
>> @@ -463,10 +463,10 @@ static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size,
>>
>>  static void lmb_print_region_flags(u32 flags)
>>  {
>> -       const char * const flag_str[] = { "none", "no-map", "no-overwrite",
>> -                                         "no-notify" };
>> -       unsigned int pflags = flags &
>> -                             (LMB_NOMAP | LMB_NOOVERWRITE | LMB_NONOTIFY);
>> +       const char *const flag_str[] = { "none", "no-map", "no-overwrite",
>> +                                        "no-notify", "fdt" };
>> +       unsigned int pflags =
>> +               flags & (LMB_NOMAP | LMB_NOOVERWRITE | LMB_NONOTIFY | LMB_FDT);
>>
>>         if (flags != pflags) {
>>                 printf("invalid %#x\n", flags);
>> @@ -654,6 +654,31 @@ long lmb_free(phys_addr_t base, phys_size_t size, u32 flags)
>>         return lmb_map_update_notify(base, size, LMB_MAP_OP_FREE, flags);
>>  }
>>
>> +void lmb_free_fdt_regions(void)
>> +{
>> +       struct alist *lmb_rgn_lst = &lmb.used_mem;
>> +       struct lmb_region *rgn = lmb_rgn_lst->data;
>> +       long ret;
>> +       int i = 0;
>> +
>> +       while (i < lmb_rgn_lst->count) {
>> +               phys_addr_t base = rgn[i].base;
>> +               phys_size_t size = rgn[i].size;
>> +               u32 flags = rgn[i].flags;
>> +
>> +               if (flags & LMB_FDT) {
>> +                       ret = lmb_free(base, size, flags);
>> +                       if (ret < 0) {
>> +                               printf("Unable to free FDT memory at 0x%08lx\n",
>> +                                      (ulong)base);
>> +                               i++;
>> +                       }
>> +               } else {
>> +                       i++;
>> +               }
>> +       }
>> +}
>> +
>>  static int _lmb_alloc_base(phys_size_t size, ulong align,
>>                            phys_addr_t *addr, u32 flags)
>>  {
>> --
>> 2.53.0
>>