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

[Simon Glass]

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?

Regards,
Simon


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