Re: [RFC PATCH 04/31] lmb: remove local instances of the lmb structure variable

[Heinrich Schuchardt]

Am 12. Juni 2024 04:41:39 MESZ schrieb Simon Glass <sjg at chromium.org>:
>Hi Tom,
>
>On Tue, 11 Jun 2024 at 16:55, Tom Rini <trini at konsulko.com> wrote:
>>
>> On Tue, Jun 11, 2024 at 04:08:56PM -0600, Simon Glass wrote:
>> > Hi Tom,
>> >
>> > On Tue, 11 Jun 2024 at 15:01, Tom Rini <trini at konsulko.com> wrote:
>> > >
>> > > On Tue, Jun 11, 2024 at 12:52:22PM -0600, Simon Glass wrote:
>> > > > Hi Sughosh,
>> > > >
>> > > > On Fri, 7 Jun 2024 at 12:53, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>> > > > >
>> > > > > With the move of the LMB structure to a persistent state, there is no
>> > > > > need to declare the variable locally, and pass it as part of the LMB
>> > > > > API's. Remove all local variable instances and change the API's
>> > > > > correspondingly.
>> > > > >
>> > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
>> > > > > ---
>> > > > >  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    |   7 +-
>> > > > >  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                |   9 +-
>> > > > >  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          |   7 +-
>> > > > >  boot/bootm.c                         |  26 ++--
>> > > > >  boot/bootm_os.c                      |   5 +-
>> > > > >  boot/image-board.c                   |  32 ++---
>> > > > >  boot/image-fdt.c                     |  29 ++---
>> > > > >  cmd/bdinfo.c                         |   6 +-
>> > > > >  cmd/booti.c                          |   2 +-
>> > > > >  cmd/bootz.c                          |   2 +-
>> > > > >  cmd/load.c                           |   7 +-
>> > > > >  drivers/iommu/apple_dart.c           |   7 +-
>> > > > >  drivers/iommu/sandbox_iommu.c        |  15 +--
>> > > > >  fs/fs.c                              |   7 +-
>> > > > >  include/image.h                      |  22 +---
>> > > > >  include/lmb.h                        |  39 +++---
>> > > > >  lib/lmb.c                            |  81 ++++++------
>> > > > >  net/tftp.c                           |   5 +-
>> > > > >  net/wget.c                           |   5 +-
>> > > > >  test/cmd/bdinfo.c                    |   2 +-
>> > > > >  test/lib/lmb.c                       | 187 +++++++++++++--------------
>> > > > >  36 files changed, 270 insertions(+), 333 deletions(-)
>> > > >
>> > > > This isn't necessary...and it will make things harder. You can have a
>> > > > global 'lmb' while still allowing passing a different pointer when
>> > > > needed.
>> > >
>> > > There's only one reservation checking system and list of known
>> > > reservations, keep in mind.
>> >
>> > There is only one driver model, too, but we use a pointer. It makes
>> > tests much easier.
>> >
>> > In fact I see elsewhere in this series that it causes problems with
>> > tests. Best to use a pointer so it is easy to update.
>>
>> Maybe? I worry that will lead to thinking that we still, like today,
>> have many LMB lists, rather than a single LMB list that everyone must
>> use.
>
>Do people worry about that with driver model?
>
>Also IMO there is only really one LMB list today. We create it at the
>start of bootm and then it is done when we boot. The file-loading
>stuff is what makes all this confusing...and with bootstd that is
>under control as well.

As we have a command line interface bootstd is not the only case to consider. We should not start making assumptions about the sequence of commands entered manually.

Best regards

Heinrich

>
>At lot of this effort seems to be about dealing with random scripts
>which load things. We want to make sure we complain if something
>overlaps. But we should be making the bootstd case work nicely and
>doing things within that framework. Also EFI sort-of has its own
>thing, which it is very-much in control of.
>
>Overall I think this is a bit more subtle that just combining allocators.
>
>Regards,
>Simon