svn commit: r344839 - head/stand/efi/loader
Rebecca Cran
bcran at FreeBSD.org
Wed Mar 6 05:39:41 UTC 2019
Author: bcran
Date: Wed Mar 6 05:39:40 2019
New Revision: 344839
URL: https://svnweb.freebsd.org/changeset/base/344839
Log:
Add retry loop around GetMemoryMap call to fix fragmentation bug
The call to BS->AllocatePages can cause the memory map to become framented,
causing BS->GetMemoryMap to return EFI_BUFFER_TOO_SMALL more than once. For
example this can happen on the MinnowBoard Turbot, causing the boot to stop
with an error. Avoid this by calling GetMemoryMap in a loop.
Reviewed by: imp, tsoome, kevans
Differential Revision: https://reviews.freebsd.org/D19341
Modified:
head/stand/efi/loader/bootinfo.c
head/stand/efi/loader/copy.c
Modified: head/stand/efi/loader/bootinfo.c
==============================================================================
--- head/stand/efi/loader/bootinfo.c Wed Mar 6 02:52:58 2019 (r344838)
+++ head/stand/efi/loader/bootinfo.c Wed Mar 6 05:39:40 2019 (r344839)
@@ -287,12 +287,12 @@ static int
bi_load_efi_data(struct preloaded_file *kfp)
{
EFI_MEMORY_DESCRIPTOR *mm;
- EFI_PHYSICAL_ADDRESS addr;
+ EFI_PHYSICAL_ADDRESS addr = 0;
EFI_STATUS status;
const char *efi_novmap;
size_t efisz;
UINTN efi_mapkey;
- UINTN mmsz, pages, retry, sz;
+ UINTN dsz, pages, retry, sz;
UINT32 mmver;
struct efi_map_header *efihdr;
bool do_vmap;
@@ -323,76 +323,94 @@ bi_load_efi_data(struct preloaded_file *kfp)
efisz = (sizeof(struct efi_map_header) + 0xf) & ~0xf;
/*
- * Assgin size of EFI_MEMORY_DESCRIPTOR to keep compatible with
+ * Assign size of EFI_MEMORY_DESCRIPTOR to keep compatible with
* u-boot which doesn't fill this value when buffer for memory
* descriptors is too small (eg. 0 to obtain memory map size)
*/
- mmsz = sizeof(EFI_MEMORY_DESCRIPTOR);
+ dsz = sizeof(EFI_MEMORY_DESCRIPTOR);
/*
- * It is possible that the first call to ExitBootServices may change
- * the map key. Fetch a new map key and retry ExitBootServices in that
- * case.
+ * Allocate enough pages to hold the bootinfo block and the
+ * memory map EFI will return to us. The memory map has an
+ * unknown size, so we have to determine that first. Note that
+ * the AllocatePages call can itself modify the memory map, so
+ * we have to take that into account as well. The changes to
+ * the memory map are caused by splitting a range of free
+ * memory into two, so that one is marked as being loader
+ * data.
*/
+
+ sz = 0;
+
+ /*
+ * Matthew Garrett has observed at least one system changing the
+ * memory map when calling ExitBootServices, causing it to return an
+ * error, probably because callbacks are allocating memory.
+ * So we need to retry calling it at least once.
+ */
for (retry = 2; retry > 0; retry--) {
- /*
- * Allocate enough pages to hold the bootinfo block and the
- * memory map EFI will return to us. The memory map has an
- * unknown size, so we have to determine that first. Note that
- * the AllocatePages call can itself modify the memory map, so
- * we have to take that into account as well. The changes to
- * the memory map are caused by splitting a range of free
- * memory into two (AFAICT), so that one is marked as being
- * loader data.
- */
- sz = 0;
- BS->GetMemoryMap(&sz, NULL, &efi_mapkey, &mmsz, &mmver);
- sz += mmsz;
- sz = (sz + 0xf) & ~0xf;
- pages = EFI_SIZE_TO_PAGES(sz + efisz);
- status = BS->AllocatePages(AllocateAnyPages, EfiLoaderData,
- pages, &addr);
- if (EFI_ERROR(status)) {
- printf("%s: AllocatePages error %lu\n", __func__,
- EFI_ERROR_CODE(status));
- return (ENOMEM);
- }
+ for (;;) {
+ status = BS->GetMemoryMap(&sz, mm, &efi_mapkey, &dsz, &mmver);
+ if (!EFI_ERROR(status))
+ break;
- /*
- * Read the memory map and stash it after bootinfo. Align the
- * memory map on a 16-byte boundary (the bootinfo block is page
- * aligned).
- */
- efihdr = (struct efi_map_header *)(uintptr_t)addr;
- mm = (void *)((uint8_t *)efihdr + efisz);
- sz = (EFI_PAGE_SIZE * pages) - efisz;
+ if (status != EFI_BUFFER_TOO_SMALL) {
+ printf("%s: GetMemoryMap error %lu\n", __func__,
+ EFI_ERROR_CODE(status));
+ return (EINVAL);
+ }
- status = BS->GetMemoryMap(&sz, mm, &efi_mapkey, &mmsz, &mmver);
- if (EFI_ERROR(status)) {
- printf("%s: GetMemoryMap error %lu\n", __func__,
- EFI_ERROR_CODE(status));
- return (EINVAL);
- }
- status = BS->ExitBootServices(IH, efi_mapkey);
- if (EFI_ERROR(status) == 0) {
+ if (addr != 0)
+ BS->FreePages(addr, pages);
+
+ /* Add 10 descriptors to the size to allow for
+ * fragmentation caused by calling AllocatePages */
+ sz += (10 * dsz);
+ pages = EFI_SIZE_TO_PAGES(sz + efisz);
+ status = BS->AllocatePages(AllocateAnyPages, EfiLoaderData,
+ pages, &addr);
+ if (EFI_ERROR(status)) {
+ printf("%s: AllocatePages error %lu\n", __func__,
+ EFI_ERROR_CODE(status));
+ return (ENOMEM);
+ }
+
/*
- * This may be disabled by setting efi_disable_vmap in
- * loader.conf(5). By default we will setup the virtual
- * map entries.
+ * Read the memory map and stash it after bootinfo. Align the
+ * memory map on a 16-byte boundary (the bootinfo block is page
+ * aligned).
*/
- if (do_vmap)
- efi_do_vmap(mm, sz, mmsz, mmver);
- efihdr->memory_size = sz;
- efihdr->descriptor_size = mmsz;
- efihdr->descriptor_version = mmver;
- file_addmetadata(kfp, MODINFOMD_EFI_MAP, efisz + sz,
- efihdr);
- return (0);
+ efihdr = (struct efi_map_header *)(uintptr_t)addr;
+ mm = (void *)((uint8_t *)efihdr + efisz);
+ sz = (EFI_PAGE_SIZE * pages) - efisz;
}
+
+ status = BS->ExitBootServices(IH, efi_mapkey);
+ if (!EFI_ERROR(status))
+ break;
+ }
+
+ if (retry == 0) {
BS->FreePages(addr, pages);
+ printf("ExitBootServices error %lu\n", EFI_ERROR_CODE(status));
+ return (EINVAL);
}
- printf("ExitBootServices error %lu\n", EFI_ERROR_CODE(status));
- return (EINVAL);
+
+ /*
+ * This may be disabled by setting efi_disable_vmap in
+ * loader.conf(5). By default we will setup the virtual
+ * map entries.
+ */
+
+ if (do_vmap)
+ efi_do_vmap(mm, sz, dsz, mmver);
+ efihdr->memory_size = sz;
+ efihdr->descriptor_size = dsz;
+ efihdr->descriptor_version = mmver;
+ file_addmetadata(kfp, MODINFOMD_EFI_MAP, efisz + sz,
+ efihdr);
+
+ return (0);
}
/*
Modified: head/stand/efi/loader/copy.c
==============================================================================
--- head/stand/efi/loader/copy.c Wed Mar 6 02:52:58 2019 (r344838)
+++ head/stand/efi/loader/copy.c Wed Mar 6 05:39:40 2019 (r344839)
@@ -95,7 +95,7 @@ static void
efi_verify_staging_size(unsigned long *nr_pages)
{
UINTN sz;
- EFI_MEMORY_DESCRIPTOR *map, *p;
+ EFI_MEMORY_DESCRIPTOR *map = NULL, *p;
EFI_PHYSICAL_ADDRESS start, end;
UINTN key, dsz;
UINT32 dver;
@@ -104,17 +104,28 @@ efi_verify_staging_size(unsigned long *nr_pages)
unsigned long available_pages = 0;
sz = 0;
- status = BS->GetMemoryMap(&sz, 0, &key, &dsz, &dver);
- if (status != EFI_BUFFER_TOO_SMALL) {
- printf("Can't determine memory map size\n");
- return;
- }
- map = malloc(sz);
- status = BS->GetMemoryMap(&sz, map, &key, &dsz, &dver);
- if (EFI_ERROR(status)) {
- printf("Can't read memory map\n");
- goto out;
+ for (;;) {
+ status = BS->GetMemoryMap(&sz, map, &key, &dsz, &dver);
+ if (!EFI_ERROR(status))
+ break;
+
+ if (status != EFI_BUFFER_TOO_SMALL) {
+ printf("Can't read memory map: %lu\n",
+ EFI_ERROR_CODE(status));
+ goto out;
+ }
+
+ free(map);
+
+ /* Allocate 10 descriptors more than the size reported,
+ * to allow for any fragmentation caused by calling
+ * malloc */
+ map = malloc(sz + (10 * dsz));
+ if (map == NULL) {
+ printf("Unable to allocate memory\n");
+ goto out;
+ }
}
ndesc = sz / dsz;
More information about the svn-src-all
mailing list