lmb: Return -EEXIST in lmb_add_region_flags() if region already added

An attempt to add the already added LMB region 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 the device tree twice, which produces an error message on
second call.

Return -EEXIST error code in case when the added 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@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Tested-by: Michal Simek <michal.simek@amd.com>

diff --git a/lib/lmb.c b/lib/lmb.c
index b03237bc06cba1919e2b88800076adaf22536061..a695edf70dfacaa6504bdfe6761a79d74e4bdf04 100644 (file)
--- 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)
@@ -217,17 +219,15 @@ static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base,
                        coalesced++;
                        break;
                } else if (lmb_addrs_overlap(base, size, rgnbase, rgnsize)) {
-                       if (flags == LMB_NONE) {
-                               ret = lmb_resize_regions(lmb_rgn_lst, i, base,
-                                                        size);
-                               if (ret < 0)
-                                       return -1;
+                       if (flags != LMB_NONE)
+                               return -EEXIST;
 
-                               coalesced++;
-                               break;
-                       } else {
+                       ret = lmb_resize_regions(lmb_rgn_lst, i, base, size);
+                       if (ret < 0)
                                return -1;
-                       }
+
+                       coalesced++;
+                       break;
                }
        }
 
@@ -667,7 +667,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 +818,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 0bd29e2a4fe710bc672ac041cb7fb4e6f5fcea39..48c3c966f8f27618c8f44927b46e6fd30f88f4b4 100644 (file)
--- 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);