Tom Huibregtse
2020-Jun-11 13:04 UTC
[syslinux] [PATCH] efi/main: add retry to exit_boot()
Sometimes the UEFI memory map changes between GetMemoryMap() and ExitBootServices(), making the MapKey value incorrect. Per the UEFI spec, the memory map should then be fetched again and exiting retried. Signed-off-by: Tom Huibregtse <thuibregtse at xes-inc.com> --- efi/main.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 11 deletions(-) diff --git a/efi/main.c b/efi/main.c index 6a748412..d7fb637e 100644 --- a/efi/main.c +++ b/efi/main.c @@ -1008,16 +1008,81 @@ static int exit_boot(struct boot_params *bp) EFI_MEMORY_DESCRIPTOR *map; EFI_STATUS status; uint32_t e820_type; - UINTN i, nr_entries, key, desc_sz; + UINTN i, nr_entries, key, desc_sz, buf_sz, map_sz; UINT32 desc_ver; + /* + * Call once with empty buffer size to + * see how large the buffer should be. + */ + buf_sz = 0; + status = uefi_call_wrapper(BS->GetMemoryMap, + 5, + &buf_sz, + map, + &key, + &desc_sz, + &desc_ver); + if (!buf_sz) + return -1; + + /* + * Allocate a pool double the size of the map in + * case if the first call to ExitBootServices() + * fails and the memory map grows before + * GetMemoryMap() is called again. + */ + buf_sz *= 2; + map = AllocatePool(buf_sz); + /* Build efi memory map */ - map = get_memory_map(&nr_entries, &key, &desc_sz, &desc_ver); - if (!map) + map_sz = buf_sz; + status = uefi_call_wrapper(BS->GetMemoryMap, + 5, + &map_sz, + map, + &key, + &desc_sz, + &desc_ver); + if (!map) { + FreePool(map); + return -1; + } + + status = uefi_call_wrapper(BS->ExitBootServices, 2, image_handle, key); + + if (status == EFI_INVALID_PARAMETER) { + /* + * Retry GetMemoryMap() and ExitBootServices() if the + * first try fails per UEFI Spec v2.6, Section 6.4: + * EFI_BOOT_SERVICES.ExitBootServices. + */ + map_sz = buf_sz; + status = uefi_call_wrapper(BS->GetMemoryMap, + 5, + &map_sz, + map, + &key, + &desc_sz, + &desc_ver); + + /* FreePool() cannot be called after ExitBootServices() */ + if (status != EFI_SUCCESS) + return -1; + + status = uefi_call_wrapper(BS->ExitBootServices, + 2, + image_handle, + key); + } + + if (status != EFI_SUCCESS) { + printf("Failed to exit boot services: 0x%016lx\n", status); return -1; + } bp->efi.memmap = (uint32_t)(unsigned long)map; - bp->efi.memmap_size = nr_entries * desc_sz; + bp->efi.memmap_size = map_sz; bp->efi.systab = (uint32_t)(unsigned long)ST; bp->efi.desc_size = desc_sz; bp->efi.desc_version = desc_ver; @@ -1033,6 +1098,7 @@ static int exit_boot(struct boot_params *bp) * e820 map because it will most likely have changed in the * interim. */ + nr_entries = map_sz / desc_sz; e = e820buf = bp->e820_map; for (i = 0; i < nr_entries && i < E820MAX; i++) { struct e820_entry *prev = NULL; @@ -1089,13 +1155,6 @@ static int exit_boot(struct boot_params *bp) bp->e820_entries = e - e820buf; - status = uefi_call_wrapper(BS->ExitBootServices, 2, image_handle, key); - if (status != EFI_SUCCESS) { - printf("Failed to exit boot services: 0x%016lx\n", status); - FreePool(map); - return -1; - } - return 0; } -- 2.27.0
Tom Huibregtse
2020-Jun-18 20:24 UTC
[syslinux] [PATCH] efi/main: add retry to exit_boot()
I am a UEFI/BIOS developer. We UEFI PXE boot dozens of times per night. We have run into this error more than a couple of times. We have also done thousands of UEFI PXE boots with debug statements to prove that the patch works. From: "Tom Huibregtse via Syslinux" <syslinux at syslinux.org> To: syslinux at syslinux.org Sent: Thursday, June 11, 2020 8:04:06 AM Subject: [syslinux] [PATCH] efi/main: add retry to exit_boot() Sometimes the UEFI memory map changes between GetMemoryMap() and ExitBootServices(), making the MapKey value incorrect. Per the UEFI spec, the memory map should then be fetched again and exiting retried. Signed-off-by: Tom Huibregtse <thuibregtse at xes-inc.com> --- efi/main.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 11 deletions(-) diff --git a/efi/main.c b/efi/main.c index 6a748412..d7fb637e 100644 --- a/efi/main.c +++ b/efi/main.c @@ -1008,16 +1008,81 @@ static int exit_boot(struct boot_params *bp) EFI_MEMORY_DESCRIPTOR *map; EFI_STATUS status; uint32_t e820_type; - UINTN i, nr_entries, key, desc_sz; + UINTN i, nr_entries, key, desc_sz, buf_sz, map_sz; UINT32 desc_ver; + /* + * Call once with empty buffer size to + * see how large the buffer should be. + */ + buf_sz = 0; + status = uefi_call_wrapper(BS->GetMemoryMap, + 5, + &buf_sz, + map, + &key, + &desc_sz, + &desc_ver); + if (!buf_sz) + return -1; + + /* + * Allocate a pool double the size of the map in + * case if the first call to ExitBootServices() + * fails and the memory map grows before + * GetMemoryMap() is called again. + */ + buf_sz *= 2; + map = AllocatePool(buf_sz); + /* Build efi memory map */ - map = get_memory_map(&nr_entries, &key, &desc_sz, &desc_ver); - if (!map) + map_sz = buf_sz; + status = uefi_call_wrapper(BS->GetMemoryMap, + 5, + &map_sz, + map, + &key, + &desc_sz, + &desc_ver); + if (!map) { + FreePool(map); + return -1; + } + + status = uefi_call_wrapper(BS->ExitBootServices, 2, image_handle, key); + + if (status == EFI_INVALID_PARAMETER) { + /* + * Retry GetMemoryMap() and ExitBootServices() if the + * first try fails per UEFI Spec v2.6, Section 6.4: + * EFI_BOOT_SERVICES.ExitBootServices. + */ + map_sz = buf_sz; + status = uefi_call_wrapper(BS->GetMemoryMap, + 5, + &map_sz, + map, + &key, + &desc_sz, + &desc_ver); + + /* FreePool() cannot be called after ExitBootServices() */ + if (status != EFI_SUCCESS) + return -1; + + status = uefi_call_wrapper(BS->ExitBootServices, + 2, + image_handle, + key); + } + + if (status != EFI_SUCCESS) { + printf("Failed to exit boot services: 0x%016lx\n", status); return -1; + } bp->efi.memmap = (uint32_t)(unsigned long)map; - bp->efi.memmap_size = nr_entries * desc_sz; + bp->efi.memmap_size = map_sz; bp->efi.systab = (uint32_t)(unsigned long)ST; bp->efi.desc_size = desc_sz; bp->efi.desc_version = desc_ver; @@ -1033,6 +1098,7 @@ static int exit_boot(struct boot_params *bp) * e820 map because it will most likely have changed in the * interim. */ + nr_entries = map_sz / desc_sz; e = e820buf = bp->e820_map; for (i = 0; i < nr_entries && i < E820MAX; i++) { struct e820_entry *prev = NULL; @@ -1089,13 +1155,6 @@ static int exit_boot(struct boot_params *bp) bp->e820_entries = e - e820buf; - status = uefi_call_wrapper(BS->ExitBootServices, 2, image_handle, key); - if (status != EFI_SUCCESS) { - printf("Failed to exit boot services: 0x%016lx\n", status); - FreePool(map); - return -1; - } - return 0; } -- 2.27.0 _______________________________________________ Syslinux mailing list Submissions to Syslinux at syslinux.org Unsubscribe or set options at: https://lists.syslinux.org/syslinux
Hi, Le 18/06/2020 ? 22:24, Tom Huibregtse via Syslinux a ?crit?:> I am a UEFI/BIOS developer. We UEFI PXE boot dozens of times per night. We have run into this error more than a couple of times. We have also done thousands of UEFI PXE boots with debug statements to prove that the patch works.<snip> As far as I can tell, the development of Syslinux has stalled since more than one year as shows the log of the git repository. And if I remember well many very old patches are still in the waiting list. If you don't want to continue maintaining your patches, either fork syslinux (good luck) or more realistically if possible move to an active project. GRUB comes to mind. Sorry not to give a more relevant answer. Have a good day, Didier