[PATCH 2/6] lmb: Return -EEXIST in lmb_add_region_flags() if region already added
Sam Protsenko
semen.protsenko at linaro.org
Sun Dec 8 01:21:17 CET 2024
An attempt to add the already added LMB region (with exactly the same
start address, size and flags) using lmb_add_region_flags() ends up in
lmb_addrs_overlap() check, which eventually leads to either returning 0
if 'flags' is LMB_NONE, or -1 otherwise. It makes it impossible for the
user of this function to catch the case when the region is already added
and differentiate it from regular errors. That in turn may lead to
incorrect error handling in the caller code, like reporting misleading
errors or interrupting the normal code path where it could be treated as
the normal case. An example is boot_fdt_reserve_region() function, which
might be called twice (e.g. during board startup in initr_lmb(), and
then during 'booti' command booting the OS), thus trying to reserve
exactly the same memory regions described in device tree twice, which
produces an error message on second call.
Implement the detection of cases when the already added region is trying
to be added again, and return -EEXIST error code in case the region
exists and it's not LMB_NONE; for LMB_NONE return 0, to conform to unit
tests (specifically test_alloc_addr() in test/lib/lmb.c) and the
preferred behavior described in commit 1d9aa4a283da ("lmb: Fix the
allocation of overlapping memory areas with !LMB_NONE"). The change of
lmb_add_region_flags() return values is described in the table below:
Return case Pre-1d9 1d9 New
-----------------------------------------------------------
Added successfully 0 0 0
Failed to add -1 -1 -1
Already added, flags == LMB_NONE 0 0 0
Already added, flags != LMB_NONE 0 -1 -EEXIST
Rework all affected functions and their documentation. Also fix the
corresponding unit test which checks reserving the same region with the
same flags to account for the changed return value.
No functional change is intended (by this patch itself).
Fixes: 1d9aa4a283da ("lmb: Fix the allocation of overlapping memory areas with !LMB_NONE")
Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
---
lib/lmb.c | 18 ++++++++++++++----
test/lib/lmb.c | 2 +-
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index 713f072f75ee..ce0dc49345fb 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -183,8 +183,10 @@ static long lmb_resize_regions(struct alist *lmb_rgn_lst,
* the function might resize an already existing region or coalesce two
* adjacent regions.
*
- *
- * Returns: 0 if the region addition successful, -1 on failure
+ * Return:
+ * * %0 - Added successfully, or it's already added (only if LMB_NONE)
+ * * %-EEXIST - The region is already added, and flags != LMB_NONE
+ * * %-1 - Failure
*/
static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base,
phys_size_t size, enum lmb_flags flags)
@@ -202,6 +204,14 @@ static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base,
phys_size_t rgnsize = rgn[i].size;
enum lmb_flags rgnflags = rgn[i].flags;
+ /* Already have this region, so we're done */
+ if (base == rgnbase && size == rgnsize && flags == rgnflags) {
+ if (flags == LMB_NONE)
+ return 0;
+ else
+ return -EEXIST;
+ }
+
ret = lmb_addrs_adjacent(base, size, rgnbase, rgnsize);
if (ret > 0) {
if (flags != rgnflags)
@@ -667,7 +677,7 @@ long lmb_add(phys_addr_t base, phys_size_t size)
*
* Free up a region of memory.
*
- * Return: 0 if successful, -1 on failure
+ * Return: 0 if successful, negative error code on failure
*/
long lmb_free_flags(phys_addr_t base, phys_size_t size,
uint flags)
@@ -818,7 +828,7 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size,
lmb_memory[rgn].size,
base + size - 1, 1)) {
/* ok, reserve the memory */
- if (lmb_reserve_flags(base, size, flags) >= 0)
+ if (!lmb_reserve_flags(base, size, flags))
return base;
}
}
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index 0bd29e2a4fe7..48c3c966f8f2 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -754,7 +754,7 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
/* reserve again, same flag */
ret = lmb_reserve_flags(0x40010000, 0x10000, LMB_NOMAP);
- ut_asserteq(ret, -1L);
+ ut_asserteq(ret, -EEXIST);
ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x10000,
0, 0, 0, 0);
--
2.39.5
More information about the U-Boot
mailing list