hi Simon, On Sat, 13 Jul 2024 at 20:46, Simon Glass <s...@chromium.org> wrote: > > Hi Sughosh, > > On Thu, 4 Jul 2024 at 08:36, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > The current LMB API's for allocating and reserving memory use a > > per-caller based memory view. Memory allocated by a caller can then be > > overwritten by another caller. Make these allocations and reservations > > persistent using the alloced list data structure. > > > > Two alloced lists are declared -- one for the available(free) memory, > > and one for the used memory. Once full, the list can then be extended > > at runtime. > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > --- > > Changes since V1: > > * Use alloced list structure for the available and reserved memory > > lists instead of static arrays. > > * Corresponding changes in the code made as a result of the above > > change. > > * Rename the reserved memory list as 'used'. > > > > include/lmb.h | 77 +++-------- > > lib/lmb.c | 346 ++++++++++++++++++++++++++++++-------------------- > > 2 files changed, 224 insertions(+), 199 deletions(-) > > > > diff --git a/include/lmb.h b/include/lmb.h > > index 99fcf5781f..27cdb18c37 100644 > > --- a/include/lmb.h > > +++ b/include/lmb.h > > @@ -24,78 +24,18 @@ enum lmb_flags { > > }; > > > > /** > > - * struct lmb_property - Description of one region. > > + * struct lmb_region - Description of one region. > > * > > * @base: Base address of the region. > > * @size: Size of the region > > * @flags: memory region attributes > > */ > > -struct lmb_property { > > +struct lmb_region { > > phys_addr_t base; > > phys_size_t size; > > enum lmb_flags flags; > > }; > > > > -/* > > - * For regions size management, see LMB configuration in KConfig > > - * all the #if test are done with CONFIG_LMB_USE_MAX_REGIONS (boolean) > > - * > > - * case 1. CONFIG_LMB_USE_MAX_REGIONS is defined (legacy mode) > > - * => CONFIG_LMB_MAX_REGIONS is used to configure the region size, > > - * directly in the array lmb_region.region[], with the same > > - * configuration for memory and reserved regions. > > - * > > - * case 2. CONFIG_LMB_USE_MAX_REGIONS is not defined, the size of each > > - * region is configurated *independently* with > > - * => CONFIG_LMB_MEMORY_REGIONS: struct lmb.memory_regions > > - * => CONFIG_LMB_RESERVED_REGIONS: struct lmb.reserved_regions > > - * lmb_region.region is only a pointer to the correct buffer, > > - * initialized in lmb_init(). This configuration is useful to > > manage > > - * more reserved memory regions with CONFIG_LMB_RESERVED_REGIONS. > > - */ > > - > > -/** > > - * struct lmb_region - Description of a set of region. > > - * > > - * @cnt: Number of regions. > > - * @max: Size of the region array, max value of cnt. > > - * @region: Array of the region properties > > - */ > > -struct lmb_region { > > - unsigned long cnt; > > - unsigned long max; > > -#if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) > > - struct lmb_property region[CONFIG_LMB_MAX_REGIONS]; > > -#else > > - struct lmb_property *region; > > -#endif > > -}; > > - > > -/** > > - * struct lmb - Logical memory block handle. > > - * > > - * Clients provide storage for Logical memory block (lmb) handles. > > - * The content of the structure is managed by the lmb library. > > - * A lmb struct is initialized by lmb_init() functions. > > - * The lmb struct is passed to all other lmb APIs. > > - * > > - * @memory: Description of memory regions. > > - * @reserved: Description of reserved regions. > > - * @memory_regions: Array of the memory regions (statically allocated) > > - * @reserved_regions: Array of the reserved regions (statically allocated) > > - */ > > -struct lmb { > > - struct lmb_region memory; > > - struct lmb_region reserved; > > -#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) > > - struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS]; > > - struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS]; > > -#endif > > -}; > > - > > -void lmb_init_and_reserve(struct bd_info *bd, void *fdt_blob); > > -void lmb_init_and_reserve_range(phys_addr_t base, phys_size_t size, > > - void *fdt_blob); > > long lmb_add(phys_addr_t base, phys_size_t size); > > long lmb_reserve(phys_addr_t base, phys_size_t size); > > /** > > @@ -134,6 +74,19 @@ void board_lmb_reserve(void); > > void arch_lmb_reserve(void); > > void arch_lmb_reserve_generic(ulong sp, ulong end, ulong align); > > > > +/** > > + * lmb_mem_regions_init() - Initialise the LMB memory > > + * > > + * Initialise the LMB subsystem related data structures. There are two > > + * alloced lists that are initialised, one for the free memory, and one > > + * for the used memory. > > + * > > + * Initialise the two lists as part of board init. > > + * > > + * Return: 0 if OK, -ve on failure. > > + */ > > +int lmb_mem_regions_init(void); > > + > > #endif /* __KERNEL__ */ > > > > #endif /* _LINUX_LMB_H */ > > diff --git a/lib/lmb.c b/lib/lmb.c > > index 80945e3cae..a46bc8a7a3 100644 > > --- a/lib/lmb.c > > +++ b/lib/lmb.c > > @@ -6,6 +6,7 @@ > > * Copyright (C) 2001 Peter Bergner. > > */ > > > > +#include <alist.h> > > #include <efi_loader.h> > > #include <image.h> > > #include <mapmem.h> > > @@ -15,24 +16,30 @@ > > > > #include <asm/global_data.h> > > #include <asm/sections.h> > > +#include <linux/kernel.h> > > > > DECLARE_GLOBAL_DATA_PTR; > > > > #define LMB_ALLOC_ANYWHERE 0 > > +#define LMB_ALIST_INITIAL_SIZE 4 > > > > -static void lmb_dump_region(struct lmb_region *rgn, char *name) > > +struct alist lmb_free_mem; > > +struct alist lmb_used_mem; > > I think these should be in a struct, e.g. struct lmb, allocated with > malloc() and pointed to by gd->lmb so we can avoid making the tests > destructive, and allow use of lmb in SPL.
Can you elaborate on the point of allowing use of lmb in SPL. Why would the current design not work in SPL ? I tested this on the sandbox SPL variant, and the two lists do get initialised as part of the SPL initialisation routine. Is there some corner-case that I am not considering ? Also, regarding putting the lmb structure pointer as part of gd, iirc Tom had a different opinion on this. Tom, can you please chime in here ? > There is a > arch_reset_for_test() which can reset the lmb back to its original > state, or perhaps just swap the pointer for tests. > > I know this series is already long, but this patch seems to do quite a > bit. Perhaps it could be split into two? Sorry, do you mean the patch, or the series. If the later, I am going to split the non-rfc version which comes next into two parts. The first part being the lmb changes. -sughosh > > Regards, > Simon