[RFC PATCH v2 12/48] lmb: allow for resizing lmb regions

[Simon Glass]

Hi Sughosh,

On Thu, 4 Jul 2024 at 08:36, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> Allow for resizing of LMB regions if the region attributes match. The
> current code returns a failure status on detecting an overlapping
> address. This worked up until now since the LMB calls were not
> persistent and global -- the LMB memory map was specific and private
> to a given caller of the LMB API's.
>
> With the change in the LMB code to make the LMB reservations
> persistent, there needs to be a check on whether the memory region can
> be resized, and then do it if so. To distinguish between memory that
> cannot be resized, add a new flag, LMB_NOOVERWRITE. Reserving a region
> of memory with this attribute would indicate that the region cannot be
> resized.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
> Changes since V1:
> * Do the check for the region flags in lmb_resize_regions() and
>   lmb_merge_overlap_regions() to decide on merging the overlapping
>   regions.
>
>  include/lmb.h |   1 +
>  lib/lmb.c     | 116 ++++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 103 insertions(+), 14 deletions(-)
>
> diff --git a/include/lmb.h b/include/lmb.h
> index 069c6af2a3..99fcf5781f 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -20,6 +20,7 @@
>  enum lmb_flags {
>         LMB_NONE                = 0x0,
>         LMB_NOMAP               = 0x4,
> +       LMB_NOOVERWRITE         = 0x8,

How about LMB_PERSIST ?

These could be adjusted to use BIT()

>  };
>
>  /**
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 0d01e58a46..80945e3cae 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -230,12 +230,88 @@ void lmb_init_and_reserve_range(phys_addr_t base, phys_size_t size,
>         lmb_reserve_common(fdt_blob);
>  }
>
> +static bool lmb_region_flags_match(struct lmb_region *rgn, unsigned long r1,

At some point the index should change to an int (or uint), rather than
unsigned long. I had a series that did that, but it died.

> +                                  enum lmb_flags flags)
> +{
> +       return rgn->region[r1].flags == flags;
> +}
> +
> +static long lmb_merge_overlap_regions(struct lmb_region *rgn, unsigned long i,
> +                                     phys_addr_t base, phys_size_t size,
> +                                     enum lmb_flags flags)
> +{
> +       phys_size_t rgnsize;
> +       unsigned long rgn_cnt, idx;
> +       phys_addr_t rgnbase, rgnend;
> +       phys_addr_t mergebase, mergeend;
> +
> +       rgn_cnt = 0;
> +       idx = i;
> +       /*
> +        * First thing to do is to identify how many regions does
> +        * the requested region overlap.

how many regions the requested region overlaps

> +        * If the flags match, combine all these overlapping
> +        * regions into a single region, and remove the merged
> +        * regions.
> +        */
> +       while (idx < rgn->cnt - 1) {
> +               rgnbase = rgn->region[idx].base;
> +               rgnsize = rgn->region[idx].size;
> +
> +               if (lmb_addrs_overlap(base, size, rgnbase,
> +                                     rgnsize)) {
> +                       if (!lmb_region_flags_match(rgn, idx, flags))
> +                               return -1;
> +                       rgn_cnt++;
> +                       idx++;
> +               }
> +       }
> +
> +       /* The merged region's base and size */
> +       rgnbase = rgn->region[i].base;
> +       mergebase = min(base, rgnbase);
> +       rgnend = rgn->region[idx].base + rgn->region[idx].size;
> +       mergeend = max(rgnend, (base + size));
> +
> +       rgn->region[i].base = mergebase;
> +       rgn->region[i].size = mergeend - mergebase;
> +
> +       /* Now remove the merged regions */
> +       while (--rgn_cnt)
> +               lmb_remove_region(rgn, i + 1);
> +
> +       return 0;
> +}
> +
> +static long lmb_resize_regions(struct lmb_region *rgn, unsigned long i,
> +                              phys_addr_t base, phys_size_t size,
> +                              enum lmb_flags flags)
> +{
> +       long ret = 0;
> +       phys_addr_t rgnend;
> +
> +       if (i == rgn->cnt - 1 ||
> +               base + size < rgn->region[i + 1].base) {
> +               if (!lmb_region_flags_match(rgn, i, flags))
> +                       return -1;
> +
> +               rgnend = rgn->region[i].base + rgn->region[i].size;
> +               rgn->region[i].base = min(base, rgn->region[i].base);
> +               rgnend = max(base + size, rgnend);
> +               rgn->region[i].size = rgnend - rgn->region[i].base;
> +       } else {
> +               ret = lmb_merge_overlap_regions(rgn, i, base, size, flags);
> +       }
> +
> +       return ret;
> +}
> +
>  /* This routine called with relocation disabled. */
>  static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
>                                  phys_size_t size, enum lmb_flags flags)

This could really use a function comment

>  {
>         unsigned long coalesced = 0;
> -       long adjacent, i;
> +       long ret, i;
>
>         if (rgn->cnt == 0) {
>                 rgn->region[0].base = base;
> @@ -260,23 +336,28 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
>                                 return -1; /* regions with new flags */
>                 }
>
> -               adjacent = lmb_addrs_adjacent(base, size, rgnbase, rgnsize);
> -               if (adjacent > 0) {
> +               ret = lmb_addrs_adjacent(base, size, rgnbase, rgnsize);
> +               if (ret > 0) {
>                         if (flags != rgnflags)
>                                 break;
>                         rgn->region[i].base -= size;
>                         rgn->region[i].size += size;
>                         coalesced++;
>                         break;
> -               } else if (adjacent < 0) {
> +               } else if (ret < 0) {
>                         if (flags != rgnflags)
>                                 break;
>                         rgn->region[i].size += size;
>                         coalesced++;
>                         break;
>                 } else if (lmb_addrs_overlap(base, size, rgnbase, rgnsize)) {
> -                       /* regions overlap */
> -                       return -1;
> +                       ret = lmb_resize_regions(rgn, i, base, size,
> +                                                flags);
> +                       if (ret < 0)
> +                               return -1;
> +
> +                       coalesced++;
> +                       break;
>                 }
>         }
>
> @@ -418,7 +499,7 @@ static phys_addr_t lmb_align_down(phys_addr_t addr, phys_size_t size)
>  }
>

Regards,
Simon