[PATCH 0/6] lmb: Fix reserving the same region multiple times

[Ilias Apalodimas]

Hi Sam,

On Sun, 8 Dec 2024 at 02:21, Sam Protsenko <semen.protsenko at 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
>