Hi Sughosh, On Mon, 5 Aug 2024 at 05:55, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > On Mon, 29 Jul 2024 at 20:56, Simon Glass <s...@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Mon, 29 Jul 2024 at 02:40, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > > > On Fri, 26 Jul 2024 at 05:02, Simon Glass <s...@chromium.org> wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Wed, 24 Jul 2024 at 00:04, Sughosh Ganu <sughosh.g...@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.g...@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