[PATCHv2 0/5] various memory related fixups

[PATCHv2 0/5] 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 first 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

Randolph Sapp (5):
  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: skip unnecessary compare
  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                          |  7 +++
 lib/efi_loader/efi_dt_fixup.c          |  3 +-
 lib/efi_loader/efi_memory.c            |  6 +--
 lib/efi_selftest/efi_selftest_memory.c | 21 ++++----
 lib/lmb.c                              | 66 +++++++++++++-------------
 8 files changed, 71 insertions(+), 52 deletions(-)

-- 
2.53.0

[PATCHv2 1/5] 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    |  7 +++++++
 lib/lmb.c        | 25 +++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

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 5d5f037ccb9..1b59a63f61d 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -25,11 +25,13 @@
  * %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_FDT: A FDT reserved region that can be reclaimed if the FDT changes
  */
 #define LMB_NONE 0
 #define LMB_NOMAP BIT(1)
 #define LMB_NOOVERWRITE BIT(2)
 #define LMB_NONOTIFY BIT(3)
+#define LMB_FDT BIT(4)
 
 /**
  * enum lmb_mem_type - type of memory allocation request
@@ -215,6 +217,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 e2d9fe86c14..18703212afb 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -669,6 +669,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",
+				       (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: [PATCHv2 1/5] lmb: add LMB_FDT for fdt reserved regions

[Randolph Sapp]

On Thu Apr 2, 2026 at 3:50 PM CDT, rs 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    |  7 +++++++
>  lib/lmb.c        | 25 +++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 1 deletion(-)
>
> 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 5d5f037ccb9..1b59a63f61d 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -25,11 +25,13 @@
>   * %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_FDT: A FDT reserved region that can be reclaimed if the FDT changes
>   */
>  #define LMB_NONE 0
>  #define LMB_NOMAP BIT(1)
>  #define LMB_NOOVERWRITE BIT(2)
>  #define LMB_NONOTIFY BIT(3)
> +#define LMB_FDT BIT(4)
>  
>  /**
>   * enum lmb_mem_type - type of memory allocation request
> @@ -215,6 +217,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 e2d9fe86c14..18703212afb 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -669,6 +669,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",
> +				       (ulong)base);

Forgot to add a newline character here.

> +				i++;
> +			}
> +		} else {
> +			i++;
> +		}
> +	}
> +}
> +
>  static int _lmb_alloc_base(phys_size_t size, ulong align,
>  			   phys_addr_t *addr, u32 flags)
>  {

Ah, forgot to update the lmb_print_region_flags function. Sorry about that.

Re: [PATCHv2 1/5] lmb: add LMB_FDT for fdt reserved regions

[Heinrich Schuchardt]

Am 2. April 2026 22:50:34 MESZ schrieb rs@ti.com:
>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    |  7 +++++++
> lib/lmb.c        | 25 +++++++++++++++++++++++++
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
>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 5d5f037ccb9..1b59a63f61d 100644
>--- a/include/lmb.h
>+++ b/include/lmb.h
>@@ -25,11 +25,13 @@
>  * %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_FDT: A FDT reserved region that can be reclaimed if the FDT changes
>  */
> #define LMB_NONE 0
> #define LMB_NOMAP BIT(1)
> #define LMB_NOOVERWRITE BIT(2)
> #define LMB_NONOTIFY BIT(3)
>+#define LMB_FDT BIT(4)

Please, document the constants according to https://docs.kernel.org/doc-guide/kernel-doc.html#object-like-macro-documentation .  This allows Sphinx to generate the API documentation.

Best regards

Heinrich

