[PATCHv3 0/6] various memory related fixups

[PATCHv3 0/6] various memory related fixups

[rs]

From: Randolph Sapp <rs@ti.com>

Nitpicks and fixes from the discovery thread on adding PocketBeagle2 support
[1]. This does a lot of general setup required for the device, but these
modifications themselves aren't device specific. For those specifically
interested in PocketBeagle2 support and don't care about these details, my
development branch is public [2].

That second patch may provoke some opinions, but honestly if that warning was
still present I wouldn't have spent a week poking holes in both the EFI and LMB
allocations systems. Please let me know if there is a specific usecase that it
breaks though.

[1] https://lore.kernel.org/all/DHHC66BBMD27.YHGIH43C6XBK@ti.com/
[2] https://github.com/StaticRocket/u-boot/tree/feature/pocketbeagle2

v2:
	- Remove additional increment and decrement in lmb_free_fdt_regions
	- Drop the patch to backfill EFI_CONVENTIONAL_MEMORY
	- Adjust the removal loop nitpick patch description
	- Change the reserve memory patch to use new end_addr_sp
v3:
	- Update lmb flags to use the macro documentation for constants
	- Change efi_mem_sort to use list_for_each_entry_safe

Randolph Sapp (6):
  lmb: allocation flags macro documentation
  lmb: add LMB_FDT for fdt reserved regions
  efi_dt_fixup: use fdtdec_get_bool
  efi_selftest_memory: check for duplicates first
  efi_mem_sort: use list_for_each_entry_safe instead
  memory: reserve from start_addr_sp to end_addr_sp

 boot/image-fdt.c                       |  5 +-
 common/board_f.c                       |  9 +++-
 include/asm-generic/global_data.h      |  6 +++
 include/lmb.h                          | 44 +++++++++++++--
 lib/efi_loader/efi_dt_fixup.c          |  3 +-
 lib/efi_loader/efi_memory.c            | 52 +++++++-----------
 lib/efi_selftest/efi_selftest_memory.c | 21 ++++----
 lib/lmb.c                              | 74 +++++++++++++-------------
 8 files changed, 123 insertions(+), 91 deletions(-)

-- 
2.53.0

[PATCHv3 1/6] lmb: allocation flags macro documentation

[rs]

From: Randolph Sapp <rs@ti.com>

Update the allocation flags inline documentation to follow the kernel
object like macro documentation.

Signed-off-by: Randolph Sapp <rs@ti.com>
---
 include/lmb.h | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/include/lmb.h b/include/lmb.h
index 5d5f037ccb9..427d701bc30 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -19,16 +19,36 @@
 #define LMB_ALIST_INITIAL_SIZE	4
 
 /**
- * DOC: Memory region attribute flags.
+ * define LMB_NONE - no special request
  *
- * %LMB_NONE: No special request
- * %LMB_NOMAP: Don't add to MMU configuration
- * %LMB_NOOVERWRITE: The memory region cannot be overwritten/re-reserved
- * %LMB_NONOTIFY: Do not notify other modules of changes to this memory region
+ * LMB Memory region attribute flag to indicate that there is no special
+ * requests for this region. Normally used as a placeholder value.
  */
 #define LMB_NONE 0
+
+/**
+ * define LMB_NOMAP - do not add to MMU configuration
+ *
+ * LMB Memory region attribute flag to indicate that the region will not be
+ * mapped by LMB. Normally used for reserved regions.
+ */
 #define LMB_NOMAP BIT(1)
+
+/**
+ * define LMB_NOOVERWRITE - do not overwrite/re-reserved
+ *
+ * LMB Memory region attribute flag to indicate that the region will not be
+ * overwritten or re-reserved. Normally used for reserved regions.
+ */
 #define LMB_NOOVERWRITE BIT(2)
