Hi Sam,
On Sun, 8 Dec 2024 at 02:21, Sam Protsenko semen.protsenko@linaro.org wrote:
Since commit 1d9aa4a283da ("lmb: Fix the allocation of overlapping memory areas with !LMB_NONE") the lmb_add_region_flags() returns -1 when the caller tries to add the already existing region with !LMB_NONE flags (it was returning 0 before that patch). That causes boot_fdt_reserve_region() function to print erroneous error messages when it's called consequently more than one time.
Make lmb_add_region_flags() return -EEXIST when the already added region with !LMB_NONE flags is being added, and then check that error code in boot_fdt_reserve_region() to avoid printing the misleading error messages.
Thanks for cathing this. It's a print only error, but still misleading
I didn't want to change lmb_add_region_flags() behavior back to always returning 0 on attempts to add already existing regions with !LMB_NONE flags, as the unit tests in test/lib/lmb.c would break, and also commit 1d9aa4a283da ("lmb: Fix the allocation of overlapping memory areas with !LMB_NONE") specifically states that behavior is expected and required for efi_allocate_pages() calls with type=EFI_ALLOCATE_ADDRESS.
TBH it's not only the EFI code that needs this. I don't think returning 0 for a region that's already allocated and the flags that happen to match is a good idea to begin with. The code should be as precise as possible.
Thanks /Ilias
All unit tests pass in sandbox U-Boot, with a small test modification in [PATCH 2/6]. I also made a bit of LMB cleanups in this series, while at it. Only [PATCH 6/6] introduces an actual functional change.
Sam Protsenko (6): lmb: Fix flags data type in lmb_add_region_flags() lmb: Return -EEXIST in lmb_add_region_flags() if region already added lmb: Make const flag_str[] in lmb_print_region_flags() more const lmb: Improve coding style lmb: Improve kernel-doc comments boot: fdt: Handle already reserved memory in boot_fdt_reserve_region()
boot/image-fdt.c | 2 +- include/lmb.h | 125 ++++++++++++++++++++++++++++------------------- lib/lmb.c | 105 +++++++++++++-------------------------- test/lib/lmb.c | 2 +- 4 files changed, 109 insertions(+), 125 deletions(-)
-- 2.39.5