[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/7] efi: add grub_efi_get_dram_base() function for arm*
From: |
Daniel Kiper |
Subject: |
Re: [PATCH 1/7] efi: add grub_efi_get_dram_base() function for arm* |
Date: |
Thu, 27 Jul 2017 16:27:26 +0200 |
User-agent: |
Mutt/1.3.28i |
On Mon, Jun 12, 2017 at 03:53:35PM +0100, Leif Lindholm wrote:
> Since ARM platforms do not have a common memory map, add a helper
> function that finds the lowest address region with the EFI_MEMORY_WB
> attribute set in the UEFI memory map.
>
> Required for the arm/arm64 linux loader to restrict the initrd
> location to where it will be accessible by the kernel at runtime.
>
> Signed-off-by: Leif Lindholm <address@hidden>
> ---
> grub-core/kern/efi/mm.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> include/grub/efi/efi.h | 1 +
> 2 files changed, 43 insertions(+)
>
> diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> index 20a47aaf5..460a4b763 100644
> --- a/grub-core/kern/efi/mm.c
> +++ b/grub-core/kern/efi/mm.c
> @@ -525,3 +525,45 @@ grub_efi_mm_init (void)
> grub_efi_free_pages ((grub_addr_t) memory_map,
> 2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
> }
> +
> +#if defined (__arm__) || defined (__aarch64__)
> +grub_err_t
> +grub_efi_get_dram_base(grub_addr_t *base_addr)
Please make this function more generic and get
attribute as an argument.
> +{
> + grub_efi_memory_descriptor_t *memory_map;
> + grub_efi_memory_descriptor_t *desc;
> + grub_efi_uintn_t mmap_size;
> + grub_efi_uintn_t desc_size;
> +
> + mmap_size = (1 << GRUB_EFI_PAGE_SHIFT);
Could not you define GRUB_EFI_PAGE and use it here?
And why GRUB_EFI_PAGE_SHIFT and other stuff is defined
in include/grub/arm64/fdtload.h? It looks like generic
thing and should be in genric place too. Please fix it
if possible.
> + while (1)
> + {
> + int ret;
> +
> + memory_map = grub_malloc (mmap_size);
> + if (! memory_map)
> + return GRUB_ERR_OUT_OF_MEMORY;
> + ret = grub_efi_get_memory_map (&mmap_size, memory_map, NULL,
> + &desc_size, NULL);
> + if (ret > 0)
> + break;
> +
> + grub_free (memory_map);
> + if (ret == 0)
> + return GRUB_ERR_BUG;
> +
> + mmap_size += (1 << GRUB_EFI_PAGE_SHIFT);
> + }
Hmmm... This is not optimal. Please take a look at e.g.
grub_efi_finish_boot_services() how buffer for memory
map should be allocated.
And going further... Could you take a look at
grub_relocator_alloc_chunk_align() and
grub_relocator_alloc_chunk_addr(). Good example
is in grub-core/loader/multiboot_mbi2.c and
grub-core/loader/multiboot_elfxx.c. If you
use this machinery then there is a chance
that you do not duplicate code so much.
Daniel
- Re: [PATCH 1/7] efi: add grub_efi_get_dram_base() function for arm*,
Daniel Kiper <=