| Message ID | 20170905204114.9462-4-leif.lindholm@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | x86,efi: prerequisites for ARM* cleanup series | expand |
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 <leif.lindholm@linaro.org> > --- > 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 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
On Wed, Sep 13, 2017 at 06:39:11AM +0200, Daniel Kiper wrote: > 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 <leif.lindholm@linaro.org> > > --- > > 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? Oh, that is completely broken, but there is no functional change introduced by this patch. I was just hoping there was some point at which I could stop fixing the world in order to get to the point where upstream grub is able to boot on arm64 systems with large amounts of memory, or holes in the memory map. A journey I commenced on February 28th this year: http://lists.gnu.org/archive/html/grub-devel/2017-02/msg00156.html I note that this patch was in itself triggered by your feedback on "efi: add grub_efi_get_dram_base() function for arm*" on 27 July. / Leif > > + 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 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
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; + + 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); + return mmap_size; +} + /* Get the memory map as defined in the EFI spec. Return 1 if successful, return 0 if partial, or return -1 if an error occurs. */ int diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h index 3fa082816..8f1f36c8c 100644 --- a/include/grub/efi/efi.h +++ b/include/grub/efi/efi.h @@ -49,6 +49,7 @@ void * EXPORT_FUNC(grub_efi_allocate_any_pages) (grub_efi_uintn_t pages); void EXPORT_FUNC(grub_efi_free_pages) (grub_efi_physical_address_t address, grub_efi_uintn_t pages); +grub_efi_uintn_t EXPORT_FUNC(grub_efi_find_mmap_size) (void); int EXPORT_FUNC(grub_efi_get_memory_map) (grub_efi_uintn_t *memory_map_size, grub_efi_memory_descriptor_t *memory_map,
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. Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> --- grub-core/kern/efi/mm.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ include/grub/efi/efi.h | 1 + 2 files changed, 48 insertions(+) -- 2.11.0 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel