Re: [PATCH v2 4/4] efi: mm: Implement runtime addition of pages

[Daniel Kiper]

On Sun, Aug 08, 2021 at 03:31:53PM +0200, Patrick Steinhardt wrote:
> Adjust the interface of `grub_efi_mm_add_regions ()` to take a set of
> `GRUB_MM_REGION_*` flags, which most notably is currently only the
> `CONSECUTVE` flag. This allows us to set the function up as callback for
> the memory subsystem and have it call out to us in case there's not
> enough pages available in the current heap.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  grub-core/kern/efi/mm.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> index 376af10af..bf7102f9c 100644
> --- a/grub-core/kern/efi/mm.c
> +++ b/grub-core/kern/efi/mm.c
> @@ -482,7 +482,8 @@ static grub_err_t
>  add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
>                   grub_efi_uintn_t desc_size,
>                   grub_efi_memory_descriptor_t *memory_map_end,
> -                 grub_efi_uint64_t required_pages)
> +                 grub_efi_uint64_t required_pages,
> +                 char consecutive)

I would prefer if you take flags directly instead of introducing
consecutive argument here.

>  {
>    grub_efi_memory_descriptor_t *desc;
>
> @@ -496,6 +497,10 @@ add_memory_regions (grub_efi_memory_descriptor_t 
> *memory_map,
>
>        start = desc->physical_start;
>        pages = desc->num_pages;
> +
> +      if (pages < required_pages && consecutive)
> +     continue;
> +
>        if (pages > required_pages)
>       {
>         start += PAGES_TO_BYTES (pages - required_pages);
> @@ -559,7 +564,7 @@ print_memory_map (grub_efi_memory_descriptor_t 
> *memory_map,
>  #endif
>
>  static grub_err_t
> -grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
> +grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes, unsigned flags)

s/unsigned/unsigned int/

>  {
>    grub_efi_memory_descriptor_t *memory_map;
>    grub_efi_memory_descriptor_t *memory_map_end;
> @@ -614,7 +619,8 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
>    /* Allocate memory regions for GRUB's memory management.  */
>    err = add_memory_regions (filtered_memory_map, desc_size,
>                           filtered_memory_map_end,
> -                         BYTES_TO_PAGES (required_bytes));
> +                         BYTES_TO_PAGES (required_bytes),
> +                         flags & GRUB_MM_REGION_CONSECUTIVE);
>    if (err != GRUB_ERR_NONE)
>      return err;
>
> @@ -641,8 +647,9 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
>  void
>  grub_efi_mm_init (void)
>  {
> -  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE) != GRUB_ERR_NONE)
> +  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE, 0) != GRUB_ERR_NONE)

s/0/GRUB_MM_REGION_NONE/?

>      grub_fatal (grub_errmsg);
> +  grub_mm_region_fn = grub_efi_mm_add_regions;

s/grub_mm_region_fn/grub_mm_add_region_fn/ Of course this change should
be done in the earlier patch.

Daniel