mbox series

[v4,00/15] Make EFI memory allocations synchronous with LMB

Message ID 20241015153717.401371-1-sughosh.ganu@linaro.org
Headers show
Series Make EFI memory allocations synchronous with LMB | expand

Message

Sughosh Ganu Oct. 15, 2024, 3:37 p.m. UTC
This is part two of the series to have the EFI and LMB modules have a
coherent view of memory. Part one of this goal was to change the LMB
module to have a global and persistent memory map. Those patches have
now been applied to the next branch.

These patches are changing the EFI memory allocation API's such that
they rely on the LMB module to allocate RAM memory. This fixes the
current scenario where the EFI memory module has no visibility of the
allocations/reservations made by the LMB module. One thing to note
here is that this is limited to the RAM memory region, i.e. the
EFI_CONVENTIONAL_MEMORY type. Any other memory type that is to be
added to the EFI memory map, still gets handled by the EFI memory
module.


Changes since V3:
* Add comments for the LMB_NOOVERWRITE and LMB_NONOTIFY flags
* Drop use of is_addr_in_ram() function
* Drop use of CONFIG_MEM_MAP_UPDATE_NOTIFY symbol to check if the
  notification needs to be sent.
* s/lmb_notify/lmb_should_notify
* Put a check for EFI_LOADER in the lmb_should_notify() function


Some test logs to highlight the issue that is being fixed by the series.

Without patch series
--------------------

