| Message ID | 20240725135629.3505072-1-sjg@chromium.org |
|---|---|
| Headers | show |
| Series | efi: Start tidying up memory management | expand |
Hi Simon On Thu, 25 Jul 2024 at 16:58, Simon Glass <sjg@chromium.org> wrote: > > We have been discussing the state of EFI memory management for some > years so I thought it might be best to send a short series showing some > of the issues we have talked about. > > This one just deals with memory allocation. It provides a way to detect > EFI 'page allocations' when U-Boot' malloc() should be used. It should > help us to clean up this area of U-Boot. > > It also updates EFI to use U-Boot memory allocation for the pool. Most > functions use that anyway and it is much more efficient. It also avoids > allocating things 'in space' in conflict with the loaded images. > > This series also includes a little patch to show more information in > the index for the EFI pages. > > Note that this series is independent from Sugosh's lmb series[1], > although I believe it will point the way to simplifying some of the > later patches in that series. > > Overall, some issues which should be resolved are: > - allocation inconsistency, e.g. efi_add_protocol() uses malloc() but > efi_dp_dup() does not (this series makes a start) The EFI memory allocations and mappings are described in the EFI spec. Look at chapter 7.2 Memory Allocation Services. This patchset breaks various aspects of the spec, since the metadata needed for the EFI memory map is not kept from malloc. The reason efi_add_protocol() uses calloc, is that this is memory used from our internal implementation of the protocol linked list -- IOW how to search and identify protocols that we add in a list and not UEFI boot or runtime services. Thanks /Ilias > - change efi_bl_init() to register events later, when the devices are > actually used > - rather than efi_set_bootdev(), provide a bootstd way to take note of > the device images came from and allow EFI to query that when needed > - EFI_LOADER_BOUNCE_BUFFER - can use U-Boot bounce buffers? > > Minor questions on my mind: > - is unaligned access useful? Is there a performance penalty? > - API confusion about whether something is an address or a pointer > > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=416441 > > > Simon Glass (6): > doc: Show more information in efi index > efi: Convert device_path allocation to use malloc() > efi: Allow monitoring of page allocations > efi: Avoid pool allocation in efi_get_memory_map_alloc() > efi: Use malloc() for the EFI pool > efi: Show the location of the bounce buffer > > doc/develop/uefi/index.rst | 2 +- > include/efi_loader.h | 24 ++++ > lib/efi_loader/efi_bootbin.c | 10 ++ > lib/efi_loader/efi_device_path.c | 43 ++++--- > lib/efi_loader/efi_device_path_to_text.c | 2 +- > lib/efi_loader/efi_memory.c | 148 +++++++++++------------ > lib/efi_loader/efi_setup.c | 7 ++ > 7 files changed, 133 insertions(+), 103 deletions(-) > > -- > 2.34.1 >
Hi Ilias, On Thu, Jul 25, 2024, 08:26 Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > Hi Simon > > On Thu, 25 Jul 2024 at 16:58, Simon Glass <sjg@chromium.org> wrote: > > > > We have been discussing the state of EFI memory management for some > > years so I thought it might be best to send a short series showing some > > of the issues we have talked about. > > > > This one just deals with memory allocation. It provides a way to detect > > EFI 'page allocations' when U-Boot' malloc() should be used. It should > > help us to clean up this area of U-Boot. > > > > It also updates EFI to use U-Boot memory allocation for the pool. Most > > functions use that anyway and it is much more efficient. It also avoids > > allocating things 'in space' in conflict with the loaded images. > > > > This series also includes a little patch to show more information in > > the index for the EFI pages. > > > > Note that this series is independent from Sugosh's lmb series[1], > > although I believe it will point the way to simplifying some of the > > later patches in that series. > > > > Overall, some issues which should be resolved are: > > - allocation inconsistency, e.g. efi_add_protocol() uses malloc() but > > efi_dp_dup() does not (this series makes a start) > > The EFI memory allocations and mappings are described in the EFI spec. > Look at chapter 7.2 Memory Allocation Services. This patchset breaks > various aspects of the spec, since the metadata needed for the EFI > memory map is not kept from malloc. The reason efi_add_protocol() uses > calloc, is that this is memory used from our internal implementation > of the protocol linked list -- IOW how to search and identify > protocols that we add in a list and not UEFI boot or runtime services. Er, yes, I am aware of that chapter:-) Can you please be more specific about which various aspects this will break? Also please let me know if you have comments on any of the patches. Regards, Simon > Thanks > /Ilias > > > > - change efi_bl_init() to register events later, when the devices are > > actually used > > - rather than efi_set_bootdev(), provide a bootstd way to take note of > > the device images came from and allow EFI to query that when needed > > - EFI_LOADER_BOUNCE_BUFFER - can use U-Boot bounce buffers? > > > > Minor questions on my mind: > > - is unaligned access useful? Is there a performance penalty? > > - API confusion about whether something is an address or a pointer > > > > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=416441 > > > > > > Simon Glass (6): > > doc: Show more information in efi index > > efi: Convert device_path allocation to use malloc() > > efi: Allow monitoring of page allocations > > efi: Avoid pool allocation in efi_get_memory_map_alloc() > > efi: Use malloc() for the EFI pool > > efi: Show the location of the bounce buffer > > > > doc/develop/uefi/index.rst | 2 +- > > include/efi_loader.h | 24 ++++ > > lib/efi_loader/efi_bootbin.c | 10 ++ > > lib/efi_loader/efi_device_path.c | 43 ++++--- > > lib/efi_loader/efi_device_path_to_text.c | 2 +- > > lib/efi_loader/efi_memory.c | 148 +++++++++++------------ > > lib/efi_loader/efi_setup.c | 7 ++ > > 7 files changed, 133 insertions(+), 103 deletions(-) > > > > -- > > 2.34.1 > > >