HI Sughosh,
On Fri, 7 Jun 2024 at 12:53, 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. With this, memory allocated or reserved will not be
> overwritten.
>
> Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org>
> ---
>
> Note: @Mark Kettenis, please review the changes made in the apple dart
> driver. I have removed the driver-local LMB instance, but I am not
> sure if the current logic makes sense. I would think that it would be
> possible to simply use memory region allocated by the LMB module(maybe
> using the max address argument), instead of adding specific memory
> region with lmb_add().
>
> arch/arm/mach-stm32mp/dram_init.c | 1 -
> board/xilinx/common/board.c | 1 -
> drivers/iommu/apple_dart.c | 1 -
> drivers/iommu/sandbox_iommu.c | 1 -
> include/lmb.h | 17 ++----
> lib/lmb.c | 91 ++++++++++++++++---------------
> test/lib/lmb.c | 18 ------
> 7 files changed, 50 insertions(+), 80 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/dram_init.c
> b/arch/arm/mach-stm32mp/dram_init.c
> index fb1208fc5d..86d6577b35 100644
> --- a/arch/arm/mach-stm32mp/dram_init.c
> +++ b/arch/arm/mach-stm32mp/dram_init.c
> @@ -59,7 +59,6 @@ phys_addr_t board_get_usable_ram_top(phys_size_t total_size)
> gd->ram_top = clamp_val(gd->ram_top, 0, SZ_4G - 1);
>
> /* found enough not-reserved memory to relocated U-Boot */
> - lmb_init(&lmb);
> lmb_add(&lmb, gd->ram_base, gd->ram_top - gd->ram_base);
> boot_fdt_add_mem_rsv_regions(&lmb, (void *)gd->fdt_blob);
> /* add 8M for reserved memory for display, fdt, gd,... */
> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
> index b47d2d23f9..ebe57da7f8 100644
> --- a/board/xilinx/common/board.c
> +++ b/board/xilinx/common/board.c
> @@ -685,7 +685,6 @@ phys_addr_t board_get_usable_ram_top(phys_size_t
> total_size)
> panic("Not 64bit aligned DT location: %p\n", gd->fdt_blob);
>
> /* found enough not-reserved memory to relocated U-Boot */
> - lmb_init(&lmb);
> lmb_add(&lmb, gd->ram_base, gd->ram_size);
> boot_fdt_add_mem_rsv_regions(&lmb, (void *)gd->fdt_blob);
> size = ALIGN(CONFIG_SYS_MALLOC_LEN + total_size, MMU_SECTION_SIZE);
> diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c
> index 6ecd84303b..ef385d9c9f 100644
> --- a/drivers/iommu/apple_dart.c
> +++ b/drivers/iommu/apple_dart.c
> @@ -214,7 +214,6 @@ static int apple_dart_probe(struct udevice *dev)
> priv->dvabase = DART_PAGE_SIZE;
> priv->dvaend = SZ_4G - DART_PAGE_SIZE;
>
> - lmb_init(&priv->lmb);
> lmb_add(&priv->lmb, priv->dvabase, priv->dvaend - priv->dvabase);
>
> /* Disable translations. */
> diff --git a/drivers/iommu/sandbox_iommu.c b/drivers/iommu/sandbox_iommu.c
> index 6ceb7fd5ec..31ce91345c 100644
> --- a/drivers/iommu/sandbox_iommu.c
> +++ b/drivers/iommu/sandbox_iommu.c
> @@ -55,7 +55,6 @@ static int sandbox_iommu_probe(struct udevice *dev)
> {
> struct sandbox_iommu_priv *priv = dev_get_priv(dev);
>
> - lmb_init(&priv->lmb);
> lmb_add(&priv->lmb, 0x89abc000, SZ_16K);
>
> return 0;
> diff --git a/include/lmb.h b/include/lmb.h
> index 7b87181b9e..bbe1c5299c 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -49,8 +49,7 @@ struct lmb_property {
> * => 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.
> + * initialized with these values.
> */
>
> /**
> @@ -73,26 +72,18 @@ struct lmb_region {
> /**
> * 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.
> + * The Logical Memory Block structure which is used to keep track
> + * of available memory which can be used for stuff like loading
> + * images(kernel, initrd, fdt).
> *
> * @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(struct lmb *lmb);
> void lmb_init_and_reserve(struct lmb *lmb, struct bd_info *bd, void
> *fdt_blob);
> void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
> phys_size_t size, void *fdt_blob);
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 4d39c0d1f9..7f34d4a8b0 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -20,6 +20,25 @@ DECLARE_GLOBAL_DATA_PTR;
>
> #define LMB_ALLOC_ANYWHERE 0
>
> +#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
> +
> +struct lmb lmb = {
> +#if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> + .memory.max = CONFIG_LMB_MAX_REGIONS,
> + .reserved.max = CONFIG_LMB_MAX_REGIONS,
> +#else
> + .memory.max = CONFIG_LMB_MEMORY_REGIONS,
> + .reserved.max = CONFIG_LMB_RESERVED_REGIONS,
> + .memory.region = memory_regions,
> + .reserved.region = reserved_regions,
> +#endif
> + .memory.cnt = 0,
> + .reserved.cnt = 0,
> +};
Please use a pointer in global_data, so we can handle tests.
Also for bootstd we likely want to create a new one and 'own
everything'. I haven't thought it through fully, though.
Regards,
Simon