> 
> /**
>  * enum lmb_mem_type - type of memory allocation request
>@@ -215,6 +217,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 e2d9fe86c14..18703212afb 100644
>--- a/lib/lmb.c
>+++ b/lib/lmb.c
>@@ -669,6 +669,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",
>+				       (ulong)base);
>+				i++;
>+			}
>+		} else {
>+			i++;
>+		}
>+	}
>+}
>+
> static int _lmb_alloc_base(phys_size_t size, ulong align,
> 			   phys_addr_t *addr, u32 flags)
> {

[PATCHv2 2/5] 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>
---
 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: [PATCHv2 2/5] efi_dt_fixup: use fdtdec_get_bool

[Anshul Dalal]

On Thu, 02 Apr 2026 15:50:35 -0500, rs@ti.com <rs@ti.com> wrote:
> Use the more straightforward fdtdec_get_bool instead of fdt_getprop and
> a return code check.
> 
> Signed-off-by: Randolph Sapp <rs@ti.com>

This is a good fix even outside the context of the series.

Reviewed-by: Anshul Dalal <anshuld@ti.com>

-- 
Anshul Dalal <anshuld@ti.com>

Re: [PATCHv2 2/5] efi_dt_fixup: use fdtdec_get_bool

[Heinrich Schuchardt]

Am 2. April 2026 22:50:35 MESZ schrieb rs@ti.com:
>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: 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);

Re: [PATCHv2 2/5] efi_dt_fixup: use fdtdec_get_bool

[Ilias Apalodimas]

Please carry over the r-b tags from older versions

On Thu, 9 Apr 2026 at 22:22, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 2. April 2026 22:50:35 MESZ schrieb rs@ti.com:
> >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: 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);
>

[PATCHv2 3/5] 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

[PATCHv2 4/5] efi_mem_sort: skip unnecessary compare

[rs]

From: Randolph Sapp <rs@ti.com>

I don't want to think about the possibility of this pointer containing a
reference to something from a previous iteration. At best it results in
a some arithmetic and a comparison that should usually be unnecessary.

Signed-off-by: Randolph Sapp <rs@ti.com>
---
 lib/efi_loader/efi_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index b77c2f980cc..882366a9f8a 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -129,13 +129,13 @@ 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;
 
 	list_sort(NULL, &efi_mem, efi_mem_cmp);
 
 	/* Now merge entries that can be merged */
 	while (merge_again) {
+		struct efi_mem_list *prevmem = NULL;
 		merge_again = false;
 		list_for_each_entry(lmem, &efi_mem, link) {
 			struct efi_mem_desc *prev;
-- 
2.53.0

Re: [PATCHv2 4/5] efi_mem_sort: skip unnecessary compare

[Heinrich Schuchardt]

Am 2. April 2026 22:50:37 MESZ schrieb rs@ti.com:
>From: Randolph Sapp <rs@ti.com>
>
>I don't want to think about the possibility of this pointer containing a
>reference to something from a previous iteration. At best it results in
>a some arithmetic and a comparison that should usually be unnecessary.

"a some arithmetic" is not understandable. Please, adjust the commit message.

>
>Signed-off-by: Randolph Sapp <rs@ti.com>
>---
> lib/efi_loader/efi_memory.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>index b77c2f980cc..882366a9f8a 100644
>--- a/lib/efi_loader/efi_memory.c
>+++ b/lib/efi_loader/efi_memory.c
>@@ -129,13 +129,13 @@ 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;
> 
> 	list_sort(NULL, &efi_mem, efi_mem_cmp);
> 
> 	/* Now merge entries that can be merged */
> 	while (merge_again) {

The suggested change looks correct. But we could go further:

We could get rid of this outer loop by either eliminating prev instead of cur or by using list_for_each_safe().

Best regards

Heinrich

>+		struct efi_mem_list *prevmem = NULL;
> 		merge_again = false;
> 		list_for_each_entry(lmem, &efi_mem, link) {
> 			struct efi_mem_desc *prev;

[PATCHv2 5/5] 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 df2b0dc899b..85b1c3f3428 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -331,6 +331,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 !!
@@ -357,7 +359,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 882366a9f8a..7968ae4b9ba 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -884,8 +884,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 18703212afb..b38141275da 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