On Tue, 12 Oct 2021 18:30:00 +1100
Daniel Axtens <d...@axtens.net> wrote:
> From: Patrick Steinhardt <p...@pks.im>
>
> In preparation of support for runtime-allocating additional memory
> region, this patch extracts the function to retrieve the EFI memory map
> and add a subset of it to GRUB's own memory regions.
>
> Signed-off-by: Patrick Steinhardt <p...@pks.im>
> ---
> grub-core/kern/efi/mm.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> index 4d276bc87a4c..cfc6a67fc0cc 100644
> --- a/grub-core/kern/efi/mm.c
> +++ b/grub-core/kern/efi/mm.c
> @@ -504,7 +504,7 @@ add_memory_regions (grub_efi_memory_descriptor_t
> *memory_map,
>
> addr = grub_efi_allocate_pages_real (start, pages,
> GRUB_EFI_ALLOCATE_ADDRESS,
> - GRUB_EFI_LOADER_CODE);
> + GRUB_EFI_LOADER_CODE);
> if (! addr)
> grub_fatal ("cannot allocate conventional memory %p with %u pages",
> (void *) ((grub_addr_t) start),
> @@ -556,8 +556,8 @@ print_memory_map (grub_efi_memory_descriptor_t
> *memory_map,
> }
> #endif
>
> -void
> -grub_efi_mm_init (void)
> +static grub_err_t
> +grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
> {
> grub_efi_memory_descriptor_t *memory_map;
> grub_efi_memory_descriptor_t *memory_map_end;
> @@ -570,7 +570,7 @@ grub_efi_mm_init (void)
> /* Prepare a memory region to store two memory maps. */
> memory_map = grub_efi_allocate_any_pages (2 * BYTES_TO_PAGES
> (MEMORY_MAP_SIZE));
> if (! memory_map)
> - grub_fatal ("cannot allocate memory");
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
Something more descriptive would be nice, may be even "cannot allocate
memory for memory map"
>
> /* Obtain descriptors for available memory. */
> map_size = MEMORY_MAP_SIZE;
> @@ -588,14 +588,14 @@ grub_efi_mm_init (void)
>
> memory_map = grub_efi_allocate_any_pages (2 * BYTES_TO_PAGES
> (map_size));
> if (! memory_map)
> - grub_fatal ("cannot allocate memory");
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
Ditto. And maybe nice to have it slightly different, perhaps "cannot
re-allocate memory map"
>
> mm_status = grub_efi_get_memory_map (&map_size, memory_map, 0,
> &desc_size, 0);
> }
>
> if (mm_status < 0)
> - grub_fatal ("cannot get memory map");
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot get memory map");
What are the range of values that mm_status could be? Would it be
useful to include the status code in the error message? IOW, could a
failure here be affected by a configuration that the user could change
to make this work? Also I like the message "failed to retrieve memory
map (status=%d)".
>
> memory_map_end = NEXT_MEMORY_DESCRIPTOR (memory_map, map_size);
>
> @@ -610,7 +610,7 @@ grub_efi_mm_init (void)
>
> /* Allocate memory regions for GRUB's memory management. */
> add_memory_regions (filtered_memory_map, desc_size,
> - filtered_memory_map_end, BYTES_TO_PAGES
> (DEFAULT_HEAP_SIZE));
> + filtered_memory_map_end, BYTES_TO_PAGES (required_bytes));
>
> #if 0
> /* For debug. */
What about turning this on when MM_DEBUG is on? Seems like it could be
a useful (would been to get rid of/change the grub_fatal calls).
> @@ -628,6 +628,15 @@ grub_efi_mm_init (void)
> /* Release the memory maps. */
> grub_efi_free_pages ((grub_addr_t) memory_map,
> 2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +void
> +grub_efi_mm_init (void)
> +{
> + if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE) != GRUB_ERR_NONE)
> + grub_fatal ("%s", grub_errmsg);
> }
>
> #if defined (__aarch64__) || defined (__arm__) || defined (__riscv)
Glenn
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
