[PATCH v4 05/27] lmb: make LMB memory map persistent and global

[Sughosh Ganu]

On Mon, 16 Sept 2024 at 11:34, Vaishnav Achath <vaishnav.a at ti.com> wrote:
>
> Hi Sughosh
>
> On 26/08/24 17:29, Sughosh Ganu 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.ganu at linaro.org>
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > [sjg: Optimise the logic to add a region in lmb_add_region_flags()]
> > [sjg: Use a stack to store pointer of lmb struct when running lmb tests]
> > ---
> > Changes since V3:
> > * Fix checkpatch warnings of spaces between function name and
> >    open parantheses.
> > * s/uint64_t/u64 as suggested by checkpatch.
> > * Remove unneccessary parantheses in lmb.c as suggested by checkpatch.
> > * Fix alignment in test/cmd/bdinfo.c as suggested by checkpatch.
> >
> >   arch/arc/lib/cache.c                 |   4 +-
> >   arch/arm/lib/stack.c                 |   4 +-
> >   arch/arm/mach-apple/board.c          |  17 +-
> >   arch/arm/mach-snapdragon/board.c     |  17 +-
> >   arch/arm/mach-stm32mp/dram_init.c    |   8 +-
> >   arch/arm/mach-stm32mp/stm32mp1/cpu.c |   6 +-
> >   arch/m68k/lib/bootm.c                |   7 +-
> >   arch/microblaze/lib/bootm.c          |   4 +-
> >   arch/mips/lib/bootm.c                |  11 +-
> >   arch/nios2/lib/bootm.c               |   4 +-
> >   arch/powerpc/cpu/mpc85xx/mp.c        |   4 +-
> >   arch/powerpc/include/asm/mp.h        |   4 +-
> >   arch/powerpc/lib/bootm.c             |  14 +-
> >   arch/riscv/lib/bootm.c               |   4 +-
> >   arch/sh/lib/bootm.c                  |   4 +-
> >   arch/x86/lib/bootm.c                 |   4 +-
> >   arch/xtensa/lib/bootm.c              |   4 +-
> >   board/xilinx/common/board.c          |   8 +-
> >   boot/bootm.c                         |  26 +-
> >   boot/bootm_os.c                      |   5 +-
> >   boot/image-board.c                   |  34 +-
> >   boot/image-fdt.c                     |  35 +-
> >   cmd/bdinfo.c                         |   6 +-
> >   cmd/booti.c                          |   2 +-
> >   cmd/bootz.c                          |   2 +-
> >   cmd/elf.c                            |   2 +-
> >   cmd/load.c                           |   7 +-
> >   drivers/iommu/apple_dart.c           |   8 +-
> >   drivers/iommu/sandbox_iommu.c        |  16 +-
> >   fs/fs.c                              |   7 +-
>
> In fs the reserved region is not being freed after read, while
> other loaders free them (cmd/load.c), this patch uncovers the issue
> since now other loaders cannot use the same memory location for load.
> For example now someone cannot do:

The idea with the LMB memory is that it is not really an allocation,
but setting aside memory for use. Now there was a discussion earlier
on the mailing list if this is actually an allocation or not, but this
is what the functions have been called from it's early days. But the
way the code is designed now, even with the global and persistent
memory map, we have the LMB_NONE flag which is used to allow for the
same memory region to be re-allocated/re-reserved.

>
> mmc load .. $loadaddr ...
> <do something with above contents>
> tftp $loadaddr ..

The issue above is what I mentioned to Prasad in one of my earlier
replies to the patch that he had sent [1]. What can be done is to
unify the manner in which callers ask for LMB memory -- that would
mean changing the behaviour of the tftp code to use the logic used in
the fs_read_lmb_check() function. I believe this method of loading to
an address is more beneficial as it allows memory re-use.

-sughosh

[1] - https://lists.denx.de/pipermail/u-boot/2024-September/564610.html

>
> where this worked previously since it was not global, I have sent a
> fix for this :
> https://lore.kernel.org/all/20240916054038.2125910-1-vaishnav.a@ti.com/
>
> Thanks and Regards,
> Vaishnav
>
> >   include/image.h                      |  28 +-
> >   include/lmb.h                        | 111 ++----
> >   lib/efi_loader/efi_dt_fixup.c        |   2 +-
> >   lib/efi_loader/efi_helper.c          |   2 +-
> >   lib/lmb.c                            | 446 +++++++++++++---------
> >   net/tftp.c                           |   5 +-
> >   net/wget.c                           |   5 +-
> >   test/cmd/bdinfo.c                    |  41 +-
> >   test/lib/lmb.c                       | 542 +++++++++++++--------------
> >   39 files changed, 714 insertions(+), 746 deletions(-)
> >
>