hi Simon, On Sat, 13 Jul 2024 at 20:45, Simon Glass <s...@chromium.org> wrote: > > Hi Sughosh, > > On Thu, 4 Jul 2024 at 08:36, Sughosh Ganu <sughosh.g...@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.g...@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 ?
Isn't LMB_NOOVERWRITE more suitable here ? I mean this is indicating that the said region of memory is not to be re-used/re-requested. > > These could be adjusted to use BIT() I am changing these to use the BIT macro in a subsequent commit. > > > }; > > > > /** > > 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 Okay > > > + * 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 Okay -sughosh > > > { > > 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