Hi Heinrich,
This looks reasonable to me as a short term solution, but I'd feel safer if
someone else had a look at it
On Wed, Jan 04, 2023 at 05:26:06AM +0100, Heinrich Schuchardt wrote:
> Add reservations for all EFI memory areas that are not
> EFI_CONVENTIONAL_MEMORY.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
> ---
> lib/lmb.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/lib/lmb.c b/lib/lmb.c
> index c599608fa3..aee2617593 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -7,7 +7,9 @@
> */
>
> #include <common.h>
> +#include <efi_loader.h>
> #include <image.h>
> +#include <mapmem.h>
> #include <lmb.h>
> #include <log.h>
> #include <malloc.h>
> @@ -153,6 +155,46 @@ void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp,
> ulong end, ulong align)
> }
> }
>
> +/**
> + * efi_lmb_reserve() - add reservations for EFI memory
> + *
> + * Add reservations for all EFI memory areas that are not
> + * EFI_CONVENTIONAL_MEMORY.
> + *
> + * @lmb: lmb environment
> + * Return: 0 on success, 1 on failure
> + */
> +static __maybe_unused int efi_lmb_reserve(struct lmb *lmb)
> +{
> + struct efi_mem_desc *memmap = NULL, *map;
> + efi_uintn_t i, map_size = 0;
> + efi_status_t ret;
> +
> + ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + map_size += sizeof(struct efi_mem_desc); /* for my own */
> + ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
> + (void *)&memmap);
> + if (ret != EFI_SUCCESS)
> + return 1;
> + ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
> + }
> + if (ret != EFI_SUCCESS) {
> + efi_free_pool(memmap);
> + return 1;
> + }
We got a very similar piece of code in cmd/efidebug.c. I think we are
better off adding a function that just calls efi_get_memory_map() and
returns the map + map_size, instead of duplicating that
> +
> + for (i = 0, map = memmap; i < map_size / sizeof(*map); ++map, ++i) {
> + if (map->type != EFI_CONVENTIONAL_MEMORY)
> + lmb_reserve(lmb,
> + map_to_sysmem((void *)(uintptr_t)
> + map->physical_start),
> + map->num_pages * EFI_PAGE_SIZE);
> + }
> + efi_free_pool(memmap);
> + return 0;
> +}
> +
> static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob)
> {
> arch_lmb_reserve(lmb);
> @@ -160,6 +202,9 @@ static void lmb_reserve_common(struct lmb *lmb, void
> *fdt_blob)
>
> if (CONFIG_IS_ENABLED(OF_LIBFDT) && fdt_blob)
> boot_fdt_add_mem_rsv_regions(lmb, fdt_blob);
> +
> + if (CONFIG_IS_ENABLED(EFI_LOADER))
> + efi_lmb_reserve(lmb);
> }
>
> /* Initialize the struct, add memory and call arch/board reserve functions */
> --
> 2.37.2
>
Other than that this looks fine
Cheers
/Ilias
