Created attachment 160058 [details] patch-sys-boot-efi-loader-arch-amd64-elf64_freebsd.c Hi, As written here: https://wiki.freebsd.org/UEFI, "this issue is still encountered on some hardware.". Indeed, I have been able to reproduce the error on a physical machine (Supermicro X9SRi-F motherboard) when netbooting loader.efi over PXE. On that machine, this only happens when booting over PXE. I have absolutely no problem booting locally (so is the MapKey change causing the error related to the efinet layer ?). The UEFI specification advises to call ExitBootServices() again if the first call failed, after having updated the MapKey with a call to GetMemoryMap() just before the second ExitBootServices() call [1]. The FreeBSD loader only performs a single attempt and gives up if it fails. Find attached a patch (for amd64, against -CURRENT r286279) that implements this second attempt to hand over control to the kernel. It will probably be necessary to implement the same behaviour for other archs too. With this patch, my machine can boot over PXE in UEFI mode. Best regards, Ganael. [1] UEFI Specification Version v2.5, p221: http://www.uefi.org/sites/default/files/resources/UEFI%202_5.pdf
This patch fix problem on toshiba c55-a-1nk.
It's not quite that easy -- the memory map itself is passed as module metadata to the kernel (bi_load_efi_data), so that needs to be redone as well.
Yes, this is incorrect as if the memory map has actually changed the kernel will now possibly corrupt firmware data (or the firmware will corrupt OS/user data). However, what we might be able to do is move ExitBootServices() earlier. I don't think we use any EFI services in between when GetMemoryMap() is called now in bi_load() and when we call ExitBootServices(). We just shuffle memory around and construct the module metadata. I will need to look more. If we can fail to construct the module metadata then instead I think we will want to make bi_load() restartable and leave ExitBootServices() where it is, but try a restart.
Adding Lawrence who had a machine with this issue yesterday.
Hi, Here is a proposal (patch-ExitBootServices_v2.txt) that glues together calls to GetMemoryMap(), ExitBootServices() and storage of (now up-to-date) MODINFOMD_EFI_MAP metadata within a single function. That new function -bi_add_efi_data_and_exit()- uses space previously allocated in bi_load_efi_data() to store the memory map (it will fail if that space is too short). It handles re-calling GetMemoryMap() once to update the map key if necessary. Finally, if ExitBootServices() is successful, it stores the memory map and its header as MODINFOMD_EFI_MAP metadata. ExitBootServices() calls are now done earlier, from within arch-independent bi_load() code. The patch has only been tested on amd64 ; tests on other archs are welcome ! I have not modified i386 code as it seems to have its own bi_load() and separate code, but it may be good to make it benefit from supporting map key re-fetching if ExitBootServices() fails. Ganael.
Created attachment 163513 [details] patch-ExitBootServices_v2.txt
Thank you for the update! I've put it up on Phabricator for others to review, at https://reviews.freebsd.org/D4296. I'm running it on my laptop now. I'm not worried about i386 because i386 UEFI boot does not work now and it will probably be refactored to match amd64/arm64 when we're ready to move ahead with it. On arm the printfs() cannot be after ExitBootServices, but having them between GetMemoryMap and ExitBootServices was also not correct (as they were prior to this change). I'm willing to commit this change and address that after.
Created attachment 163954 [details] loader.efi built from diff 10892 in review D4296 Attached is a copy of loader.efi built from the most recent diff in review D4296, in case someone is willing to test on a previously-failing system.
A commit references this bug: Author: emaste Date: Wed Dec 16 16:19:18 UTC 2015 New revision: 292338 URL: https://svnweb.freebsd.org/changeset/base/292338 Log: UEFI: combine GetMemoryMap and ExitBootServices and retry on error The EFI memory map may change before or during the first ExitBootServices call. In that case ExitBootServices returns an error, and GetMemoryMap and ExitBootServices must be retried. Glue together calls to GetMemoryMap(), ExitBootServices() and storage of (now up-to-date) MODINFOMD_EFI_MAP metadata within a single function. That new function - bi_add_efi_data_and_exit() - uses space previously allocated in bi_load_efi_data() to store the memory map (it will fail if that space is too short). It handles re-calling GetMemoryMap() once to update the map key if necessary. Finally, if ExitBootServices() is successful, it stores the memory map and its header as MODINFOMD_EFI_MAP metadata. ExitBootServices() calls are now done earlier, from within arch- independent bi_load() code. PR: 202455 Submitted by: Ganael LAPLANCHE Reviewed by: kib MFC after: 2 weeks Relnotes: Yes Differential Revision: https://reviews.freebsd.org/D4296 Changes: head/sys/boot/efi/loader/arch/amd64/elf64_freebsd.c head/sys/boot/efi/loader/arch/arm/exec.c head/sys/boot/efi/loader/arch/arm64/exec.c head/sys/boot/efi/loader/bootinfo.c head/sys/boot/efi/loader/loader_efi.h
A commit references this bug: Author: emaste Date: Sun Dec 20 16:07:09 UTC 2015 New revision: 292515 URL: https://svnweb.freebsd.org/changeset/base/292515 Log: loader.efi: refresh size in GetMemoryMap retry loop If ExitBootServices fails due to a changed efi_mapkey then GetMemoryMap must be called again. In this case it is also possible for the memory map to grow, so repeat the initial GetMemoryMap call to fetch the new size. Also roll bi_add_efi_data_and_exit into bi_load_efi_data as there's no need for it to be a separate function. PR: 202455 Reported by: Berislav Purgar <bpurgar@gmail.com> Tested by: Berislav Purgar <bpurgar@gmail.com> Reviewed by: kib MFC after: 1 week MFC with: r292338 Relnotes: Yes Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D4621 Changes: head/sys/boot/efi/loader/bootinfo.c
A commit references this bug: Author: emaste Date: Thu Jan 7 04:02:38 UTC 2016 New revision: 293304 URL: https://svnweb.freebsd.org/changeset/base/293304 Log: loader.efi: combine GetMemoryMap and ExitBootServices and retry on error MFC r292338: UEFI: combine GetMemoryMap and ExitBootServices and retry on error The EFI memory map may change before or during the first ExitBootServices call. In that case ExitBootServices returns an error, and GetMemoryMap and ExitBootServices must be retried. Glue together calls to GetMemoryMap(), ExitBootServices() and storage of (now up-to-date) MODINFOMD_EFI_MAP metadata within a single function. That new function - bi_add_efi_data_and_exit() - uses space previously allocated in bi_load_efi_data() to store the memory map (it will fail if that space is too short). It handles re-calling GetMemoryMap() once to update the map key if necessary. Finally, if ExitBootServices() is successful, it stores the memory map and its header as MODINFOMD_EFI_MAP metadata. ExitBootServices() calls are now done earlier, from within arch- independent bi_load() code. MFC r292442: loader.efi: show EFI error number, not full status value EFI return values set the high bit to indicate an error. The log messages changed here are printed only in the case of an error, so including the error bit is redundant. Also switch to decimal to match the error definitions (in sys/boot/efi/include/efierr.h). MFC r292515: loader.efi: refresh size in GetMemoryMap retry loop If ExitBootServices fails due to a changed efi_mapkey then GetMemoryMap must be called again. In this case it is also possible for the memory map to grow, so repeat the initial GetMemoryMap call to fetch the new size. Also roll bi_add_efi_data_and_exit into bi_load_efi_data as there's no need for it to be a separate function. PR: 202455 Relnotes: Yes Sponsored by: The FreeBSD Foundation Changes: _U stable/10/ stable/10/sys/boot/efi/loader/arch/amd64/elf64_freebsd.c stable/10/sys/boot/efi/loader/bootinfo.c stable/10/sys/boot/efi/loader/loader_efi.h