+
+/**
+ * define LMB_NONOTIFY - do not notify other modules of changes
+ *
+ * LMB Memory region attribute flag to indicate that the region will not notify
+ * downstream allocators (currently just the EFI allocator) of changes to this
+ * region through lmb_map_update_notify().
+ */
 #define LMB_NONOTIFY BIT(3)
 
 /**
-- 
2.53.0

Re: [PATCHv3 1/6] lmb: allocation flags macro documentation

[Ilias Apalodimas]

On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> Update the allocation flags inline documentation to follow the kernel
> object like macro documentation.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>

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

> ---
>  include/lmb.h | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/include/lmb.h b/include/lmb.h
> index 5d5f037ccb9..427d701bc30 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -19,16 +19,36 @@
>  #define LMB_ALIST_INITIAL_SIZE 4
>
>  /**
> - * DOC: Memory region attribute flags.
> + * define LMB_NONE - no special request
>   *
> - * %LMB_NONE: No special request
> - * %LMB_NOMAP: Don't add to MMU configuration
> - * %LMB_NOOVERWRITE: The memory region cannot be overwritten/re-reserved
> - * %LMB_NONOTIFY: Do not notify other modules of changes to this memory region
> + * LMB Memory region attribute flag to indicate that there is no special
> + * requests for this region. Normally used as a placeholder value.
>   */
>  #define LMB_NONE 0
> +
> +/**
> + * define LMB_NOMAP - do not add to MMU configuration
> + *
> + * LMB Memory region attribute flag to indicate that the region will not be
> + * mapped by LMB. Normally used for reserved regions.
> + */
>  #define LMB_NOMAP BIT(1)
> +
> +/**
> + * define LMB_NOOVERWRITE - do not overwrite/re-reserved
> + *
> + * LMB Memory region attribute flag to indicate that the region will not be
> + * overwritten or re-reserved. Normally used for reserved regions.
> + */
>  #define LMB_NOOVERWRITE BIT(2)
> +
> +/**
> + * define LMB_NONOTIFY - do not notify other modules of changes
> + *
> + * LMB Memory region attribute flag to indicate that the region will not notify
> + * downstream allocators (currently just the EFI allocator) of changes to this
> + * region through lmb_map_update_notify().
> + */
>  #define LMB_NONOTIFY BIT(3)
>
>  /**
> --
> 2.53.0
>

Re: [PATCHv3 1/6] lmb: allocation flags macro documentation

[Heinrich Schuchardt]

Am 16. April 2026 10:30:32 MESZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>>
>> From: Randolph Sapp <rs@ti.com>
>>
>> Update the allocation flags inline documentation to follow the kernel
>> object like macro documentation.
>>
>> Signed-off-by: Randolph Sapp <rs@ti.com>
>
>Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
>> ---
>>  include/lmb.h | 30 +++++++++++++++++++++++++-----
>>  1 file changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/lmb.h b/include/lmb.h
>> index 5d5f037ccb9..427d701bc30 100644
>> --- a/include/lmb.h
>> +++ b/include/lmb.h
>> @@ -19,16 +19,36 @@
>>  #define LMB_ALIST_INITIAL_SIZE 4
>>
>>  /**
>> - * DOC: Memory region attribute flags.
>> + * define LMB_NONE - no special request
>>   *
>> - * %LMB_NONE: No special request
>> - * %LMB_NOMAP: Don't add to MMU configuration
>> - * %LMB_NOOVERWRITE: The memory region cannot be overwritten/re-reserved
>> - * %LMB_NONOTIFY: Do not notify other modules of changes to this memory region
>> + * LMB Memory region attribute flag to indicate that there is no special
>> + * requests for this region. Normally used as a placeholder value.
>>   */
>>  #define LMB_NONE 0
>> +
>> +/**
>> + * define LMB_NOMAP - do not add to MMU configuration
>> + *
>> + * LMB Memory region attribute flag to indicate that the region will not be
>> + * mapped by LMB. Normally used for reserved regions.
>> + */
>>  #define LMB_NOMAP BIT(1)
>> +
>> +/**
>> + * define LMB_NOOVERWRITE - do not overwrite/re-reserved

%s/reserved/reserve/

Best regards

Heinrich

>> + *
>> + * LMB Memory region attribute flag to indicate that the region will not be
>> + * overwritten or re-reserved. Normally used for reserved regions.
>> + */
>>  #define LMB_NOOVERWRITE BIT(2)
>> +
>> +/**
>> + * define LMB_NONOTIFY - do not notify other modules of changes
>> + *
>> + * LMB Memory region attribute flag to indicate that the region will not notify
>> + * downstream allocators (currently just the EFI allocator) of changes to this
>> + * region through lmb_map_update_notify().
>> + */
>>  #define LMB_NONOTIFY BIT(3)
>>
>>  /**
>> --
>> 2.53.0
>>

Re: [PATCHv3 1/6] lmb: allocation flags macro documentation

[Simon Glass]

Hi Randolph,

On 2026-04-13T20:35:52, Randolph Sapp <rs@ti.com> wrote:
> lmb: allocation flags macro documentation
>
> Update the allocation flags inline documentation to follow the kernel
> object like macro documentation.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> include/lmb.h | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)

> diff --git a/include/lmb.h b/include/lmb.h
> @@ -19,16 +19,36 @@
> + * LMB Memory region attribute flag to indicate that there is no special
> + * requests for this region. Normally used as a placeholder value.

there are no special requests

Regards,
Simon

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

[rs]

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.

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

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

[Ilias Apalodimas]

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
>
> 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
>

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

[Randolph Sapp]

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
>>

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

[Ilias Apalodimas]

On Thu, 16 Apr 2026 at 22:23, Randolph Sapp <rs@ti.com> wrote:
>
> 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.

Can you explain a bit more when the problem happens? Is it because
boot_fdt_add_mem_rsv_regions() get called twice in some boot flows? Or
is there another reason?

Thanks
/Ilias
>
> >>
> >> 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
> >>
>

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

[Randolph Sapp]

On Thu Apr 16, 2026 at 2:54 PM CDT, Ilias Apalodimas wrote:
> On Thu, 16 Apr 2026 at 22:23, Randolph Sapp <rs@ti.com> wrote:
>>
>> 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.
>
> Can you explain a bit more when the problem happens? Is it because
> boot_fdt_add_mem_rsv_regions() get called twice in some boot flows? Or
> is there another reason?
>
> Thanks
> /Ilias

The initial comment explicitly calls out that as the reason they disabled the
warning, but it should occur any time a device tree is loaded. Internal or
external.

That also lead me to believe if one were to load multiple external device trees
that you could theoretically trip an OOM with a bunch of non-overlapping
reserved regions. I have yet to test that, but culling old allocations seemed
practical enough without that PoC.

>>
>> >>
>> >> 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
>> >>
>>

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

[Ilias Apalodimas]

On Thu, 16 Apr 2026 at 23:32, Randolph Sapp <rs@ti.com> wrote:
>
> On Thu Apr 16, 2026 at 2:54 PM CDT, Ilias Apalodimas wrote:
> > On Thu, 16 Apr 2026 at 22:23, Randolph Sapp <rs@ti.com> wrote:
> >>
> >> 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.
> >
> > Can you explain a bit more when the problem happens? Is it because
> > boot_fdt_add_mem_rsv_regions() get called twice in some boot flows? Or
> > is there another reason?
> >
> > Thanks
> > /Ilias
>
> The initial comment explicitly calls out that as the reason they disabled the
> warning, but it should occur any time a device tree is loaded. Internal or
> external.
>

Yes, but that's requesting to reserve specific address(es) which is
always the same address ranges unless the DTB changes.  In which case
you just have to find and free the reserved-ranges. Keeping in mind
that's rare, you can re-parse the DT with minimal overhead and you
dont have to keep any metadat stored in LMB or anywhere else. and the
LMB subsystem is not the place to do that. The EEXIST basically means
"I've already done that" here so I am trying to understand what can
trigger this problem.

> That also lead me to believe if one were to load multiple external device trees
> that you could theoretically trip an OOM with a bunch of non-overlapping
> reserved regions. I have yet to test that, but culling old allocations seemed
> practical enough without that PoC.

Yea but that's a user error. You can also load a bigger binary that
your entire DRAM and trigger an OOM

Thanks
/Ilias
>
> >>
> >> >>
> >> >> 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
> >> >>
> >>
>

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

[Randolph Sapp]

On Fri Apr 17, 2026 at 3:12 AM CDT, Ilias Apalodimas wrote:
> On Thu, 16 Apr 2026 at 23:32, Randolph Sapp <rs@ti.com> wrote:
>>
>> On Thu Apr 16, 2026 at 2:54 PM CDT, Ilias Apalodimas wrote:
>> > On Thu, 16 Apr 2026 at 22:23, Randolph Sapp <rs@ti.com> wrote:
>> >>
>> >> 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.
>> >
>> > Can you explain a bit more when the problem happens? Is it because
>> > boot_fdt_add_mem_rsv_regions() get called twice in some boot flows? Or
>> > is there another reason?
>> >
>> > Thanks
>> > /Ilias
>>
>> The initial comment explicitly calls out that as the reason they disabled the
>> warning, but it should occur any time a device tree is loaded. Internal or
>> external.
>>
>
> Yes, but that's requesting to reserve specific address(es) which is
> always the same address ranges unless the DTB changes.  In which case
> you just have to find and free the reserved-ranges. Keeping in mind
> that's rare, you can re-parse the DT with minimal overhead and you
> dont have to keep any metadat stored in LMB or anywhere else. and the
> LMB subsystem is not the place to do that. The EEXIST basically means
> "I've already done that" here so I am trying to understand what can
> trigger this problem.
>
>> That also lead me to believe if one were to load multiple external device trees
>> that you could theoretically trip an OOM with a bunch of non-overlapping
>> reserved regions. I have yet to test that, but culling old allocations seemed
>> practical enough without that PoC.
>
> Yea but that's a user error. You can also load a bigger binary that
> your entire DRAM and trigger an OOM
>
> Thanks
> /Ilias

The explicit case were I really wish we had this warning was when the u-boot
stack was creeping into a FDT reserved region. The FDT reservation naturally
failed with EEXIST. That error is overloaded as is. We have to track the
reservations we successfully create in order to clean them up properly.

>>
>> >>
>> >> >>
>> >> >> 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
>> >> >>
>> >>
>>

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

[Simon Glass]

Hi Randolph,

On Tue, 14 Apr 2026 at 08: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.
>
> 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(-)
>

With bootstd we maintain a list of images attached to each bootflow,
including the address when loaded. Could that provide a solution here?

Regards,
Simon

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

[Randolph Sapp]

On Thu Apr 16, 2026 at 4:02 PM CDT, Simon Glass wrote:
> Hi Randolph,
>
> On Tue, 14 Apr 2026 at 08: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.
>>
>> 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(-)
>>
>
> With bootstd we maintain a list of images attached to each bootflow,
> including the address when loaded. Could that provide a solution here?
>
> Regards,
> Simon

Hey Simon, you may have to elaborate on that a little more. Are you suggesting
we treat FDT reserved regions as dummy bootstd binary entries? That might work
if it counts as an LMB reservation and we can dynamically update it if the FDT
is reloaded/changed. I haven't looked into that too much yet.

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

[Simon Glass]

Hi Randolph,

On Fri, 17 Apr 2026 at 09:12, Randolph Sapp <rs@ti.com> wrote:
>
> On Thu Apr 16, 2026 at 4:02 PM CDT, Simon Glass wrote:
> > Hi Randolph,
> >
> > On Tue, 14 Apr 2026 at 08: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.
> >>
> >> 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(-)
> >>
> >
> > With bootstd we maintain a list of images attached to each bootflow,
> > including the address when loaded. Could that provide a solution here?
> >
> > Regards,
> > Simon
>
> Hey Simon, you may have to elaborate on that a little more. Are you suggesting
> we treat FDT reserved regions as dummy bootstd binary entries? That might work
> if it counts as an LMB reservation and we can dynamically update it if the FDT
> is reloaded/changed. I haven't looked into that too much yet.

Not so much dummies, I mean real bootflows. I am not sure if you are
using bootstd, though?

Basically, once you have a bootflow there is a list of images attached
- see struct bootflow_img

Each image has a type so you can see if it is an FDT.

This is just an idea though...I'm not sure if you are even using bootstd.

Regards,
Simon

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

[Randolph Sapp]

On Thu Apr 16, 2026 at 4:20 PM CDT, Simon Glass wrote:
> Hi Randolph,
>
> On Fri, 17 Apr 2026 at 09:12, Randolph Sapp <rs@ti.com> wrote:
>>
>> On Thu Apr 16, 2026 at 4:02 PM CDT, Simon Glass wrote:
>> > Hi Randolph,
>> >
>> > On Tue, 14 Apr 2026 at 08: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.
>> >>
>> >> 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(-)
>> >>
>> >
>> > With bootstd we maintain a list of images attached to each bootflow,
>> > including the address when loaded. Could that provide a solution here?
>> >
>> > Regards,
>> > Simon
>>
>> Hey Simon, you may have to elaborate on that a little more. Are you suggesting
>> we treat FDT reserved regions as dummy bootstd binary entries? That might work
>> if it counts as an LMB reservation and we can dynamically update it if the FDT
>> is reloaded/changed. I haven't looked into that too much yet.
>
> Not so much dummies, I mean real bootflows. I am not sure if you are
> using bootstd, though?
>
> Basically, once you have a bootflow there is a list of images attached
> - see struct bootflow_img
>
> Each image has a type so you can see if it is an FDT.
>
> This is just an idea though...I'm not sure if you are even using bootstd.
>
> Regards,
> Simon

Oh, the issue isn't in the region reserved to load the FDT itself, it's the
regions the FDT reserves for other components. Specifically the
"reserved-memory" nodes and other things like that.

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

[Simon Glass]

Hi Randolph,

On Fri, 17 Apr 2026 at 09:30, Randolph Sapp <rs@ti.com> wrote:
>
> On Thu Apr 16, 2026 at 4:20 PM CDT, Simon Glass wrote:
> > Hi Randolph,
> >
> > On Fri, 17 Apr 2026 at 09:12, Randolph Sapp <rs@ti.com> wrote:
> >>
> >> On Thu Apr 16, 2026 at 4:02 PM CDT, Simon Glass wrote:
> >> > Hi Randolph,
> >> >
> >> > On Tue, 14 Apr 2026 at 08: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.
> >> >>
> >> >> 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(-)
> >> >>
> >> >
> >> > With bootstd we maintain a list of images attached to each bootflow,
> >> > including the address when loaded. Could that provide a solution here?
> >> >
> >> > Regards,
> >> > Simon
> >>
> >> Hey Simon, you may have to elaborate on that a little more. Are you suggesting
> >> we treat FDT reserved regions as dummy bootstd binary entries? That might work
> >> if it counts as an LMB reservation and we can dynamically update it if the FDT
> >> is reloaded/changed. I haven't looked into that too much yet.
> >
> > Not so much dummies, I mean real bootflows. I am not sure if you are
> > using bootstd, though?
> >
> > Basically, once you have a bootflow there is a list of images attached
> > - see struct bootflow_img
> >
> > Each image has a type so you can see if it is an FDT.
> >
> > This is just an idea though...I'm not sure if you are even using bootstd.
> >
> > Regards,
> > Simon
>
> Oh, the issue isn't in the region reserved to load the FDT itself, it's the
> regions the FDT reserves for other components. Specifically the
> "reserved-memory" nodes and other things like that.

OK, got it.

Regards,
Simon

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

[Simon Glass]

Hi Randolph,

On 2026-04-13T20:35:52, Randolph Sapp <rs@ti.com> wrote:
> lmb: add LMB_FDT for fdt reserved regions
>
> 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.
>
> 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/lib/lmb.c b/lib/lmb.c
> @@ -654,6 +654,31 @@ long lmb_free(phys_addr_t base, phys_size_t size, u32 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;

Could you move 'ret' into the block which needs it?

> diff --git a/lib/lmb.c b/lib/lmb.c
> @@ -654,6 +654,31 @@ long lmb_free(phys_addr_t base, phys_size_t size, u32 flags)
> +void lmb_free_fdt_regions(void)
> +{
> +     struct alist *lmb_rgn_lst = &lmb.used_mem;
> +     struct lmb_region *rgn = lmb_rgn_lst->data;

Since you are adding a new exported function, please can you add a
test for it in test/lib/lmb.c ?

Regards,
Simon

[PATCHv3 3/6] efi_dt_fixup: use fdtdec_get_bool

[rs]

From: Randolph Sapp <rs@ti.com>

Use the more straightforward fdtdec_get_bool instead of fdt_getprop and
a return code check.

Signed-off-by: Randolph Sapp <rs@ti.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Anshul Dalal <anshuld@ti.com>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_dt_fixup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c
index 544e1aa9808..333711b9957 100644
--- a/lib/efi_loader/efi_dt_fixup.c
+++ b/lib/efi_loader/efi_dt_fixup.c
@@ -123,8 +123,7 @@ void efi_carve_out_dt_rsv(void *fdt)
 			    fdtdec_get_is_enabled(fdt, subnode)) {
 				bool nomap;
 
-				nomap = !!fdt_getprop(fdt, subnode, "no-map",
-						      NULL);
+				nomap = fdtdec_get_bool(fdt, subnode, "no-map");
 				efi_reserve_memory(fdt_addr, fdt_size, nomap);
 			}
 			subnode = fdt_next_subnode(fdt, subnode);
-- 
2.53.0

Re: [PATCHv3 3/6] efi_dt_fixup: use fdtdec_get_bool

[Simon Glass]

On 2026-04-13T20:35:52, Randolph Sapp <rs@ti.com> wrote:
> efi_dt_fixup: use fdtdec_get_bool
>
> Use the more straightforward fdtdec_get_bool instead of fdt_getprop and
> a return code check.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Reviewed-by: Anshul Dalal <anshuld@ti.com>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> lib/efi_loader/efi_dt_fixup.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

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

[PATCHv3 4/6] efi_selftest_memory: check for duplicates first

[rs]

From: Randolph Sapp <rs@ti.com>

Check for duplicate memory mappings before reporting any incorrect
attributes. Could be that second allocation has the correct type while
the first doesn't. Knowing there is a duplicate in this scenario is
more helpful than just reporting the first mismatch.

Signed-off-by: Randolph Sapp <rs@ti.com>
---
 lib/efi_selftest/efi_selftest_memory.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_memory.c b/lib/efi_selftest/efi_selftest_memory.c
index 4d32a280061..7320964c129 100644
--- a/lib/efi_selftest/efi_selftest_memory.c
+++ b/lib/efi_selftest/efi_selftest_memory.c
@@ -60,7 +60,7 @@ static int find_in_memory_map(efi_uintn_t map_size,
 			      u64 addr, int memory_type)
 {
 	efi_uintn_t i;
-	bool found = false;
+	struct efi_mem_desc *match = NULL;
 
 	for (i = 0; map_size; ++i, map_size -= desc_size) {
 		struct efi_mem_desc *entry = &memory_map[i];
@@ -72,24 +72,23 @@ static int find_in_memory_map(efi_uintn_t map_size,
 
 		if (addr >= entry->physical_start &&
 		    addr < entry->physical_start +
-			    (entry->num_pages << EFI_PAGE_SHIFT)) {
-			if (found) {
+				    (entry->num_pages << EFI_PAGE_SHIFT)) {
+			if (match) {
 				efi_st_error("Duplicate memory map entry\n");
 				return EFI_ST_FAILURE;
 			}
-			found = true;
-			if (memory_type != entry->type) {
-				efi_st_error
-					("Wrong memory type %d, expected %d\n",
-					 entry->type, memory_type);
-				return EFI_ST_FAILURE;
-			}
+			match = entry;
 		}
 	}
-	if (!found) {
+	if (!match) {
 		efi_st_error("Missing memory map entry\n");
 		return EFI_ST_FAILURE;
 	}
+	if (memory_type != match->type) {
+		efi_st_error("Wrong memory type %d, expected %d\n", match->type,
+			     memory_type);
+		return EFI_ST_FAILURE;
+	}
 	return EFI_ST_SUCCESS;
 }
 
-- 
2.53.0

Re: [PATCHv3 4/6] efi_selftest_memory: check for duplicates first

[Ilias Apalodimas]

On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> Check for duplicate memory mappings before reporting any incorrect
> attributes. Could be that second allocation has the correct type while
> the first doesn't. Knowing there is a duplicate in this scenario is
> more helpful than just reporting the first mismatch.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>

[...]

>                         }
> -                       found = true;
> -                       if (memory_type != entry->type) {
> -                               efi_st_error
> -                                       ("Wrong memory type %d, expected %d\n",
> -                                        entry->type, memory_type);
> -                               return EFI_ST_FAILURE;
> -                       }
> +                       match = entry;
>                 }
>         }
> -       if (!found) {
> +       if (!match) {
>                 efi_st_error("Missing memory map entry\n");
>                 return EFI_ST_FAILURE;
>         }
> +       if (memory_type != match->type) {
> +               efi_st_error("Wrong memory type %d, expected %d\n", match->type,
> +                            memory_type);
> +               return EFI_ST_FAILURE;

This check is now outside the loop and only checks for the last entry.
If you wan't to split the fucntionality, don't we need a loop over all
memory areas and the type?

Thanks
/Ilias


> +       }
>         return EFI_ST_SUCCESS;
>  }
>
> --
> 2.53.0
>

Re: [PATCHv3 4/6] efi_selftest_memory: check for duplicates first

[Randolph Sapp]

On Thu Apr 16, 2026 at 3:55 AM CDT, Ilias Apalodimas wrote:
> On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>>
>> From: Randolph Sapp <rs@ti.com>
>>
>> Check for duplicate memory mappings before reporting any incorrect
>> attributes. Could be that second allocation has the correct type while
>> the first doesn't. Knowing there is a duplicate in this scenario is
>> more helpful than just reporting the first mismatch.
>>
>> Signed-off-by: Randolph Sapp <rs@ti.com>
>
> [...]
>
>>                         }
>> -                       found = true;
>> -                       if (memory_type != entry->type) {
>> -                               efi_st_error
>> -                                       ("Wrong memory type %d, expected %d\n",
>> -                                        entry->type, memory_type);
>> -                               return EFI_ST_FAILURE;
>> -                       }
>> +                       match = entry;
>>                 }
>>         }
>> -       if (!found) {
>> +       if (!match) {
>>                 efi_st_error("Missing memory map entry\n");
>>                 return EFI_ST_FAILURE;
>>         }
>> +       if (memory_type != match->type) {
>> +               efi_st_error("Wrong memory type %d, expected %d\n", match->type,
>> +                            memory_type);
>> +               return EFI_ST_FAILURE;
>
> This check is now outside the loop and only checks for the last entry.
> If you wan't to split the fucntionality, don't we need a loop over all
> memory areas and the type?
>
> Thanks
> /Ilias

Not necessarily. At the end of the day we can only really raise one exception
anyway. I just think informing the user about a duplicate should take priority
above mismatched attributes. It hints at a bigger issue.

>> +       }
>>         return EFI_ST_SUCCESS;
>>  }
>>
>> --
>> 2.53.0
>>

Re: [PATCHv3 4/6] efi_selftest_memory: check for duplicates first

[Ilias Apalodimas]

On Thu, 16 Apr 2026 at 23:26, Randolph Sapp <rs@ti.com> wrote:
>
> On Thu Apr 16, 2026 at 3:55 AM CDT, Ilias Apalodimas wrote:
> > On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
> >>
> >> From: Randolph Sapp <rs@ti.com>
> >>
> >> Check for duplicate memory mappings before reporting any incorrect
> >> attributes. Could be that second allocation has the correct type while
> >> the first doesn't. Knowing there is a duplicate in this scenario is
> >> more helpful than just reporting the first mismatch.
> >>
> >> Signed-off-by: Randolph Sapp <rs@ti.com>
> >
> > [...]
> >
> >>                         }
> >> -                       found = true;
> >> -                       if (memory_type != entry->type) {
> >> -                               efi_st_error
> >> -                                       ("Wrong memory type %d, expected %d\n",
> >> -                                        entry->type, memory_type);
> >> -                               return EFI_ST_FAILURE;
> >> -                       }
> >> +                       match = entry;
> >>                 }
> >>         }
> >> -       if (!found) {
> >> +       if (!match) {
> >>                 efi_st_error("Missing memory map entry\n");
> >>                 return EFI_ST_FAILURE;
> >>         }
> >> +       if (memory_type != match->type) {
> >> +               efi_st_error("Wrong memory type %d, expected %d\n", match->type,
> >> +                            memory_type);
> >> +               return EFI_ST_FAILURE;
> >
> > This check is now outside the loop and only checks for the last entry.
> > If you wan't to split the fucntionality, don't we need a loop over all
> > memory areas and the type?
> >
> > Thanks
> > /Ilias
>
> Not necessarily. At the end of the day we can only really raise one exception
> anyway. I just think informing the user about a duplicate should take priority
> above mismatched attributes. It hints at a bigger issue.

I dont mind about the priority. We can swap that over. But with this
patch we will check for less problems than we currently do.

Thanks
/Ilias
>
> >> +       }
> >>         return EFI_ST_SUCCESS;
> >>  }
> >>
> >> --
> >> 2.53.0
> >>
>

Re: [PATCHv3 4/6] efi_selftest_memory: check for duplicates first

[Randolph Sapp]

On Fri Apr 17, 2026 at 3:17 AM CDT, Ilias Apalodimas wrote:
> On Thu, 16 Apr 2026 at 23:26, Randolph Sapp <rs@ti.com> wrote:
>>
>> On Thu Apr 16, 2026 at 3:55 AM CDT, Ilias Apalodimas wrote:
>> > On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>> >>
>> >> From: Randolph Sapp <rs@ti.com>
>> >>
>> >> Check for duplicate memory mappings before reporting any incorrect
>> >> attributes. Could be that second allocation has the correct type while
>> >> the first doesn't. Knowing there is a duplicate in this scenario is
>> >> more helpful than just reporting the first mismatch.
>> >>
>> >> Signed-off-by: Randolph Sapp <rs@ti.com>
>> >
>> > [...]
>> >
>> >>                         }
>> >> -                       found = true;
>> >> -                       if (memory_type != entry->type) {
>> >> -                               efi_st_error
>> >> -                                       ("Wrong memory type %d, expected %d\n",
>> >> -                                        entry->type, memory_type);
>> >> -                               return EFI_ST_FAILURE;
>> >> -                       }
>> >> +                       match = entry;
>> >>                 }
>> >>         }
>> >> -       if (!found) {
>> >> +       if (!match) {
>> >>                 efi_st_error("Missing memory map entry\n");
>> >>                 return EFI_ST_FAILURE;
>> >>         }
>> >> +       if (memory_type != match->type) {
>> >> +               efi_st_error("Wrong memory type %d, expected %d\n", match->type,
>> >> +                            memory_type);
>> >> +               return EFI_ST_FAILURE;
>> >
>> > This check is now outside the loop and only checks for the last entry.
>> > If you wan't to split the fucntionality, don't we need a loop over all
>> > memory areas and the type?
>> >
>> > Thanks
>> > /Ilias
>>
>> Not necessarily. At the end of the day we can only really raise one exception
>> anyway. I just think informing the user about a duplicate should take priority
>> above mismatched attributes. It hints at a bigger issue.
>
> I dont mind about the priority. We can swap that over. But with this
> patch we will check for less problems than we currently do.
>
> Thanks
> /Ilias

In what regards? We're still checking for mismatched attributes, we just check
for duplicates first. If there's a dupe it doesn't really matter what the
attributes are on either allocation, good or bad something horrible has
occurred.

The prior version of this would never report if there was a duplicate unless the
first allocation it saw had the correct attributes.

>>
>> >> +       }
>> >>         return EFI_ST_SUCCESS;
>> >>  }
>> >>
>> >> --
>> >> 2.53.0
>> >>
>>

Re: [PATCHv3 4/6] efi_selftest_memory: check for duplicates first

[Simon Glass]

On 2026-04-13T20:35:52, Randolph Sapp <rs@ti.com> wrote:
> efi_selftest_memory: check for duplicates first
>
> Check for duplicate memory mappings before reporting any incorrect
> attributes. Could be that second allocation has the correct type while
> the first doesn't. Knowing there is a duplicate in this scenario is
> more helpful than just reporting the first mismatch.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>
>
> lib/efi_selftest/efi_selftest_memory.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)

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

[PATCHv3 5/6] efi_mem_sort: use list_for_each_entry_safe instead

[rs]

From: Randolph Sapp <rs@ti.com>

Use list_for_each_entry_safe and comparisons against the current and
next efi_mem_desc. This reduces the computation required for merging
regions, prevents unnecessary additional iterations of the list, and
requires less temporary values.

Signed-off-by: Randolph Sapp <rs@ti.com>
---
 lib/efi_loader/efi_memory.c | 48 +++++++++++++------------------------
 1 file changed, 17 insertions(+), 31 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index b77c2f980cc..b3b292ebf56 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -128,44 +128,30 @@ static uint64_t desc_get_end(struct efi_mem_desc *desc)
  */
 static void efi_mem_sort(void)
 {
-	struct efi_mem_list *lmem;
-	struct efi_mem_list *prevmem = NULL;
-	bool merge_again = true;
+	struct efi_mem_list *curmem, *nextmem = NULL;
 
 	list_sort(NULL, &efi_mem, efi_mem_cmp);
 
 	/* Now merge entries that can be merged */
-	while (merge_again) {
-		merge_again = false;
-		list_for_each_entry(lmem, &efi_mem, link) {
-			struct efi_mem_desc *prev;
-			struct efi_mem_desc *cur;
-			uint64_t pages;
+	list_for_each_entry_safe(curmem, nextmem, &efi_mem, link) {
+		struct efi_mem_desc *cur;
+		struct efi_mem_desc *next;
 
-			if (!prevmem) {
-				prevmem = lmem;
-				continue;
-			}
+		/* Exit when we've got nothing to compare with */
+		if (&nextmem->link == &efi_mem) {
+			break;
+		}
 
-			cur = &lmem->desc;
-			prev = &prevmem->desc;
-
-			if ((desc_get_end(cur) == prev->physical_start) &&
-			    (prev->type == cur->type) &&
-			    (prev->attribute == cur->attribute)) {
-				/* There is an existing map before, reuse it */
-				pages = cur->num_pages;
-				prev->num_pages += pages;
-				prev->physical_start -= pages << EFI_PAGE_SHIFT;
-				prev->virtual_start -= pages << EFI_PAGE_SHIFT;
-				list_del(&lmem->link);
-				free(lmem);
-
-				merge_again = true;
-				break;
-			}
+		cur = &curmem->desc;
+		next = &nextmem->desc;
 
-			prevmem = lmem;
+		if ((cur->physical_start == desc_get_end(next)) &&
+		    (cur->type == next->type) &&
+		    (cur->attribute == next->attribute)) {
+			/* There is another similar map coming up, reuse it */
+			next->num_pages += cur->num_pages;
+			list_del(&curmem->link);
+			free(curmem);
 		}
 	}
 }
-- 
2.53.0

Re: [PATCHv3 5/6] efi_mem_sort: use list_for_each_entry_safe instead

[Ilias Apalodimas]

On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> Use list_for_each_entry_safe and comparisons against the current and
> next efi_mem_desc. This reduces the computation required for merging
> regions, prevents unnecessary additional iterations of the list, and
> requires less temporary values.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>
> ---

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

>  lib/efi_loader/efi_memory.c | 48 +++++++++++++------------------------
>  1 file changed, 17 insertions(+), 31 deletions(-)
>
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index b77c2f980cc..b3b292ebf56 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -128,44 +128,30 @@ static uint64_t desc_get_end(struct efi_mem_desc *desc)
>   */
>  static void efi_mem_sort(void)
>  {
> -       struct efi_mem_list *lmem;
> -       struct efi_mem_list *prevmem = NULL;
> -       bool merge_again = true;
> +       struct efi_mem_list *curmem, *nextmem = NULL;
>
>         list_sort(NULL, &efi_mem, efi_mem_cmp);
>
>         /* Now merge entries that can be merged */
> -       while (merge_again) {
> -               merge_again = false;
> -               list_for_each_entry(lmem, &efi_mem, link) {
> -                       struct efi_mem_desc *prev;
> -                       struct efi_mem_desc *cur;
> -                       uint64_t pages;
> +       list_for_each_entry_safe(curmem, nextmem, &efi_mem, link) {
> +               struct efi_mem_desc *cur;
> +               struct efi_mem_desc *next;
>
> -                       if (!prevmem) {
> -                               prevmem = lmem;
> -                               continue;
> -                       }
> +               /* Exit when we've got nothing to compare with */
> +               if (&nextmem->link == &efi_mem) {
> +                       break;
> +               }
>
> -                       cur = &lmem->desc;
> -                       prev = &prevmem->desc;
> -
> -                       if ((desc_get_end(cur) == prev->physical_start) &&
> -                           (prev->type == cur->type) &&
> -                           (prev->attribute == cur->attribute)) {
> -                               /* There is an existing map before, reuse it */
> -                               pages = cur->num_pages;
> -                               prev->num_pages += pages;
> -                               prev->physical_start -= pages << EFI_PAGE_SHIFT;
> -                               prev->virtual_start -= pages << EFI_PAGE_SHIFT;
> -                               list_del(&lmem->link);
> -                               free(lmem);
> -
> -                               merge_again = true;
> -                               break;
> -                       }
> +               cur = &curmem->desc;
> +               next = &nextmem->desc;
>
> -                       prevmem = lmem;
> +               if ((cur->physical_start == desc_get_end(next)) &&
> +                   (cur->type == next->type) &&
> +                   (cur->attribute == next->attribute)) {
> +                       /* There is another similar map coming up, reuse it */
> +                       next->num_pages += cur->num_pages;
> +                       list_del(&curmem->link);
> +                       free(curmem);
>                 }
>         }
>  }
> --
> 2.53.0
>

Re: [PATCHv3 5/6] efi_mem_sort: use list_for_each_entry_safe instead

[Simon Glass]

On 2026-04-13T20:35:52, Randolph Sapp <rs@ti.com> wrote:
> efi_mem_sort: use list_for_each_entry_safe instead
>
> Use list_for_each_entry_safe and comparisons against the current and
> next efi_mem_desc. This reduces the computation required for merging
> regions, prevents unnecessary additional iterations of the list, and
> requires less temporary values.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> lib/efi_loader/efi_memory.c | 48 ++++++++++++++++-----------------------------
>  1 file changed, 17 insertions(+), 31 deletions(-)

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

[PATCHv3 6/6] memory: reserve from start_addr_sp to end_addr_sp

[rs]

From: Randolph Sapp <rs@ti.com>

Add a new global data struct member called end_addr_sp. This stores the
original value of relocaddr, directly from setup_dest_addr. This is
specifically to avoid any adjustments made by other init functions.

Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to
gd->end_addr_sp instead of gd->ram_top. This allows platform specific
relocation addresses to work without unnecessarily painting over a large
range.

Signed-off-by: Randolph Sapp <rs@ti.com>
---
 common/board_f.c                  |  9 ++++++-
 include/asm-generic/global_data.h |  6 +++++
 lib/efi_loader/efi_memory.c       |  4 +--
 lib/lmb.c                         | 41 ++++++-------------------------
 4 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c
index 11ad5779115..ebf2365210f 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -330,6 +330,8 @@ __weak int arch_setup_dest_addr(void)
 
 static int setup_dest_addr(void)
 {
+	int ret;
+
 	debug("Monitor len: %08x\n", gd->mon_len);
 	/*
 	 * Ram is setup, size stored in gd !!
@@ -356,7 +358,12 @@ static int setup_dest_addr(void)
 	gd->relocaddr = gd->ram_top;
 	debug("Ram top: %08llX\n", (unsigned long long)gd->ram_top);
 
-	return arch_setup_dest_addr();
+	ret = arch_setup_dest_addr();
+	if (ret != 0)
+		return ret;
+
+	gd->end_addr_sp = gd->relocaddr;
+	return ret;
 }
 
 #ifdef CFG_PRAM
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 745d2c3a966..2f3ef26a17c 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -115,6 +115,12 @@ struct global_data {
 	 * @start_addr_sp: initial stack pointer address
 	 */
 	unsigned long start_addr_sp;
+	/**
+	 * @end_addr_sp: the end of memory currently in use by uboot,
+	 *               should be the original value of the relocaddr before
+	 *               any other allocations shift it
+	 */
+	unsigned long end_addr_sp;
 	/**
 	 * @reloc_off: relocation offset
 	 */
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index b3b292ebf56..bd0d875a8cd 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -870,8 +870,8 @@ static void add_u_boot_and_runtime(void)
 	/* Add U-Boot */
 	uboot_start = ((uintptr_t)map_sysmem(gd->start_addr_sp, 0) -
 		       uboot_stack_size) & ~EFI_PAGE_MASK;
-	uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) -
-		       uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
+	uboot_pages = ((gd->end_addr_sp - uboot_start) + EFI_PAGE_MASK) >>
+		      EFI_PAGE_SHIFT;
 	efi_update_memory_map(uboot_start, uboot_pages, EFI_BOOT_SERVICES_CODE,
 			      false, false);
 #if defined(__aarch64__)
diff --git a/lib/lmb.c b/lib/lmb.c
index 7ecc548d831..da61b005154 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -534,46 +534,19 @@ static long lmb_reserve(phys_addr_t base, phys_size_t size, u32 flags)
 
 static void lmb_reserve_uboot_region(void)
 {
-	int bank;
-	ulong end, bank_end;
+	ulong size;
 	phys_addr_t rsv_start;
-	ulong pram = 0;
 
 	rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE;
-	end = gd->ram_top;
+	size = gd->end_addr_sp - rsv_start;
 
-	/*
-	 * 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);
-
-#ifdef CFG_PRAM
-	pram = env_get_ulong("pram", 10, CFG_PRAM);
-	pram = pram << 10; /* size is in kB */
-#endif
+	debug("## Current stack ends at 0x%08lx\n", (ulong)rsv_start);
 
-	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(rsv_start, bank_end - rsv_start - pram + 1,
+	if (gd->flags & GD_FLG_SKIP_RELOC)
+		lmb_reserve((phys_addr_t)(uintptr_t)_start, gd->mon_len,
 			    LMB_NOOVERWRITE);
-
-		if (gd->flags & GD_FLG_SKIP_RELOC)
-			lmb_reserve((phys_addr_t)(uintptr_t)_start,
-				    gd->mon_len, LMB_NOOVERWRITE);
-
-		break;
-	}
+	else
+		lmb_reserve(rsv_start, size, LMB_NOOVERWRITE);
 }
 
 static void lmb_reserve_common(void *fdt_blob)
-- 
2.53.0

Re: [PATCHv3 6/6] memory: reserve from start_addr_sp to end_addr_sp

[Ilias Apalodimas]

Hi Randolph,


On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> Add a new global data struct member called end_addr_sp. This stores the
> original value of relocaddr, directly from setup_dest_addr. This is
> specifically to avoid any adjustments made by other init functions.
>
> Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to
> gd->end_addr_sp instead of gd->ram_top. This allows platform specific
> relocation addresses to work without unnecessarily painting over a large
> range.
>

Is there a platform we can test the changes? Running the in QEMU had
no differences.

> Signed-off-by: Randolph Sapp <rs@ti.com>
> ---

[...]

>  #ifdef CFG_PRAM
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index 745d2c3a966..2f3ef26a17c 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -115,6 +115,12 @@ struct global_data {
>          * @start_addr_sp: initial stack pointer address
>          */
>         unsigned long start_addr_sp;
> +       /**
> +        * @end_addr_sp: the end of memory currently in use by uboot,
> +        *               should be the original value of the relocaddr before
> +        *               any other allocations shift it
> +        */
> +       unsigned long end_addr_sp;
>         /**
>          * @reloc_off: relocation offset
>          */
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index b3b292ebf56..bd0d875a8cd 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -870,8 +870,8 @@ static void add_u_boot_and_runtime(void)
>         /* Add U-Boot */
>         uboot_start = ((uintptr_t)map_sysmem(gd->start_addr_sp, 0) -
>                        uboot_stack_size) & ~EFI_PAGE_MASK;
> -       uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) -
> -                      uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
> +       uboot_pages = ((gd->end_addr_sp - uboot_start) + EFI_PAGE_MASK) >>
> +                     EFI_PAGE_SHIFT;

The map_sysmem dance is still needed here for sandbox.

>         efi_update_memory_map(uboot_start, uboot_pages, EFI_BOOT_SERVICES_CODE,
>                               false, false);
>  #if defined(__aarch64__)
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 7ecc548d831..da61b005154 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -534,46 +534,19 @@ static long lmb_reserve(phys_addr_t base, phys_size_t size, u32 flags)
>
>  static void lmb_reserve_uboot_region(void)
>  {
> -       int bank;
> -       ulong end, bank_end;
> +       ulong size;
>         phys_addr_t rsv_start;
> -       ulong pram = 0;
>
>         rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE;
> -       end = gd->ram_top;
> +       size = gd->end_addr_sp - rsv_start;
>
> -       /*
> -        * 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);
> -
> -#ifdef CFG_PRAM
> -       pram = env_get_ulong("pram", 10, CFG_PRAM);
> -       pram = pram << 10; /* size is in kB */
> -#endif

This functionality is removed, but we still have boards defining CFG_PRAM

> +       debug("## Current stack ends at 0x%08lx\n", (ulong)rsv_start);
>
> -       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(rsv_start, bank_end - rsv_start - pram + 1,
> +       if (gd->flags & GD_FLG_SKIP_RELOC)
> +               lmb_reserve((phys_addr_t)(uintptr_t)_start, gd->mon_len,
>                             LMB_NOOVERWRITE);
> -
> -               if (gd->flags & GD_FLG_SKIP_RELOC)
> -                       lmb_reserve((phys_addr_t)(uintptr_t)_start,
> -                                   gd->mon_len, LMB_NOOVERWRITE);
> -
> -               break;
> -       }
> +       else
> +               lmb_reserve(rsv_start, size, LMB_NOOVERWRITE);
>  }
>
>  static void lmb_reserve_common(void *fdt_blob)
> --
> 2.53.0
>

Thanks
/Ilias

Re: [PATCHv3 6/6] memory: reserve from start_addr_sp to end_addr_sp

[Randolph Sapp]

On Thu Apr 16, 2026 at 9:37 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 a new global data struct member called end_addr_sp. This stores the
>> original value of relocaddr, directly from setup_dest_addr. This is
>> specifically to avoid any adjustments made by other init functions.
>>
>> Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to
>> gd->end_addr_sp instead of gd->ram_top. This allows platform specific
>> relocation addresses to work without unnecessarily painting over a large
>> range.
>>
>
> Is there a platform we can test the changes? Running the in QEMU had
> no differences.

Not yet. This is a mandatory requirement for pocketbeagle2, but otherwise the
default behavior is the same as it ever was.

You can change the relocation address in the function you snipped out below to
an arbitrary value for testing.

>> Signed-off-by: Randolph Sapp <rs@ti.com>
>> ---
>
> [...]
>
>>  #ifdef CFG_PRAM
>> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
>> index 745d2c3a966..2f3ef26a17c 100644
>> --- a/include/asm-generic/global_data.h
>> +++ b/include/asm-generic/global_data.h
>> @@ -115,6 +115,12 @@ struct global_data {
>>          * @start_addr_sp: initial stack pointer address
>>          */
>>         unsigned long start_addr_sp;
>> +       /**
>> +        * @end_addr_sp: the end of memory currently in use by uboot,
>> +        *               should be the original value of the relocaddr before
>> +        *               any other allocations shift it
>> +        */
>> +       unsigned long end_addr_sp;
>>         /**
>>          * @reloc_off: relocation offset
>>          */
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index b3b292ebf56..bd0d875a8cd 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -870,8 +870,8 @@ static void add_u_boot_and_runtime(void)
>>         /* Add U-Boot */
>>         uboot_start = ((uintptr_t)map_sysmem(gd->start_addr_sp, 0) -
>>                        uboot_stack_size) & ~EFI_PAGE_MASK;
>> -       uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) -
>> -                      uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
>> +       uboot_pages = ((gd->end_addr_sp - uboot_start) + EFI_PAGE_MASK) >>
>> +                     EFI_PAGE_SHIFT;
>
> The map_sysmem dance is still needed here for sandbox.
>

Why? We're not using that address directly. We're just trying to figure out how
large the carveout region is.

There's not anything funky going on with the global data structure for sandbox
functionality is there? LMB definitely didn't do anything to account for that.

>>         efi_update_memory_map(uboot_start, uboot_pages, EFI_BOOT_SERVICES_CODE,
>>                               false, false);
>>  #if defined(__aarch64__)
>> diff --git a/lib/lmb.c b/lib/lmb.c
>> index 7ecc548d831..da61b005154 100644
>> --- a/lib/lmb.c
>> +++ b/lib/lmb.c
>> @@ -534,46 +534,19 @@ static long lmb_reserve(phys_addr_t base, phys_size_t size, u32 flags)
>>
>>  static void lmb_reserve_uboot_region(void)
>>  {
>> -       int bank;
>> -       ulong end, bank_end;
>> +       ulong size;
>>         phys_addr_t rsv_start;
>> -       ulong pram = 0;
>>
>>         rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE;
>> -       end = gd->ram_top;
>> +       size = gd->end_addr_sp - rsv_start;
>>
>> -       /*
>> -        * 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);
>> -
>> -#ifdef CFG_PRAM
>> -       pram = env_get_ulong("pram", 10, CFG_PRAM);
>> -       pram = pram << 10; /* size is in kB */
>> -#endif
>
> This functionality is removed, but we still have boards defining CFG_PRAM

The PRAM allocation occurs before this, and it is carved out of the region
covered by start_addr_sp through end_addr_sp. It's actually part of the reason I
needed to add end_addr_sp, as it moves the relocation address pointer. We don't
need to account for it separately now.

>> +       debug("## Current stack ends at 0x%08lx\n", (ulong)rsv_start);
>>
>> -       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(rsv_start, bank_end - rsv_start - pram + 1,
>> +       if (gd->flags & GD_FLG_SKIP_RELOC)
>> +               lmb_reserve((phys_addr_t)(uintptr_t)_start, gd->mon_len,
>>                             LMB_NOOVERWRITE);
>> -
>> -               if (gd->flags & GD_FLG_SKIP_RELOC)
>> -                       lmb_reserve((phys_addr_t)(uintptr_t)_start,
>> -                                   gd->mon_len, LMB_NOOVERWRITE);
>> -
>> -               break;
>> -       }
>> +       else
>> +               lmb_reserve(rsv_start, size, LMB_NOOVERWRITE);
>>  }
>>
>>  static void lmb_reserve_common(void *fdt_blob)
>> --
>> 2.53.0
>>
>
> Thanks
> /Ilias

Re: [PATCHv3 6/6] memory: reserve from start_addr_sp to end_addr_sp

[Randolph Sapp]

On Thu Apr 16, 2026 at 2:01 PM CDT, Randolph Sapp wrote:
> On Thu Apr 16, 2026 at 9:37 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 a new global data struct member called end_addr_sp. This stores the
>>> original value of relocaddr, directly from setup_dest_addr. This is
>>> specifically to avoid any adjustments made by other init functions.
>>>
>>> Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to
>>> gd->end_addr_sp instead of gd->ram_top. This allows platform specific
>>> relocation addresses to work without unnecessarily painting over a large
>>> range.
>>>
>>
>> Is there a platform we can test the changes? Running the in QEMU had
>> no differences.
>
> Not yet. This is a mandatory requirement for pocketbeagle2, but otherwise the
> default behavior is the same as it ever was.
>
> You can change the relocation address in the function you snipped out below to
> an arbitrary value for testing.
>
>>> Signed-off-by: Randolph Sapp <rs@ti.com>
>>> ---
>>
>> [...]
>>
>>>  #ifdef CFG_PRAM
>>> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
>>> index 745d2c3a966..2f3ef26a17c 100644
>>> --- a/include/asm-generic/global_data.h
>>> +++ b/include/asm-generic/global_data.h
>>> @@ -115,6 +115,12 @@ struct global_data {
>>>          * @start_addr_sp: initial stack pointer address
>>>          */
>>>         unsigned long start_addr_sp;
>>> +       /**
>>> +        * @end_addr_sp: the end of memory currently in use by uboot,
>>> +        *               should be the original value of the relocaddr before
>>> +        *               any other allocations shift it
>>> +        */
>>> +       unsigned long end_addr_sp;
>>>         /**
>>>          * @reloc_off: relocation offset
>>>          */
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index b3b292ebf56..bd0d875a8cd 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -870,8 +870,8 @@ static void add_u_boot_and_runtime(void)
>>>         /* Add U-Boot */
>>>         uboot_start = ((uintptr_t)map_sysmem(gd->start_addr_sp, 0) -
>>>                        uboot_stack_size) & ~EFI_PAGE_MASK;
>>> -       uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) -
>>> -                      uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
>>> +       uboot_pages = ((gd->end_addr_sp - uboot_start) + EFI_PAGE_MASK) >>
>>> +                     EFI_PAGE_SHIFT;
>>
>> The map_sysmem dance is still needed here for sandbox.
>>
>
> Why? We're not using that address directly. We're just trying to figure out how
> large the carveout region is.
>
> There's not anything funky going on with the global data structure for sandbox
> functionality is there? LMB definitely didn't do anything to account for that.

Ah. Symmetry, to answer my own question. We *may* get a virtual address.

That does bring up another issue though. What good does reserving a virtual
region that simply maps to a physical region do? Shouldn't we reserve the
physical region outright? Isn't that what LMB is tracking?

Suppose I need to read up on this sandbox and architecture dependent virtual
addressing scheme.

>>>         efi_update_memory_map(uboot_start, uboot_pages, EFI_BOOT_SERVICES_CODE,
>>>                               false, false);
>>>  #if defined(__aarch64__)
>>> diff --git a/lib/lmb.c b/lib/lmb.c
>>> index 7ecc548d831..da61b005154 100644
>>> --- a/lib/lmb.c
>>> +++ b/lib/lmb.c
>>> @@ -534,46 +534,19 @@ static long lmb_reserve(phys_addr_t base, phys_size_t size, u32 flags)
>>>
>>>  static void lmb_reserve_uboot_region(void)
>>>  {
>>> -       int bank;
>>> -       ulong end, bank_end;
>>> +       ulong size;
>>>         phys_addr_t rsv_start;
>>> -       ulong pram = 0;
>>>
>>>         rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE;
>>> -       end = gd->ram_top;
>>> +       size = gd->end_addr_sp - rsv_start;
>>>
>>> -       /*
>>> -        * 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);
>>> -
>>> -#ifdef CFG_PRAM
>>> -       pram = env_get_ulong("pram", 10, CFG_PRAM);
>>> -       pram = pram << 10; /* size is in kB */
>>> -#endif
>>
>> This functionality is removed, but we still have boards defining CFG_PRAM
>
> The PRAM allocation occurs before this, and it is carved out of the region
> covered by start_addr_sp through end_addr_sp. It's actually part of the reason I
> needed to add end_addr_sp, as it moves the relocation address pointer. We don't
> need to account for it separately now.
>
>>> +       debug("## Current stack ends at 0x%08lx\n", (ulong)rsv_start);
>>>
>>> -       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(rsv_start, bank_end - rsv_start - pram + 1,
>>> +       if (gd->flags & GD_FLG_SKIP_RELOC)
>>> +               lmb_reserve((phys_addr_t)(uintptr_t)_start, gd->mon_len,
>>>                             LMB_NOOVERWRITE);
>>> -
>>> -               if (gd->flags & GD_FLG_SKIP_RELOC)
>>> -                       lmb_reserve((phys_addr_t)(uintptr_t)_start,
>>> -                                   gd->mon_len, LMB_NOOVERWRITE);
>>> -
>>> -               break;
>>> -       }
>>> +       else
>>> +               lmb_reserve(rsv_start, size, LMB_NOOVERWRITE);
>>>  }
>>>
>>>  static void lmb_reserve_common(void *fdt_blob)
>>> --
>>> 2.53.0
>>>
>>
>> Thanks
>> /Ilias

Re: [PATCHv3 6/6] memory: reserve from start_addr_sp to end_addr_sp

[Simon Glass]

Hi Randolph,

On 2026-04-13T20:35:52, Randolph Sapp <rs@ti.com> wrote:
> memory: reserve from start_addr_sp to end_addr_sp
>
> Add a new global data struct member called end_addr_sp. This stores the
> original value of relocaddr, directly from setup_dest_addr. This is
> specifically to avoid any adjustments made by other init functions.
>
> Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to
> gd->end_addr_sp instead of gd->ram_top. This allows platform specific
> relocation addresses to work without unnecessarily painting over a large
> range.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>
>
> common/board_f.c                  |  9 ++++++++-
>  include/asm-generic/global_data.h |  6 ++++++
>  lib/efi_loader/efi_memory.c       |  4 ++--
>  lib/lmb.c                         | 41 +++++++--------------------------------
>  4 files changed, 23 insertions(+), 37 deletions(-)

> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> @@ -115,6 +115,12 @@ struct global_data {
> +     /**
> +      * @end_addr_sp: the end of memory currently in use by uboot,
> +      *               should be the original value of the relocaddr before
> +      *               any other allocations shift it
> +      */
> +     unsigned long end_addr_sp;

The naming is confusing, although I realise where it comes from. This
is actually the top of the U-Boot reserved region - how above
reserve_top or initial_relocaddr ?

> diff --git a/lib/lmb.c b/lib/lmb.c
> @@ -534,46 +534,19 @@ static long lmb_reserve(phys_addr_t base, phys_size_t size, u32 flags)
> +     if (gd->flags & GD_FLG_SKIP_RELOC)
> +             lmb_reserve((phys_addr_t)(uintptr_t)_start, gd->mon_len,
> +                         LMB_NOOVERWRITE);
> +     else
> +             lmb_reserve(rsv_start, size, LMB_NOOVERWRITE);

This changes behaviour for GD_FLG_SKIP_RELOC, doesn't it? At present
both the stack region and the monitor region are reserved, but with
this only the monitor region is reserved. Is this intentional?

Regards,
Simon