[PATCH 0/6] various memory related fixups

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

Randolph Sapp (6):
  lmb: add LMB_FDT for fdt reserved regions
  efi_dt_fixup: use fdtdec_get_bool
  efi_selftest_memory: check for duplicates first
  efi_memory: nitpick removal loop
  efi_memory: backfill EFI_CONVENTIONAL_MEMORY
  memory: reserve from start_addr_sp to relocaddr

 boot/image-fdt.c                       |  5 +++-
 include/lmb.h                          |  7 +++++
 lib/efi_loader/efi_dt_fixup.c          |  3 +-
 lib/efi_loader/efi_memory.c            | 41 ++++++++++++++++++++++----
 lib/efi_selftest/efi_selftest_memory.c | 21 +++++++------
 lib/lmb.c                              | 25 +++++++++++++++-
 6 files changed, 82 insertions(+), 20 deletions(-)

-- 
2.53.0

[PATCH 1/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    |  7 +++++++
 lib/lmb.c        | 23 +++++++++++++++++++++++
 3 files changed, 34 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..542bb11dcf5 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -669,6 +669,29 @@ 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;
+
+	for (int i = 0; i < lmb_rgn_lst->count; i++) {
+		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);
+				continue;
+			}
+			i--;
+		}
+	}
+}
+
 static int _lmb_alloc_base(phys_size_t size, ulong align,
 			   phys_addr_t *addr, u32 flags)
 {
-- 
2.53.0

[PATCH 2/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>
---
 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

[PATCH 3/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

[PATCH 4/6] efi_memory: nitpick removal loop

[rs]

From: Randolph Sapp <rs@ti.com>

I don't even want to think about the possibility of this pointer
containing a reference to something from a previous iteration.

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

[PATCH 5/6] efi_memory: backfill EFI_CONVENTIONAL_MEMORY

[rs]

From: Randolph Sapp <rs@ti.com>

Backfill any fragmented EFI_CONVENTIONAL_MEMORY when LMB allocations
start to fail. This may be a low memory environment, or maybe LMB is
running up against a tricky reserved region.

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

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 882366a9f8a..f07cc39b157 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -441,6 +441,27 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated)
 	return EFI_NOT_FOUND;
 }
 
+/**
+ * efi_get_conventional_start - find the highest conventional region start
+ *                              address with at least the specified number of
+ *                              pages
+ *
+ * @pages:                      number of pages required to be in that carveout
+ * Return:                      starting address of the give carveout
+ */
+static u64 efi_get_conventional_start(u64 pages)
+{
+	struct efi_mem_list *item;
+
+	list_for_each_entry(item, &efi_mem, link) {
+		if (item->desc.type != EFI_CONVENTIONAL_MEMORY)
+			continue;
+		if (item->desc.num_pages >= pages)
+			return item->desc.physical_start;
+	}
+	return 0;
+}
+
 /**
  * efi_allocate_pages - allocate memory pages
  *
@@ -507,10 +528,20 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
 	/* Reserve that map in our memory maps */
 	ret = efi_update_memory_map(efi_addr, pages, memory_type, true, false);
 	if (ret != EFI_SUCCESS) {
-		/* Map would overlap, bail out */
+		/* Map would overlap, try something else */
 		lmb_free(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
 		unmap_sysmem((void *)(uintptr_t)efi_addr);
-		return  EFI_OUT_OF_RESOURCES;
+
+		/* See if there is any EFI_CONVENTIONAL_MEMORY allocations */
+		if (type != EFI_ALLOCATE_ADDRESS) {
+			*memory = efi_get_conventional_start(pages);
+			if (*memory != 0)
+				return efi_allocate_pages(EFI_ALLOCATE_ADDRESS,
+							  memory_type, pages,
+							  memory);
+		}
+
+		return EFI_OUT_OF_RESOURCES;
 	}
 
 	*memory = efi_addr;
-- 
2.53.0

[PATCH 6/6] memory: reserve from start_addr_sp to relocaddr

[rs]

From: Randolph Sapp <rs@ti.com>

Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to
gd->relocaddr 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>
---
 lib/efi_loader/efi_memory.c | 4 ++--
 lib/lmb.c                   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index f07cc39b157..57691d15758 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -915,8 +915,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->relocaddr - 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 542bb11dcf5..0df8157db7f 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -540,7 +540,7 @@ static void lmb_reserve_uboot_region(void)
 	ulong pram = 0;
 
 	rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE;
-	end = gd->ram_top;
+	end = gd->relocaddr;
 
 	/*
 	 * Reserve memory from aligned address below the bottom of U-Boot stack
-- 
2.53.0

Re: [PATCH 1/6] lmb: add LMB_FDT for fdt reserved regions

[Randolph Sapp]

On Wed Apr 1, 2026 at 7:14 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        | 23 +++++++++++++++++++++++
>  3 files changed, 34 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..542bb11dcf5 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -669,6 +669,29 @@ 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;
> +
> +	for (int i = 0; i < lmb_rgn_lst->count; i++) {
> +		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);
> +				continue;
> +			}
> +			i--;
> +		}
> +	}
> +}
> +
>  static int _lmb_alloc_base(phys_size_t size, ulong align,
>  			   phys_addr_t *addr, u32 flags)
>  {

Ugh. Looking at that loop in lmb_free_fdt_regions more I really should have
written it differently to avoid the additional increments and decrements. I'll
fix that in v2.

Re: [PATCH 2/6] efi_dt_fixup: use fdtdec_get_bool

[Ilias Apalodimas]

On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>
> 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>

>  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: [PATCH 5/6] efi_memory: backfill EFI_CONVENTIONAL_MEMORY

[Ilias Apalodimas]

Hi Randolph

On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> Backfill any fragmented EFI_CONVENTIONAL_MEMORY when LMB allocations
> start to fail. This may be a low memory environment, or maybe LMB is
> running up against a tricky reserved region.
>
> Signed-off-by: Randolph Sapp <rs@ti.com>
> ---
>  lib/efi_loader/efi_memory.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 882366a9f8a..f07cc39b157 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -441,6 +441,27 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated)
>         return EFI_NOT_FOUND;
>  }
>
> +/**
> + * efi_get_conventional_start - find the highest conventional region start
> + *                              address with at least the specified number of
> + *                              pages
> + *
> + * @pages:                      number of pages required to be in that carveout
> + * Return:                      starting address of the give carveout
> + */
> +static u64 efi_get_conventional_start(u64 pages)
> +{
> +       struct efi_mem_list *item;
> +
> +       list_for_each_entry(item, &efi_mem, link) {
> +               if (item->desc.type != EFI_CONVENTIONAL_MEMORY)
> +                       continue;
> +               if (item->desc.num_pages >= pages)
> +                       return item->desc.physical_start;
> +       }
> +       return 0;

We've been bitten by this in the past and although unlikely 0 is a
valid address for some hardware. Can we return something different if
we don't find a match? UINT64_MAX perhaps?

> +}
> +
>  /**
>   * efi_allocate_pages - allocate memory pages
>   *
> @@ -507,10 +528,20 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
>         /* Reserve that map in our memory maps */
>         ret = efi_update_memory_map(efi_addr, pages, memory_type, true, false);

In theory the lmb and the EFI maps are in sync. I haven't checked
close enough yet, but do you have cases where lmb_alloc worked and
updating the efi memory map failed?

>         if (ret != EFI_SUCCESS) {
> -               /* Map would overlap, bail out */
> +               /* Map would overlap, try something else */
>                 lmb_free(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
>                 unmap_sysmem((void *)(uintptr_t)efi_addr);
> -               return  EFI_OUT_OF_RESOURCES;
> +
> +               /* See if there is any EFI_CONVENTIONAL_MEMORY allocations */
> +               if (type != EFI_ALLOCATE_ADDRESS) {

Can you please inverse this. It's going to reduce the identation.
if (type == EFI_ALLOCATE_ADDRESS)
> +                       *memory = efi_get_conventional_start(pages);
> +                       if (*memory != 0)

Same here

> +                               return efi_allocate_pages(EFI_ALLOCATE_ADDRESS,
> +                                                         memory_type, pages,
> +                                                         memory);
> +               }
> +
> +               return EFI_OUT_OF_RESOURCES;
>         }
>
>         *memory = efi_addr;
> --
> 2.53.0
>

Thanks
/Ilias

Re: [PATCH 4/6] efi_memory: nitpick removal loop

[Ilias Apalodimas]

I don't mind merging this, but why is the current code flawed?

Looking at the function even if prevmem contains a previous valid
entry the check will fail

if ((desc_get_end(cur) == prev->physical_start) &&
    (prev->type == cur->type) &&
    (prev->attribute == cur->attribute)) {

and re-assign prevmem.

So please adjust the commit message if you want this merged.

Thanks
/Ilias

On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> I don't even want to think about the possibility of this pointer
> containing a reference to something from a previous iteration.
>
> 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: [PATCH 6/6] memory: reserve from start_addr_sp to relocaddr

[Ilias Apalodimas]

On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to
> gd->relocaddr instead of gd->ram_top. This allows platform specific
> relocation addresses to work without unnecessarily painting over a large
> range.

This hangs when trying to boot with UEFI on QEMU aarch64.
With the qemu_arm64_lwip_defconfig and this command line

qemu-system-aarch64 -m 8192 -smp 2 -nographic -cpu cortex-a57 \
    -machine virt,secure=off \
    -bios u-boot.bin \
    -device virtio-rng-pci \
    -drive id=os,if=none,file=my.iso \
    -device virtio-blk-device,drive=os \
    -object memory-backend-ram,id=ram0,size=4G \
    -object memory-backend-ram,id=ram1,size=4G \
    -numa node,memdev=ram0 \
    -numa node,memdev=ram1

It also hangs when trying to initialize the EFI subsystem in general,
e.g 'efidebug memmap', probably because some EFI code is relocated on
the region you exlcuded.

Thanks
/Ilias

>
> Signed-off-by: Randolph Sapp <rs@ti.com>
> ---
>  lib/efi_loader/efi_memory.c | 4 ++--
>  lib/lmb.c                   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index f07cc39b157..57691d15758 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -915,8 +915,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->relocaddr - 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 542bb11dcf5..0df8157db7f 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -540,7 +540,7 @@ static void lmb_reserve_uboot_region(void)
>         ulong pram = 0;
>
>         rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE;
> -       end = gd->ram_top;
> +       end = gd->relocaddr;
>
>         /*
>          * Reserve memory from aligned address below the bottom of U-Boot stack
> --
> 2.53.0
>

Re: [PATCH 5/6] efi_memory: backfill EFI_CONVENTIONAL_MEMORY

[Randolph Sapp]

On Thu Apr 2, 2026 at 3:53 AM CDT, Ilias Apalodimas wrote:
> Hi Randolph
>
> On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>>
>> From: Randolph Sapp <rs@ti.com>
>>
>> Backfill any fragmented EFI_CONVENTIONAL_MEMORY when LMB allocations
>> start to fail. This may be a low memory environment, or maybe LMB is
>> running up against a tricky reserved region.
>>
>> Signed-off-by: Randolph Sapp <rs@ti.com>
>> ---
>>  lib/efi_loader/efi_memory.c | 35 +++++++++++++++++++++++++++++++++--
>>  1 file changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index 882366a9f8a..f07cc39b157 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -441,6 +441,27 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated)
>>         return EFI_NOT_FOUND;
>>  }
>>
>> +/**
>> + * efi_get_conventional_start - find the highest conventional region start
>> + *                              address with at least the specified number of
>> + *                              pages
>> + *
>> + * @pages:                      number of pages required to be in that carveout
>> + * Return:                      starting address of the give carveout
>> + */
>> +static u64 efi_get_conventional_start(u64 pages)
>> +{
>> +       struct efi_mem_list *item;
>> +
>> +       list_for_each_entry(item, &efi_mem, link) {
>> +               if (item->desc.type != EFI_CONVENTIONAL_MEMORY)
>> +                       continue;
>> +               if (item->desc.num_pages >= pages)
>> +                       return item->desc.physical_start;
>> +       }
>> +       return 0;
>
> We've been bitten by this in the past and although unlikely 0 is a
> valid address for some hardware. Can we return something different if
> we don't find a match? UINT64_MAX perhaps?
>
>> +}
>> +
>>  /**
>>   * efi_allocate_pages - allocate memory pages
>>   *
>> @@ -507,10 +528,20 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
>>         /* Reserve that map in our memory maps */
>>         ret = efi_update_memory_map(efi_addr, pages, memory_type, true, false);
>
> In theory the lmb and the EFI maps are in sync. I haven't checked
> close enough yet, but do you have cases where lmb_alloc worked and
> updating the efi memory map failed?

Fair point. Now that I've fixed the FDT warning I should probably check if I can
actually reproduce the issue that actually required this.

>>         if (ret != EFI_SUCCESS) {
>> -               /* Map would overlap, bail out */
>> +               /* Map would overlap, try something else */
>>                 lmb_free(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
>>                 unmap_sysmem((void *)(uintptr_t)efi_addr);
>> -               return  EFI_OUT_OF_RESOURCES;
>> +
>> +               /* See if there is any EFI_CONVENTIONAL_MEMORY allocations */
>> +               if (type != EFI_ALLOCATE_ADDRESS) {
>
> Can you please inverse this. It's going to reduce the identation.
> if (type == EFI_ALLOCATE_ADDRESS)
>> +                       *memory = efi_get_conventional_start(pages);
>> +                       if (*memory != 0)
>
> Same here
>
>> +                               return efi_allocate_pages(EFI_ALLOCATE_ADDRESS,
>> +                                                         memory_type, pages,
>> +                                                         memory);
>> +               }
>> +
>> +               return EFI_OUT_OF_RESOURCES;
>>         }
>>
>>         *memory = efi_addr;
>> --
>> 2.53.0
>>
>
> Thanks
> /Ilias

Re: [PATCH 4/6] efi_memory: nitpick removal loop

[Randolph Sapp]

On Thu Apr 2, 2026 at 4:04 AM CDT, Ilias Apalodimas wrote:
> I don't mind merging this, but why is the current code flawed?
>
> Looking at the function even if prevmem contains a previous valid
> entry the check will fail
>
> if ((desc_get_end(cur) == prev->physical_start) &&
>     (prev->type == cur->type) &&
>     (prev->attribute == cur->attribute)) {
>
> and re-assign prevmem.
>
> So please adjust the commit message if you want this merged.
>
> Thanks
> /Ilias

Ah, not really that it is "flawed" right now, but more so that we should never
even have to think about that occurrence, because at best it's an unnecessary
compare. Why waste time trying to defend or attack a line of code that simply
shouldn't exist?

> On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>>
>> From: Randolph Sapp <rs@ti.com>
>>
>> I don't even want to think about the possibility of this pointer
>> containing a reference to something from a previous iteration.
>>
>> 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: [PATCH 4/6] efi_memory: nitpick removal loop

[Ilias Apalodimas]

On Thu, 2 Apr 2026 at 20:39, Randolph Sapp <rs@ti.com> wrote:
>
> On Thu Apr 2, 2026 at 4:04 AM CDT, Ilias Apalodimas wrote:
> > I don't mind merging this, but why is the current code flawed?
> >
> > Looking at the function even if prevmem contains a previous valid
> > entry the check will fail
> >
> > if ((desc_get_end(cur) == prev->physical_start) &&
> >     (prev->type == cur->type) &&
> >     (prev->attribute == cur->attribute)) {
> >
> > and re-assign prevmem.
> >
> > So please adjust the commit message if you want this merged.
> >
> > Thanks
> > /Ilias
>
> Ah, not really that it is "flawed" right now, but more so that we should never
> even have to think about that occurrence, because at best it's an unnecessary
> compare. Why waste time trying to defend or attack a line of code that simply
> shouldn't exist?

Not attacking anything. But the commit message wasn't as clear and I
ended up looking all the code. Chances are this is going to happen if
someone reads that commit message in a few moths. So something along
the lines of "This not currently a problem but ...." etc is what we
should have in that commit.

Cheers
/Ilias
>
> > On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
> >>
> >> From: Randolph Sapp <rs@ti.com>
> >>
> >> I don't even want to think about the possibility of this pointer
> >> containing a reference to something from a previous iteration.
> >>
> >> 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: [PATCH 6/6] memory: reserve from start_addr_sp to relocaddr

[Randolph Sapp]

On Thu Apr 2, 2026 at 4:21 AM CDT, Ilias Apalodimas wrote:
> On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>>
>> From: Randolph Sapp <rs@ti.com>
>>
>> Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to
>> gd->relocaddr instead of gd->ram_top. This allows platform specific
>> relocation addresses to work without unnecessarily painting over a large
>> range.
>
> This hangs when trying to boot with UEFI on QEMU aarch64.
> With the qemu_arm64_lwip_defconfig and this command line
>
> qemu-system-aarch64 -m 8192 -smp 2 -nographic -cpu cortex-a57 \
>     -machine virt,secure=off \
>     -bios u-boot.bin \
>     -device virtio-rng-pci \
>     -drive id=os,if=none,file=my.iso \
>     -device virtio-blk-device,drive=os \
>     -object memory-backend-ram,id=ram0,size=4G \
>     -object memory-backend-ram,id=ram1,size=4G \
>     -numa node,memdev=ram0 \
>     -numa node,memdev=ram1
>
> It also hangs when trying to initialize the EFI subsystem in general,
> e.g 'efidebug memmap', probably because some EFI code is relocated on
> the region you exlcuded.
>
> Thanks
> /Ilias

Thanks for the QEMU testcase. I see now there are a few cases where
gd->relocaddr can actually be decremented, meaning we won't actually cover the
entire u-boot memory region. I've added a new global data struct member
(end_addr_sp, but if anyone has any better name feel free to comment) to track
the original value of relocaddr set by setup_dest_addr and it works like a
charm.

I also realized that means I could cut out the PRAM and all of that bank
specific logic in lmb_reserve_uboot_region now, so that's cool.

>>
>> Signed-off-by: Randolph Sapp <rs@ti.com>
>> ---
>>  lib/efi_loader/efi_memory.c | 4 ++--
>>  lib/lmb.c                   | 2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index f07cc39b157..57691d15758 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -915,8 +915,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->relocaddr - 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 542bb11dcf5..0df8157db7f 100644
>> --- a/lib/lmb.c
>> +++ b/lib/lmb.c
>> @@ -540,7 +540,7 @@ static void lmb_reserve_uboot_region(void)
>>         ulong pram = 0;
>>
>>         rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE;
>> -       end = gd->ram_top;
>> +       end = gd->relocaddr;
>>
>>         /*
>>          * Reserve memory from aligned address below the bottom of U-Boot stack
>> --
>> 2.53.0
>>

Re: [PATCH 4/6] efi_memory: nitpick removal loop

[Randolph Sapp]

On Thu Apr 2, 2026 at 2:02 PM CDT, Ilias Apalodimas wrote:
> On Thu, 2 Apr 2026 at 20:39, Randolph Sapp <rs@ti.com> wrote:
>>
>> On Thu Apr 2, 2026 at 4:04 AM CDT, Ilias Apalodimas wrote:
>> > I don't mind merging this, but why is the current code flawed?
>> >
>> > Looking at the function even if prevmem contains a previous valid
>> > entry the check will fail
>> >
>> > if ((desc_get_end(cur) == prev->physical_start) &&
>> >     (prev->type == cur->type) &&
>> >     (prev->attribute == cur->attribute)) {
>> >
>> > and re-assign prevmem.
>> >
>> > So please adjust the commit message if you want this merged.
>> >
>> > Thanks
>> > /Ilias
>>
>> Ah, not really that it is "flawed" right now, but more so that we should never
>> even have to think about that occurrence, because at best it's an unnecessary
>> compare. Why waste time trying to defend or attack a line of code that simply
>> shouldn't exist?
>
> Not attacking anything. But the commit message wasn't as clear and I
> ended up looking all the code. Chances are this is going to happen if
> someone reads that commit message in a few moths. So something along
> the lines of "This not currently a problem but ...." etc is what we
> should have in that commit.
>
> Cheers
> /Ilias

I didn't mean for it to sound as if you were attacking it. Change requests (like
this patch) are always the "attack." I'm just being lazy and saying I didn't
want to parse this and come up with some specific scenario where it could be a
problem.

I'll adjust the description to indicate the *usually* unnecessary comparison
that's occurring right now. Should be a good enough justification on it's own.

>>
>> > On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>> >>
>> >> From: Randolph Sapp <rs@ti.com>
>> >>
>> >> I don't even want to think about the possibility of this pointer
>> >> containing a reference to something from a previous iteration.
>> >>
>> >> 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: [PATCH 5/6] efi_memory: backfill EFI_CONVENTIONAL_MEMORY

[Randolph Sapp]

On Thu Apr 2, 2026 at 12:32 PM CDT, Randolph Sapp wrote:
> On Thu Apr 2, 2026 at 3:53 AM CDT, Ilias Apalodimas wrote:
>> Hi Randolph
>>
>> On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>>>
>>> From: Randolph Sapp <rs@ti.com>
>>>
>>> Backfill any fragmented EFI_CONVENTIONAL_MEMORY when LMB allocations
>>> start to fail. This may be a low memory environment, or maybe LMB is
>>> running up against a tricky reserved region.
>>>
>>> Signed-off-by: Randolph Sapp <rs@ti.com>
>>> ---
>>>  lib/efi_loader/efi_memory.c | 35 +++++++++++++++++++++++++++++++++--
>>>  1 file changed, 33 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index 882366a9f8a..f07cc39b157 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -441,6 +441,27 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated)
>>>         return EFI_NOT_FOUND;
>>>  }
>>>
>>> +/**
>>> + * efi_get_conventional_start - find the highest conventional region start
>>> + *                              address with at least the specified number of
>>> + *                              pages
>>> + *
>>> + * @pages:                      number of pages required to be in that carveout
>>> + * Return:                      starting address of the give carveout
>>> + */
>>> +static u64 efi_get_conventional_start(u64 pages)
>>> +{
>>> +       struct efi_mem_list *item;
>>> +
>>> +       list_for_each_entry(item, &efi_mem, link) {
>>> +               if (item->desc.type != EFI_CONVENTIONAL_MEMORY)
>>> +                       continue;
>>> +               if (item->desc.num_pages >= pages)
>>> +                       return item->desc.physical_start;
>>> +       }
>>> +       return 0;
>>
>> We've been bitten by this in the past and although unlikely 0 is a
>> valid address for some hardware. Can we return something different if
>> we don't find a match? UINT64_MAX perhaps?
>>
>>> +}
>>> +
>>>  /**
>>>   * efi_allocate_pages - allocate memory pages
>>>   *
>>> @@ -507,10 +528,20 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
>>>         /* Reserve that map in our memory maps */
>>>         ret = efi_update_memory_map(efi_addr, pages, memory_type, true, false);
>>
>> In theory the lmb and the EFI maps are in sync. I haven't checked
>> close enough yet, but do you have cases where lmb_alloc worked and
>> updating the efi memory map failed?
>
> Fair point. Now that I've fixed the FDT warning I should probably check if I can
> actually reproduce the issue that actually required this.

Yeah, this patch isn't necessary anymore. I suppose it should be dropped as it
would potentially mask any discrepancies between the two maps.

Out of curiosity, is there any plan to merge the EFI and LMB allocators more in
the future, or is the current layering scheme the furthest we want to go?

>>>         if (ret != EFI_SUCCESS) {
>>> -               /* Map would overlap, bail out */
>>> +               /* Map would overlap, try something else */
>>>                 lmb_free(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
>>>                 unmap_sysmem((void *)(uintptr_t)efi_addr);
>>> -               return  EFI_OUT_OF_RESOURCES;
>>> +
>>> +               /* See if there is any EFI_CONVENTIONAL_MEMORY allocations */
>>> +               if (type != EFI_ALLOCATE_ADDRESS) {
>>
>> Can you please inverse this. It's going to reduce the identation.
>> if (type == EFI_ALLOCATE_ADDRESS)
>>> +                       *memory = efi_get_conventional_start(pages);
>>> +                       if (*memory != 0)
>>
>> Same here
>>
>>> +                               return efi_allocate_pages(EFI_ALLOCATE_ADDRESS,
>>> +                                                         memory_type, pages,
>>> +                                                         memory);
>>> +               }
>>> +
>>> +               return EFI_OUT_OF_RESOURCES;
>>>         }
>>>
>>>         *memory = efi_addr;
>>> --
>>> 2.53.0
>>>
>>
>> Thanks
>> /Ilias

Re: [PATCH 5/6] efi_memory: backfill EFI_CONVENTIONAL_MEMORY

[Ilias Apalodimas]

[...]

> >>
> >>> +}
> >>> +
> >>>  /**
> >>>   * efi_allocate_pages - allocate memory pages
> >>>   *
> >>> @@ -507,10 +528,20 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
> >>>         /* Reserve that map in our memory maps */
> >>>         ret = efi_update_memory_map(efi_addr, pages, memory_type, true, false);
> >>
> >> In theory the lmb and the EFI maps are in sync. I haven't checked
> >> close enough yet, but do you have cases where lmb_alloc worked and
> >> updating the efi memory map failed?
> >
> > Fair point. Now that I've fixed the FDT warning I should probably check if I can
> > actually reproduce the issue that actually required this.
>
> Yeah, this patch isn't necessary anymore. I suppose it should be dropped as it
> would potentially mask any discrepancies between the two maps.

Yea and if we really need a similar fix in the future, it's best if we
put in the lmb core code.

>
> Out of curiosity, is there any plan to merge the EFI and LMB allocators more in
> the future, or is the current layering scheme the furthest we want to go?

We do, but havent due to lack of time. It's been a while since I
discussed this with Heinrich but iirc the only thing missing is for
lmb to be aware of the memory flags EFI expects.

/Ilias
>
> >>>         if (ret != EFI_SUCCESS) {
> >>> -               /* Map would overlap, bail out */
> >>> +               /* Map would overlap, try something else */
> >>>                 lmb_free(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
> >>>                 unmap_sysmem((void *)(uintptr_t)efi_addr);
> >>> -               return  EFI_OUT_OF_RESOURCES;
> >>> +
> >>> +               /* See if there is any EFI_CONVENTIONAL_MEMORY allocations */
> >>> +               if (type != EFI_ALLOCATE_ADDRESS) {
> >>
> >> Can you please inverse this. It's going to reduce the identation.
> >> if (type == EFI_ALLOCATE_ADDRESS)
> >>> +                       *memory = efi_get_conventional_start(pages);
> >>> +                       if (*memory != 0)
> >>
> >> Same here
> >>
> >>> +                               return efi_allocate_pages(EFI_ALLOCATE_ADDRESS,
> >>> +                                                         memory_type, pages,
> >>> +                                                         memory);
> >>> +               }
> >>> +
> >>> +               return EFI_OUT_OF_RESOURCES;
> >>>         }
> >>>
> >>>         *memory = efi_addr;
> >>> --
> >>> 2.53.0
> >>>
> >>
> >> Thanks
> >> /Ilias
>

Re: [PATCH 5/6] efi_memory: backfill EFI_CONVENTIONAL_MEMORY

[Randolph Sapp]

On Fri Apr 3, 2026 at 12:28 AM CDT, Ilias Apalodimas wrote:
> [...]
>
>> >>
>> >>> +}
>> >>> +
>> >>>  /**
>> >>>   * efi_allocate_pages - allocate memory pages
>> >>>   *
>> >>> @@ -507,10 +528,20 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
>> >>>         /* Reserve that map in our memory maps */
>> >>>         ret = efi_update_memory_map(efi_addr, pages, memory_type, true, false);
>> >>
>> >> In theory the lmb and the EFI maps are in sync. I haven't checked
>> >> close enough yet, but do you have cases where lmb_alloc worked and
>> >> updating the efi memory map failed?
>> >
>> > Fair point. Now that I've fixed the FDT warning I should probably check if I can
>> > actually reproduce the issue that actually required this.
>>
>> Yeah, this patch isn't necessary anymore. I suppose it should be dropped as it
>> would potentially mask any discrepancies between the two maps.
>
> Yea and if we really need a similar fix in the future, it's best if we
> put in the lmb core code.
>
>>
>> Out of curiosity, is there any plan to merge the EFI and LMB allocators more in
>> the future, or is the current layering scheme the furthest we want to go?
>
> We do, but havent due to lack of time. It's been a while since I
> discussed this with Heinrich but iirc the only thing missing is for
> lmb to be aware of the memory flags EFI expects.
>
> /Ilias

I mean, trimming the last x bits off of the LMB allocation flag value to store
the current EFI enum is a little bit of a hack, but honestly it'll allow the
current allocation coalescence logic to continue to work as-is. It would just
need helpers to store and retrieve the enum type for the code that actually
cares about the EFI memory types. I could look into doing that if you guys think
that's a valid approach, or if you've got other ideas.

>>
>> >>>         if (ret != EFI_SUCCESS) {
>> >>> -               /* Map would overlap, bail out */
>> >>> +               /* Map would overlap, try something else */
>> >>>                 lmb_free(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
>> >>>                 unmap_sysmem((void *)(uintptr_t)efi_addr);
>> >>> -               return  EFI_OUT_OF_RESOURCES;
>> >>> +
>> >>> +               /* See if there is any EFI_CONVENTIONAL_MEMORY allocations */
>> >>> +               if (type != EFI_ALLOCATE_ADDRESS) {
>> >>
>> >> Can you please inverse this. It's going to reduce the identation.
>> >> if (type == EFI_ALLOCATE_ADDRESS)
>> >>> +                       *memory = efi_get_conventional_start(pages);
>> >>> +                       if (*memory != 0)
>> >>
>> >> Same here
>> >>
>> >>> +                               return efi_allocate_pages(EFI_ALLOCATE_ADDRESS,
>> >>> +                                                         memory_type, pages,
>> >>> +                                                         memory);
>> >>> +               }
>> >>> +
>> >>> +               return EFI_OUT_OF_RESOURCES;
>> >>>         }
>> >>>
>> >>>         *memory = efi_addr;
>> >>> --
>> >>> 2.53.0
>> >>>
>> >>
>> >> Thanks
>> >> /Ilias
>>