lmb_dump_all:
 memory.count = 0x1
 memory[0]	[0x40000000-0x820fffff], 0x42100000 bytes flags: none
 reserved.count = 0x3
 reserved[0]	[0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
 reserved[1]	[0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
 reserved[2]	[0x7f77da00-0x820fffff], 0x02982600 bytes flags: no-overwrite


=> efidebug memmap -- does not show regions allocated by lmb

Missing TPMv2 device for EFI_TCG_PROTOCOL
Type             Start            End              Attributes
================ ================ ================ ==========
CONVENTIONAL     0000000040000000-000000007f751000 WB
BOOT DATA        000000007f751000-000000007f756000 WB
RUNTIME DATA     000000007f756000-000000007f757000 WB|RT
BOOT DATA        000000007f757000-000000007f758000 WB
RUNTIME DATA     000000007f758000-000000007f77a000 WB|RT
BOOT DATA        000000007f77a000-000000007f781000 WB
BOOT CODE        000000007f781000-00000000807b5000 WB
RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
BOOT CODE        00000000807b6000-00000000817c0000 WB
RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
BOOT CODE        00000000817d0000-0000000082100000 WB
=> 


Trying to allocate EFI memory with already allocated region succeeds(should fail)
---------------------------------------------------------------------------------

=> efi_mem alloc 2000 42000000
Address returned 0x42000000

=> efidebug memmap
Type             Start            End              Attributes
================ ================ ================ ==========
CONVENTIONAL     0000000040000000-0000000042000000 WB
BOOT DATA        0000000042000000-0000000042002000 WB
CONVENTIONAL     0000000042002000-000000007f751000 WB
BOOT DATA        000000007f751000-000000007f756000 WB
RUNTIME DATA     000000007f756000-000000007f757000 WB|RT
BOOT DATA        000000007f757000-000000007f758000 WB
RUNTIME DATA     000000007f758000-000000007f77a000 WB|RT
BOOT DATA        000000007f77a000-000000007f781000 WB
BOOT CODE        000000007f781000-00000000807b5000 WB
RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
BOOT CODE        00000000807b6000-00000000817c0000 WB
RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
BOOT CODE        00000000817d0000-0000000082100000 WB
=> 


With patch series
-----------------

lmb_dump_all:
 memory.count = 0x1
 memory[0]	[0x40000000-0x820fffff], 0x42100000 bytes flags: none
 reserved.count = 0x4
 reserved[0]	[0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
 reserved[1]	[0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
 reserved[2]	[0x7f74f000-0x7f77dfff], 0x0002f000 bytes flags: no-notify, no-overwrite
 reserved[3]	[0x7f77ea00-0x820fffff], 0x02981600 bytes flags: no-overwrite


=> efidebug memmap
Type             Start            End              Attributes
================ ================ ================ ==========
BOOT DATA        000000000e100000-000000000f000000 WB
CONVENTIONAL     0000000040000000-0000000042000000 WB
BOOT DATA        0000000042000000-0000000042200000 WB
CONVENTIONAL     0000000042200000-000000007f74e000 WB
BOOT DATA        000000007f74e000-000000007f753000 WB
RUNTIME DATA     000000007f753000-000000007f754000 WB|RT
BOOT DATA        000000007f754000-000000007f755000 WB
RUNTIME DATA     000000007f755000-000000007f777000 WB|RT
BOOT DATA        000000007f777000-00000000807b6000 WB
RUNTIME DATA     00000000807b6000-00000000807b7000 WB|RT
BOOT DATA        00000000807b7000-00000000817c0000 WB
RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
BOOT DATA        00000000817d0000-0000000082100000 WB


Trying to allocate EFI memory with already allocated region fails
-----------------------------------------------------------------

=> efi_mem alloc 2000 42000000
efi_allocate_pages failed 800000000000000e
=> 

Trying to allocate EFI memory with non-allocated region succeeds
----------------------------------------------------------------

=> efi_mem alloc 2000 42200000
Address returned 0x42200000

=> efidebug memmap            
Type             Start            End              Attributes
================ ================ ================ ==========
BOOT DATA        000000000e100000-000000000f000000 WB
CONVENTIONAL     0000000040000000-0000000042000000 WB
BOOT DATA        0000000042000000-0000000042202000 WB
CONVENTIONAL     0000000042202000-000000007f74d000 WB
BOOT DATA        000000007f74d000-000000007f752000 WB
RUNTIME DATA     000000007f752000-000000007f753000 WB|RT
BOOT DATA        000000007f753000-000000007f754000 WB
RUNTIME DATA     000000007f754000-000000007f776000 WB|RT
BOOT DATA        000000007f776000-00000000807b5000 WB
RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
BOOT DATA        00000000807b6000-00000000817c0000 WB
RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
BOOT DATA        00000000817d0000-0000000082100000 WB
=> 

lmb_dump_all:
 memory.count = 0x1
 memory[0]	[0x40000000-0x820fffff], 0x42100000 bytes flags: none
 reserved.count = 0x5
 reserved[0]	[0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
 reserved[1]	[0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
 reserved[2]	[0x42200000-0x42201fff], 0x00002000 bytes flags: no-notify, no-overwrite
 reserved[3]	[0x7f74e000-0x7f77cfff], 0x0002f000 bytes flags: no-notify, no-overwrite
 reserved[4]	[0x7f77da00-0x820fffff], 0x02982600 bytes flags: no-overwrite


Sughosh Ganu (15):
  lmb: add versions of the lmb API with flags
  lmb: add a flag to allow suppressing memory map change notification
  lmb: add and reserve memory above ram_top
  efi: memory: use the lmb API's for allocating and freeing memory
  lmb: notify of any changes to the LMB memory map
  efi_memory: do not add U-Boot memory to the memory map
  ti: k3: remove efi_add_known_memory() function definition
  stm32mp: remove efi_add_known_memory() function definition
  lmb: allow for boards to specify memory map
  layerscape: use the lmb API's to add RAM memory
  x86: e820: use the lmb API for adding RAM memory
  efi_memory: do not add RAM memory to the memory map
  lmb: remove call to efi_lmb_reserve()
  efi_memory: rename variable to highlight overlap with free memory
  lmb: replace the double-underscore with single-underscore for all
    functions

 arch/arm/cpu/armv8/fsl-layerscape/cpu.c |   8 +-
 arch/arm/mach-k3/common.c               |  11 --
 arch/arm/mach-stm32mp/dram_init.c       |  11 --
 arch/x86/lib/e820.c                     |  47 +++--
 include/efi_loader.h                    |  27 ++-
 include/lmb.h                           |  63 ++++++
 lib/Kconfig                             |  19 ++
 lib/efi_loader/Kconfig                  |   1 +
 lib/efi_loader/efi_memory.c             | 217 ++++++---------------
 lib/lmb.c                               | 242 ++++++++++++++++++------
 10 files changed, 389 insertions(+), 257 deletions(-)

Comments

Simon Glass Oct. 15, 2024, 6:45 p.m. UTC | #1
Hi,

On Tue, 15 Oct 2024 at 09:37, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
>
> This is part two of the series to have the EFI and LMB modules have a
> coherent view of memory. Part one of this goal was to change the LMB
> module to have a global and persistent memory map. Those patches have
> now been applied to the next branch.
>
> These patches are changing the EFI memory allocation API's such that
> they rely on the LMB module to allocate RAM memory. This fixes the
> current scenario where the EFI memory module has no visibility of the
> allocations/reservations made by the LMB module. One thing to note
> here is that this is limited to the RAM memory region, i.e. the
> EFI_CONVENTIONAL_MEMORY type. Any other memory type that is to be
> added to the EFI memory map, still gets handled by the EFI memory
> module.
>
>
> Changes since V3:
> * Add comments for the LMB_NOOVERWRITE and LMB_NONOTIFY flags
> * Drop use of is_addr_in_ram() function
> * Drop use of CONFIG_MEM_MAP_UPDATE_NOTIFY symbol to check if the
>   notification needs to be sent.
> * s/lmb_notify/lmb_should_notify
> * Put a check for EFI_LOADER in the lmb_should_notify() function
>
>
> Some test logs to highlight the issue that is being fixed by the series.
>
> Without patch series
> --------------------
>
> lmb_dump_all:
>  memory.count = 0x1
>  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
>  reserved.count = 0x3
>  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
>  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
>  reserved[2]    [0x7f77da00-0x820fffff], 0x02982600 bytes flags: no-overwrite
>
>
> => efidebug memmap -- does not show regions allocated by lmb
>
> Missing TPMv2 device for EFI_TCG_PROTOCOL
> Type             Start            End              Attributes
> ================ ================ ================ ==========
> CONVENTIONAL     0000000040000000-000000007f751000 WB
> BOOT DATA        000000007f751000-000000007f756000 WB
> RUNTIME DATA     000000007f756000-000000007f757000 WB|RT
> BOOT DATA        000000007f757000-000000007f758000 WB
> RUNTIME DATA     000000007f758000-000000007f77a000 WB|RT
> BOOT DATA        000000007f77a000-000000007f781000 WB
> BOOT CODE        000000007f781000-00000000807b5000 WB
> RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> BOOT CODE        00000000807b6000-00000000817c0000 WB
> RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> BOOT CODE        00000000817d0000-0000000082100000 WB
> =>
>
>
> Trying to allocate EFI memory with already allocated region succeeds(should fail)
> ---------------------------------------------------------------------------------
>
> => efi_mem alloc 2000 42000000
> Address returned 0x42000000
>
> => efidebug memmap
> Type             Start            End              Attributes
> ================ ================ ================ ==========
> CONVENTIONAL     0000000040000000-0000000042000000 WB
> BOOT DATA        0000000042000000-0000000042002000 WB
> CONVENTIONAL     0000000042002000-000000007f751000 WB
> BOOT DATA        000000007f751000-000000007f756000 WB
> RUNTIME DATA     000000007f756000-000000007f757000 WB|RT
> BOOT DATA        000000007f757000-000000007f758000 WB
> RUNTIME DATA     000000007f758000-000000007f77a000 WB|RT
> BOOT DATA        000000007f77a000-000000007f781000 WB
> BOOT CODE        000000007f781000-00000000807b5000 WB
> RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> BOOT CODE        00000000807b6000-00000000817c0000 WB
> RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> BOOT CODE        00000000817d0000-0000000082100000 WB
> =>
>
>
> With patch series
> -----------------
>
> lmb_dump_all:
>  memory.count = 0x1
>  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
>  reserved.count = 0x4
>  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
>  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
>  reserved[2]    [0x7f74f000-0x7f77dfff], 0x0002f000 bytes flags: no-notify, no-overwrite
>  reserved[3]    [0x7f77ea00-0x820fffff], 0x02981600 bytes flags: no-overwrite
>
>
> => efidebug memmap
> Type             Start            End              Attributes
> ================ ================ ================ ==========
> BOOT DATA        000000000e100000-000000000f000000 WB
> CONVENTIONAL     0000000040000000-0000000042000000 WB
> BOOT DATA        0000000042000000-0000000042200000 WB
> CONVENTIONAL     0000000042200000-000000007f74e000 WB
> BOOT DATA        000000007f74e000-000000007f753000 WB
> RUNTIME DATA     000000007f753000-000000007f754000 WB|RT
> BOOT DATA        000000007f754000-000000007f755000 WB
> RUNTIME DATA     000000007f755000-000000007f777000 WB|RT
> BOOT DATA        000000007f777000-00000000807b6000 WB
> RUNTIME DATA     00000000807b6000-00000000807b7000 WB|RT
> BOOT DATA        00000000807b7000-00000000817c0000 WB
> RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> BOOT DATA        00000000817d0000-0000000082100000 WB
>
>
> Trying to allocate EFI memory with already allocated region fails
> -----------------------------------------------------------------
>
> => efi_mem alloc 2000 42000000
> efi_allocate_pages failed 800000000000000e
> =>
>
> Trying to allocate EFI memory with non-allocated region succeeds
> ----------------------------------------------------------------
>
> => efi_mem alloc 2000 42200000
> Address returned 0x42200000
>
> => efidebug memmap
> Type             Start            End              Attributes
> ================ ================ ================ ==========
> BOOT DATA        000000000e100000-000000000f000000 WB
> CONVENTIONAL     0000000040000000-0000000042000000 WB
> BOOT DATA        0000000042000000-0000000042202000 WB
> CONVENTIONAL     0000000042202000-000000007f74d000 WB
> BOOT DATA        000000007f74d000-000000007f752000 WB
> RUNTIME DATA     000000007f752000-000000007f753000 WB|RT
> BOOT DATA        000000007f753000-000000007f754000 WB
> RUNTIME DATA     000000007f754000-000000007f776000 WB|RT
> BOOT DATA        000000007f776000-00000000807b5000 WB
> RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> BOOT DATA        00000000807b6000-00000000817c0000 WB
> RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> BOOT DATA        00000000817d0000-0000000082100000 WB
> =>
>
> lmb_dump_all:
>  memory.count = 0x1
>  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
>  reserved.count = 0x5
>  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
>  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
>  reserved[2]    [0x42200000-0x42201fff], 0x00002000 bytes flags: no-notify, no-overwrite
>  reserved[3]    [0x7f74e000-0x7f77cfff], 0x0002f000 bytes flags: no-notify, no-overwrite
>  reserved[4]    [0x7f77da00-0x820fffff], 0x02982600 bytes flags: no-overwrite
>
>
> Sughosh Ganu (15):
>   lmb: add versions of the lmb API with flags
>   lmb: add a flag to allow suppressing memory map change notification
>   lmb: add and reserve memory above ram_top
>   efi: memory: use the lmb API's for allocating and freeing memory
>   lmb: notify of any changes to the LMB memory map
>   efi_memory: do not add U-Boot memory to the memory map
>   ti: k3: remove efi_add_known_memory() function definition
>   stm32mp: remove efi_add_known_memory() function definition
>   lmb: allow for boards to specify memory map
>   layerscape: use the lmb API's to add RAM memory
>   x86: e820: use the lmb API for adding RAM memory
>   efi_memory: do not add RAM memory to the memory map
>   lmb: remove call to efi_lmb_reserve()
>   efi_memory: rename variable to highlight overlap with free memory
>   lmb: replace the double-underscore with single-underscore for all
>     functions
>
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c |   8 +-
>  arch/arm/mach-k3/common.c               |  11 --
>  arch/arm/mach-stm32mp/dram_init.c       |  11 --
>  arch/x86/lib/e820.c                     |  47 +++--
>  include/efi_loader.h                    |  27 ++-
>  include/lmb.h                           |  63 ++++++
>  lib/Kconfig                             |  19 ++
>  lib/efi_loader/Kconfig                  |   1 +
>  lib/efi_loader/efi_memory.c             | 217 ++++++---------------
>  lib/lmb.c                               | 242 ++++++++++++++++++------
>  10 files changed, 389 insertions(+), 257 deletions(-)
>
> --
> 2.34.1
>
>

I have stated my objections and won't repeat them here. With -master
currently broken for EFI, and many of the patches in this series being
required even with my view of the world, I think the best thing is to
apply this series now, and move on.

Regards,
SImon
Tom Rini Oct. 15, 2024, 7:16 p.m. UTC | #2
On Tue, Oct 15, 2024 at 12:45:59PM -0600, Simon Glass wrote:
> Hi,
> 
> On Tue, 15 Oct 2024 at 09:37, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> >
> > This is part two of the series to have the EFI and LMB modules have a
> > coherent view of memory. Part one of this goal was to change the LMB
> > module to have a global and persistent memory map. Those patches have
> > now been applied to the next branch.
> >
> > These patches are changing the EFI memory allocation API's such that
> > they rely on the LMB module to allocate RAM memory. This fixes the
> > current scenario where the EFI memory module has no visibility of the
> > allocations/reservations made by the LMB module. One thing to note
> > here is that this is limited to the RAM memory region, i.e. the
> > EFI_CONVENTIONAL_MEMORY type. Any other memory type that is to be
> > added to the EFI memory map, still gets handled by the EFI memory
> > module.
> >
> >
> > Changes since V3:
> > * Add comments for the LMB_NOOVERWRITE and LMB_NONOTIFY flags
> > * Drop use of is_addr_in_ram() function
> > * Drop use of CONFIG_MEM_MAP_UPDATE_NOTIFY symbol to check if the
> >   notification needs to be sent.
> > * s/lmb_notify/lmb_should_notify
> > * Put a check for EFI_LOADER in the lmb_should_notify() function
> >
> >
> > Some test logs to highlight the issue that is being fixed by the series.
> >
> > Without patch series
> > --------------------
> >
> > lmb_dump_all:
> >  memory.count = 0x1
> >  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
> >  reserved.count = 0x3
> >  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
> >  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
> >  reserved[2]    [0x7f77da00-0x820fffff], 0x02982600 bytes flags: no-overwrite
> >
> >
> > => efidebug memmap -- does not show regions allocated by lmb
> >
> > Missing TPMv2 device for EFI_TCG_PROTOCOL
> > Type             Start            End              Attributes
> > ================ ================ ================ ==========
> > CONVENTIONAL     0000000040000000-000000007f751000 WB
> > BOOT DATA        000000007f751000-000000007f756000 WB
> > RUNTIME DATA     000000007f756000-000000007f757000 WB|RT
> > BOOT DATA        000000007f757000-000000007f758000 WB
> > RUNTIME DATA     000000007f758000-000000007f77a000 WB|RT
> > BOOT DATA        000000007f77a000-000000007f781000 WB
> > BOOT CODE        000000007f781000-00000000807b5000 WB
> > RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> > BOOT CODE        00000000807b6000-00000000817c0000 WB
> > RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> > BOOT CODE        00000000817d0000-0000000082100000 WB
> > =>
> >
> >
> > Trying to allocate EFI memory with already allocated region succeeds(should fail)
> > ---------------------------------------------------------------------------------
> >
> > => efi_mem alloc 2000 42000000
> > Address returned 0x42000000
> >
> > => efidebug memmap
> > Type             Start            End              Attributes
> > ================ ================ ================ ==========
> > CONVENTIONAL     0000000040000000-0000000042000000 WB
> > BOOT DATA        0000000042000000-0000000042002000 WB
> > CONVENTIONAL     0000000042002000-000000007f751000 WB
> > BOOT DATA        000000007f751000-000000007f756000 WB
> > RUNTIME DATA     000000007f756000-000000007f757000 WB|RT
> > BOOT DATA        000000007f757000-000000007f758000 WB
> > RUNTIME DATA     000000007f758000-000000007f77a000 WB|RT
> > BOOT DATA        000000007f77a000-000000007f781000 WB
> > BOOT CODE        000000007f781000-00000000807b5000 WB
> > RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> > BOOT CODE        00000000807b6000-00000000817c0000 WB
> > RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> > BOOT CODE        00000000817d0000-0000000082100000 WB
> > =>
> >
> >
> > With patch series
> > -----------------
> >
> > lmb_dump_all:
> >  memory.count = 0x1
> >  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
> >  reserved.count = 0x4
> >  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
> >  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
> >  reserved[2]    [0x7f74f000-0x7f77dfff], 0x0002f000 bytes flags: no-notify, no-overwrite
> >  reserved[3]    [0x7f77ea00-0x820fffff], 0x02981600 bytes flags: no-overwrite
> >
> >
> > => efidebug memmap
> > Type             Start            End              Attributes
> > ================ ================ ================ ==========
> > BOOT DATA        000000000e100000-000000000f000000 WB
> > CONVENTIONAL     0000000040000000-0000000042000000 WB
> > BOOT DATA        0000000042000000-0000000042200000 WB
> > CONVENTIONAL     0000000042200000-000000007f74e000 WB
> > BOOT DATA        000000007f74e000-000000007f753000 WB
> > RUNTIME DATA     000000007f753000-000000007f754000 WB|RT
> > BOOT DATA        000000007f754000-000000007f755000 WB
> > RUNTIME DATA     000000007f755000-000000007f777000 WB|RT
> > BOOT DATA        000000007f777000-00000000807b6000 WB
> > RUNTIME DATA     00000000807b6000-00000000807b7000 WB|RT
> > BOOT DATA        00000000807b7000-00000000817c0000 WB
> > RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> > BOOT DATA        00000000817d0000-0000000082100000 WB
> >
> >
> > Trying to allocate EFI memory with already allocated region fails
> > -----------------------------------------------------------------
> >
> > => efi_mem alloc 2000 42000000
> > efi_allocate_pages failed 800000000000000e
> > =>
> >
> > Trying to allocate EFI memory with non-allocated region succeeds
> > ----------------------------------------------------------------
> >
> > => efi_mem alloc 2000 42200000
> > Address returned 0x42200000
> >
> > => efidebug memmap
> > Type             Start            End              Attributes
> > ================ ================ ================ ==========
> > BOOT DATA        000000000e100000-000000000f000000 WB
> > CONVENTIONAL     0000000040000000-0000000042000000 WB
> > BOOT DATA        0000000042000000-0000000042202000 WB
> > CONVENTIONAL     0000000042202000-000000007f74d000 WB
> > BOOT DATA        000000007f74d000-000000007f752000 WB
> > RUNTIME DATA     000000007f752000-000000007f753000 WB|RT
> > BOOT DATA        000000007f753000-000000007f754000 WB
> > RUNTIME DATA     000000007f754000-000000007f776000 WB|RT
> > BOOT DATA        000000007f776000-00000000807b5000 WB
> > RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> > BOOT DATA        00000000807b6000-00000000817c0000 WB
> > RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> > BOOT DATA        00000000817d0000-0000000082100000 WB
> > =>
> >
> > lmb_dump_all:
> >  memory.count = 0x1
> >  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
> >  reserved.count = 0x5
> >  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
> >  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
> >  reserved[2]    [0x42200000-0x42201fff], 0x00002000 bytes flags: no-notify, no-overwrite
> >  reserved[3]    [0x7f74e000-0x7f77cfff], 0x0002f000 bytes flags: no-notify, no-overwrite
> >  reserved[4]    [0x7f77da00-0x820fffff], 0x02982600 bytes flags: no-overwrite
> >
> >
> > Sughosh Ganu (15):
> >   lmb: add versions of the lmb API with flags
> >   lmb: add a flag to allow suppressing memory map change notification
> >   lmb: add and reserve memory above ram_top
> >   efi: memory: use the lmb API's for allocating and freeing memory
> >   lmb: notify of any changes to the LMB memory map
> >   efi_memory: do not add U-Boot memory to the memory map
> >   ti: k3: remove efi_add_known_memory() function definition
> >   stm32mp: remove efi_add_known_memory() function definition
> >   lmb: allow for boards to specify memory map
> >   layerscape: use the lmb API's to add RAM memory
> >   x86: e820: use the lmb API for adding RAM memory
> >   efi_memory: do not add RAM memory to the memory map
> >   lmb: remove call to efi_lmb_reserve()
> >   efi_memory: rename variable to highlight overlap with free memory
> >   lmb: replace the double-underscore with single-underscore for all
> >     functions
> >
> >  arch/arm/cpu/armv8/fsl-layerscape/cpu.c |   8 +-
> >  arch/arm/mach-k3/common.c               |  11 --
> >  arch/arm/mach-stm32mp/dram_init.c       |  11 --
> >  arch/x86/lib/e820.c                     |  47 +++--
> >  include/efi_loader.h                    |  27 ++-
> >  include/lmb.h                           |  63 ++++++
> >  lib/Kconfig                             |  19 ++
> >  lib/efi_loader/Kconfig                  |   1 +
> >  lib/efi_loader/efi_memory.c             | 217 ++++++---------------
> >  lib/lmb.c                               | 242 ++++++++++++++++++------
> >  10 files changed, 389 insertions(+), 257 deletions(-)
> 
> I have stated my objections and won't repeat them here. With -master
> currently broken for EFI, and many of the patches in this series being
> required even with my view of the world, I think the best thing is to
> apply this series now, and move on.

Thank you, Simon, for saying this. I will be applying this shortly.
Ilias Apalodimas Oct. 15, 2024, 7:25 p.m. UTC | #3
Thanks Tom,

On Tue, 15 Oct 2024 at 22:16, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Oct 15, 2024 at 12:45:59PM -0600, Simon Glass wrote:
> > Hi,
> >
> > On Tue, 15 Oct 2024 at 09:37, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > >
> > > This is part two of the series to have the EFI and LMB modules have a
> > > coherent view of memory. Part one of this goal was to change the LMB
> > > module to have a global and persistent memory map. Those patches have
> > > now been applied to the next branch.
> > >
> > > These patches are changing the EFI memory allocation API's such that
> > > they rely on the LMB module to allocate RAM memory. This fixes the
> > > current scenario where the EFI memory module has no visibility of the
> > > allocations/reservations made by the LMB module. One thing to note
> > > here is that this is limited to the RAM memory region, i.e. the
> > > EFI_CONVENTIONAL_MEMORY type. Any other memory type that is to be
> > > added to the EFI memory map, still gets handled by the EFI memory
> > > module.
> > >
> > >
> > > Changes since V3:
> > > * Add comments for the LMB_NOOVERWRITE and LMB_NONOTIFY flags
> > > * Drop use of is_addr_in_ram() function
> > > * Drop use of CONFIG_MEM_MAP_UPDATE_NOTIFY symbol to check if the
> > >   notification needs to be sent.
> > > * s/lmb_notify/lmb_should_notify
> > > * Put a check for EFI_LOADER in the lmb_should_notify() function
> > >
> > >
> > > Some test logs to highlight the issue that is being fixed by the series.
> > >
> > > Without patch series
> > > --------------------
> > >
> > > lmb_dump_all:
> > >  memory.count = 0x1
> > >  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
> > >  reserved.count = 0x3
> > >  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
> > >  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
> > >  reserved[2]    [0x7f77da00-0x820fffff], 0x02982600 bytes flags: no-overwrite
> > >
> > >
> > > => efidebug memmap -- does not show regions allocated by lmb
> > >
> > > Missing TPMv2 device for EFI_TCG_PROTOCOL
> > > Type             Start            End              Attributes
> > > ================ ================ ================ ==========
> > > CONVENTIONAL     0000000040000000-000000007f751000 WB
> > > BOOT DATA        000000007f751000-000000007f756000 WB
> > > RUNTIME DATA     000000007f756000-000000007f757000 WB|RT
> > > BOOT DATA        000000007f757000-000000007f758000 WB
> > > RUNTIME DATA     000000007f758000-000000007f77a000 WB|RT
> > > BOOT DATA        000000007f77a000-000000007f781000 WB
> > > BOOT CODE        000000007f781000-00000000807b5000 WB
> > > RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> > > BOOT CODE        00000000807b6000-00000000817c0000 WB
> > > RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> > > BOOT CODE        00000000817d0000-0000000082100000 WB
> > > =>
> > >
> > >
> > > Trying to allocate EFI memory with already allocated region succeeds(should fail)
> > > ---------------------------------------------------------------------------------
> > >
> > > => efi_mem alloc 2000 42000000
> > > Address returned 0x42000000
> > >
> > > => efidebug memmap
> > > Type             Start            End              Attributes
> > > ================ ================ ================ ==========
> > > CONVENTIONAL     0000000040000000-0000000042000000 WB
> > > BOOT DATA        0000000042000000-0000000042002000 WB
> > > CONVENTIONAL     0000000042002000-000000007f751000 WB
> > > BOOT DATA        000000007f751000-000000007f756000 WB
> > > RUNTIME DATA     000000007f756000-000000007f757000 WB|RT
> > > BOOT DATA        000000007f757000-000000007f758000 WB
> > > RUNTIME DATA     000000007f758000-000000007f77a000 WB|RT
> > > BOOT DATA        000000007f77a000-000000007f781000 WB
> > > BOOT CODE        000000007f781000-00000000807b5000 WB
> > > RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> > > BOOT CODE        00000000807b6000-00000000817c0000 WB
> > > RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> > > BOOT CODE        00000000817d0000-0000000082100000 WB
> > > =>
> > >
> > >
> > > With patch series
> > > -----------------
> > >
> > > lmb_dump_all:
> > >  memory.count = 0x1
> > >  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
> > >  reserved.count = 0x4
> > >  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
> > >  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
> > >  reserved[2]    [0x7f74f000-0x7f77dfff], 0x0002f000 bytes flags: no-notify, no-overwrite
> > >  reserved[3]    [0x7f77ea00-0x820fffff], 0x02981600 bytes flags: no-overwrite
> > >
> > >
> > > => efidebug memmap
> > > Type             Start            End              Attributes
> > > ================ ================ ================ ==========
> > > BOOT DATA        000000000e100000-000000000f000000 WB
> > > CONVENTIONAL     0000000040000000-0000000042000000 WB
> > > BOOT DATA        0000000042000000-0000000042200000 WB
> > > CONVENTIONAL     0000000042200000-000000007f74e000 WB
> > > BOOT DATA        000000007f74e000-000000007f753000 WB
> > > RUNTIME DATA     000000007f753000-000000007f754000 WB|RT
> > > BOOT DATA        000000007f754000-000000007f755000 WB
> > > RUNTIME DATA     000000007f755000-000000007f777000 WB|RT
> > > BOOT DATA        000000007f777000-00000000807b6000 WB
> > > RUNTIME DATA     00000000807b6000-00000000807b7000 WB|RT
> > > BOOT DATA        00000000807b7000-00000000817c0000 WB
> > > RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> > > BOOT DATA        00000000817d0000-0000000082100000 WB
> > >
> > >
> > > Trying to allocate EFI memory with already allocated region fails
> > > -----------------------------------------------------------------
> > >
> > > => efi_mem alloc 2000 42000000
> > > efi_allocate_pages failed 800000000000000e
> > > =>
> > >
> > > Trying to allocate EFI memory with non-allocated region succeeds
> > > ----------------------------------------------------------------
> > >
> > > => efi_mem alloc 2000 42200000
> > > Address returned 0x42200000
> > >
> > > => efidebug memmap
> > > Type             Start            End              Attributes
> > > ================ ================ ================ ==========
> > > BOOT DATA        000000000e100000-000000000f000000 WB
> > > CONVENTIONAL     0000000040000000-0000000042000000 WB
> > > BOOT DATA        0000000042000000-0000000042202000 WB
> > > CONVENTIONAL     0000000042202000-000000007f74d000 WB
> > > BOOT DATA        000000007f74d000-000000007f752000 WB
> > > RUNTIME DATA     000000007f752000-000000007f753000 WB|RT
> > > BOOT DATA        000000007f753000-000000007f754000 WB
> > > RUNTIME DATA     000000007f754000-000000007f776000 WB|RT
> > > BOOT DATA        000000007f776000-00000000807b5000 WB
> > > RUNTIME DATA     00000000807b5000-00000000807b6000 WB|RT
> > > BOOT DATA        00000000807b6000-00000000817c0000 WB
> > > RUNTIME CODE     00000000817c0000-00000000817d0000 WB|RT
> > > BOOT DATA        00000000817d0000-0000000082100000 WB
> > > =>
> > >
> > > lmb_dump_all:
> > >  memory.count = 0x1
> > >  memory[0]      [0x40000000-0x820fffff], 0x42100000 bytes flags: none
> > >  reserved.count = 0x5
> > >  reserved[0]    [0xe100000-0xeffffff], 0x00f00000 bytes flags: no-map
> > >  reserved[1]    [0x42000000-0x421fffff], 0x00200000 bytes flags: no-map
> > >  reserved[2]    [0x42200000-0x42201fff], 0x00002000 bytes flags: no-notify, no-overwrite
> > >  reserved[3]    [0x7f74e000-0x7f77cfff], 0x0002f000 bytes flags: no-notify, no-overwrite
> > >  reserved[4]    [0x7f77da00-0x820fffff], 0x02982600 bytes flags: no-overwrite
> > >
> > >
> > > Sughosh Ganu (15):
> > >   lmb: add versions of the lmb API with flags
> > >   lmb: add a flag to allow suppressing memory map change notification
> > >   lmb: add and reserve memory above ram_top
> > >   efi: memory: use the lmb API's for allocating and freeing memory
> > >   lmb: notify of any changes to the LMB memory map
> > >   efi_memory: do not add U-Boot memory to the memory map
> > >   ti: k3: remove efi_add_known_memory() function definition
> > >   stm32mp: remove efi_add_known_memory() function definition
> > >   lmb: allow for boards to specify memory map
> > >   layerscape: use the lmb API's to add RAM memory
> > >   x86: e820: use the lmb API for adding RAM memory
> > >   efi_memory: do not add RAM memory to the memory map
> > >   lmb: remove call to efi_lmb_reserve()
> > >   efi_memory: rename variable to highlight overlap with free memory
> > >   lmb: replace the double-underscore with single-underscore for all
> > >     functions
> > >
> > >  arch/arm/cpu/armv8/fsl-layerscape/cpu.c |   8 +-
> > >  arch/arm/mach-k3/common.c               |  11 --
> > >  arch/arm/mach-stm32mp/dram_init.c       |  11 --
> > >  arch/x86/lib/e820.c                     |  47 +++--
> > >  include/efi_loader.h                    |  27 ++-
> > >  include/lmb.h                           |  63 ++++++
> > >  lib/Kconfig                             |  19 ++
> > >  lib/efi_loader/Kconfig                  |   1 +
> > >  lib/efi_loader/efi_memory.c             | 217 ++++++---------------
> > >  lib/lmb.c                               | 242 ++++++++++++++++++------
> > >  10 files changed, 389 insertions(+), 257 deletions(-)
> >
> > I have stated my objections and won't repeat them here. With -master
> > currently broken for EFI, and many of the patches in this series being
> > required even with my view of the world, I think the best thing is to
> > apply this series now, and move on.
>
> Thank you, Simon, for saying this. I will be applying this shortly.

I know this is kind of urgent, but it's late here, I'll be able to
have a look at the patches tomorrow. From a quick skim they look
reasonable, so feel free to apply it and we can send fixes if anything
isn't working as intended.

Thanks
/Ilias
>
> --
> Tom
Tom Rini Oct. 15, 2024, 10:26 p.m. UTC | #4
On Tue, 15 Oct 2024 21:07:02 +0530, Sughosh Ganu wrote:

> This is part two of the series to have the EFI and LMB modules have a
> coherent view of memory. Part one of this goal was to change the LMB
> module to have a global and persistent memory map. Those patches have
> now been applied to the next branch.
> 
> These patches are changing the EFI memory allocation API's such that
> they rely on the LMB module to allocate RAM memory. This fixes the
> current scenario where the EFI memory module has no visibility of the
> allocations/reservations made by the LMB module. One thing to note
> here is that this is limited to the RAM memory region, i.e. the
> EFI_CONVENTIONAL_MEMORY type. Any other memory type that is to be
> added to the EFI memory map, still gets handled by the EFI memory
> module.
> 
> [...]

Applied to u-boot/master, thanks!