grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] efi: add central copy of grub_efi_find_mmap_size


From: Daniel Kiper
Subject: Re: [PATCH 3/4] efi: add central copy of grub_efi_find_mmap_size
Date: Wed, 13 Sep 2017 06:39:11 +0200
User-agent: Mutt/1.3.28i

On Tue, Sep 05, 2017 at 09:41:13PM +0100, Leif Lindholm wrote:
> There are several implementations of this function in the tree.
> Add a central version in grub-core/efi/mm.c.
>
> Taken from grub-core/loader/i386/linux.c, changing some hard-coded constants
> to use macros from efi/memory.h.

I am OK with the idea but...

> Signed-off-by: Leif Lindholm <address@hidden>
> ---
>  grub-core/kern/efi/mm.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/grub/efi/efi.h  |  1 +
>  2 files changed, 48 insertions(+)
>
> diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> index ac2a4c556..8795aa1e0 100644
> --- a/grub-core/kern/efi/mm.c
> +++ b/grub-core/kern/efi/mm.c
> @@ -218,6 +218,53 @@ grub_efi_finish_boot_services (grub_efi_uintn_t 
> *outbuf_size, void *outbuf,
>    return GRUB_ERR_NONE;
>  }
>
> +/* To obtain the UEFI memory map, we must pass a buffer of sufficient size
> +   to hold the entire map. This function returns a sane start value for
> +   buffer size.  */
> +grub_efi_uintn_t
> +grub_efi_find_mmap_size (void)
> +{
> +  static grub_efi_uintn_t mmap_size = 0;
> +
> +  if (mmap_size != 0)
> +    return mmap_size;

What will happen if memory will be fragmented further after this call
and number of memory map entries increases above mmap_size?

> +  mmap_size = 1 * GRUB_EFI_PAGE_SIZE;
> +  while (1)
> +    {
> +      int ret;
> +      grub_efi_memory_descriptor_t *mmap;
> +      grub_efi_uintn_t desc_size;
> +      grub_efi_uintn_t cur_mmap_size = mmap_size;
> +
> +      mmap = grub_malloc (cur_mmap_size);
> +      if (! mmap)
> +     return 0;
> +
> +      ret = grub_efi_get_memory_map (&cur_mmap_size, mmap, 0, &desc_size, 0);
> +      grub_free (mmap);
> +
> +      if (ret < 0)
> +     {
> +       grub_error (GRUB_ERR_IO, "cannot get memory map");
> +       return 0;
> +     }
> +      else if (ret > 0)
> +     break;
> +
> +      if (mmap_size < cur_mmap_size)
> +     mmap_size = cur_mmap_size;
> +      mmap_size += GRUB_EFI_PAGE_SIZE;
> +    }
> +
> +  /* Increase the size a bit for safety, because GRUB allocates more on
> +     later, and EFI itself may allocate more.  */
> +  mmap_size += 3 * GRUB_EFI_PAGE_SIZE;
> +
> +  mmap_size = ALIGN_UP (mmap_size, GRUB_EFI_PAGE_SIZE);

I prefer if you do something like that:

map_size = 0;

if (grub_efi_get_memory_map (&map_size, NULL, NULL, &desc_size, 0) < 0)
{
  grub_error (GRUB_ERR_IO, "cannot get EFI memory map size");
  return 0;
}

return ALIGN_UP (map_size + GRUB_EFI_PAGE_SIZE, GRUB_EFI_PAGE_SIZE);

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]