d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add retry loop around GetMemoryMap call to fix fragmentation bug

Browse Source 
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
This commit is contained in:
bcran 2019-03-06 05:39:40 +00:00
parent d06387205a
commit 8347d20afe
2 changed files with 99 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

136
stand/efi/loader/bootinfo.c
View File

@ -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);
}
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);
}
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) {
/*
* 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;
}
BS->FreePages(addr, pages);
status = BS->ExitBootServices(IH, efi_mapkey);
if (!EFI_ERROR(status))
break;
}
printf("ExitBootServices error %lu\n", EFI_ERROR_CODE(status));
return (EINVAL);
if (retry == 0) {
BS->FreePages(addr, pages);
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);
}
/*

33
stand/efi/loader/copy.c
View File

@ -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;