[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