Re: [PATCH v2 2/4] efi: mm: Extract function to add memory regions

[Daniel Kiper]

On Sun, Aug 08, 2021 at 03:31:45PM +0200, Patrick Steinhardt wrote:
> 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.
>
> Note that this commit also changes how many bytes we request by default.
> Previously, we would've tried to allocate a quarter of available system
> memory, bounded by a minimum/maximum value. As we're about to implement
> runtime allocation of memory, we now instead always request the minimum
> amount of bytes and let the memory allocator call out to our callback.

This change should go to separate patch.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  grub-core/kern/efi/mm.c | 57 ++++++++++++++---------------------------
>  1 file changed, 19 insertions(+), 38 deletions(-)
>
> diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> index 9838fb2f5..ec64c08c0 100644
> --- a/grub-core/kern/efi/mm.c
> +++ b/grub-core/kern/efi/mm.c
> @@ -38,9 +38,8 @@
>     a multiplier of 4KB.  */
>  #define MEMORY_MAP_SIZE      0x3000
>
> -/* The minimum and maximum heap size for GRUB itself.  */
> -#define MIN_HEAP_SIZE        0x100000
> -#define MAX_HEAP_SIZE        (1600 * 0x100000)
> +/* The default heap size for GRUB itself in bytes.  */
> +#define DEFAULT_HEAP_SIZE    0x100000
>
>  static void *finish_mmap_buf = 0;
>  static grub_efi_uintn_t finish_mmap_size = 0;
> @@ -478,23 +477,6 @@ filter_memory_map (grub_efi_memory_descriptor_t 
> *memory_map,
>    return filtered_desc;
>  }
>
> -/* Return the total number of pages.  */
> -static grub_efi_uint64_t
> -get_total_pages (grub_efi_memory_descriptor_t *memory_map,
> -              grub_efi_uintn_t desc_size,
> -              grub_efi_memory_descriptor_t *memory_map_end)
> -{
> -  grub_efi_memory_descriptor_t *desc;
> -  grub_efi_uint64_t total = 0;
> -
> -  for (desc = memory_map;
> -       desc < memory_map_end;
> -       desc = NEXT_MEMORY_DESCRIPTOR (desc, desc_size))
> -    total += desc->num_pages;
> -
> -  return total;
> -}
> -
>  /* Add memory regions.  */
>  static void
>  add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
> @@ -522,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),
> @@ -574,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;
> @@ -583,14 +565,12 @@ grub_efi_mm_init (void)
>    grub_efi_memory_descriptor_t *filtered_memory_map_end;
>    grub_efi_uintn_t map_size;
>    grub_efi_uintn_t desc_size;
> -  grub_efi_uint64_t total_pages;
> -  grub_efi_uint64_t required_pages;
>    int mm_status;
>
>    /* 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");
>
>    /* Obtain descriptors for available memory.  */
>    map_size = MEMORY_MAP_SIZE;
> @@ -608,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");
>
>        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");
>
>    memory_map_end = NEXT_MEMORY_DESCRIPTOR (memory_map, map_size);
>
> @@ -624,22 +604,14 @@ grub_efi_mm_init (void)
>    filtered_memory_map_end = filter_memory_map (memory_map, 
> filtered_memory_map,
>                                              desc_size, memory_map_end);
>
> -  /* By default, request a quarter of the available memory.  */
> -  total_pages = get_total_pages (filtered_memory_map, desc_size,
> -                              filtered_memory_map_end);
> -  required_pages = (total_pages >> 2);
> -  if (required_pages < BYTES_TO_PAGES (MIN_HEAP_SIZE))
> -    required_pages = BYTES_TO_PAGES (MIN_HEAP_SIZE);
> -  else if (required_pages > BYTES_TO_PAGES (MAX_HEAP_SIZE))
> -    required_pages = BYTES_TO_PAGES (MAX_HEAP_SIZE);
> -
>    /* Sort the filtered descriptors, so that GRUB can allocate pages
>       from smaller regions.  */
>    sort_memory_map (filtered_memory_map, desc_size, filtered_memory_map_end);
>
>    /* Allocate memory regions for GRUB's memory management.  */
>    add_memory_regions (filtered_memory_map, desc_size,
> -                   filtered_memory_map_end, required_pages);
> +                   filtered_memory_map_end,
> +                   BYTES_TO_PAGES (required_bytes));
>
>  #if 0
>    /* For debug.  */
> @@ -657,6 +629,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 (grub_errmsg);

grub_fatal ("%s", grub_errmsg);

Daniel