[PATCH 23/40] lmb: add a flags parameter to the API's

[Sughosh Ganu]

On Wed, 7 Aug 2024 at 03:21, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Mon, 5 Aug 2024 at 05:55, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > On Mon, 29 Jul 2024 at 20:56, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Mon, 29 Jul 2024 at 02:40, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > >
> > > > On Fri, 26 Jul 2024 at 05:02, Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Wed, 24 Jul 2024 at 00:04, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > >
> > > > > > Add a flags parameter to the LMB API functions. The parameter can then
> > > > > > be used to pass any other type of reservations or allocations needed
> > > > > > by the callers. These will be used in a subsequent set of changes for
> > > > > > allocation requests coming from the EFI subsystem.
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > > > ---
> > > > > > Changes since rfc: New patch
> > > > > >
> > > > > >  arch/arm/mach-apple/board.c       |  17 ++--
> > > > > >  arch/arm/mach-snapdragon/board.c  |   2 +-
> > > > > >  arch/arm/mach-stm32mp/dram_init.c |   4 +-
> > > > > >  arch/powerpc/cpu/mpc85xx/mp.c     |   2 +-
> > > > > >  arch/powerpc/lib/bootm.c          |   2 +-
> > > > > >  board/xilinx/common/board.c       |   4 +-
> > > > > >  boot/bootm.c                      |   5 +-
> > > > > >  boot/image-board.c                |  15 ++-
> > > > > >  boot/image-fdt.c                  |  15 +--
> > > > > >  cmd/booti.c                       |   2 +-
> > > > > >  cmd/bootz.c                       |   2 +-
> > > > > >  cmd/load.c                        |   4 +-
> > > > > >  drivers/iommu/apple_dart.c        |   6 +-
> > > > > >  drivers/iommu/sandbox_iommu.c     |   6 +-
> > > > > >  fs/fs.c                           |   2 +-
> > > > > >  include/lmb.h                     |  23 ++---
> > > > > >  lib/lmb.c                         |  48 ++++------
> > > > > >  test/lib/lmb.c                    | 150 +++++++++++++++---------------
> > > > > >  18 files changed, 150 insertions(+), 159 deletions(-)
> > > > >
> > > > > This negates any code-size advantage of dropping the lmb parameter.
> > > > >
> > > > > All of these are LMB_NONE. Can we have a separate function (e.g.
> > > > > lmb_alloc_type()) for when we actually need to specify the type?
> > > >
> > > > We will be passing different values when we call the LMB API's from
> > > > the EFI allocation function. This is only adding a parameter to the
> > > > allocation API's, which I believe is better than adding separate
> > > > functions which take a flag parameter only to be called from the EFI
> > > > subsystem.
> > >
> > > No i believe it is worse, unless there are a lot of such functions.
> > > The flags are a special case, not the common case.
> >
> > I have done some size impact tests on the two scenarios, one where we
> > have a common set of lmb allocation API functions, with an added flags
> > parameter, and second where we have separate API's to be called from
> > the EFI memory module. I have put out the results of the size impact
> > [1].
> >
> > You will see that with common API's, we are not losing much even on
> > boards with EFI_LOADER disabled. But otoh, on boards which have
> > EFI_LOADER enabled, the gains are pretty significant. I believe we
> > should reconsider using a common LMB API with the flags parameter.
>
> Thanks for looking at it.
>
> Did you do special versions of just lmb_alloc() and lmb_add() which
> call the flags versions? It seems that there is no size advantage with
> EFI_LOADER and only a small one with !EFI_LOADER. Can you please point
> me to the code?

For the separate API version, I introduced new versions
lmb_alloc_flags(), lmb_alloc_base_flags(), lmb_alloc_addr_flags() and
lmb_free_flags(), which are being called from the EFI memory module. I
have pushed the two branches [1] [2] on my github. Please take a look.

Btw, both these branches are based on your v5 of the alist patches,
and also incorporate the stack based implementation for running the
lmb tests. So except for either having common API's, or not, there are
no other differences between the two. Thanks.

-sughosh

[1] - https://github.com/sughoshg/u-boot/tree/lmb_efi_common_apis_nrfc_v2
[2] - https://github.com/sughoshg/u-boot/tree/lmb_efi_separate_flags_apis_nrfc_v2

>
> Regards,
> Simon
>
>
> >
> > [1] - https://gist.github.com/sughoshg/a20207f26e19238fef86f710134d6efd
> >
> > >
> > > Regards,
> > > SImon