Bug 202455 - [EFI] boot/loader.efi: ExitBootServices() returned 0x8000000000000002
Summary: [EFI] boot/loader.efi: ExitBootServices() returned 0x8000000000000002
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: amd64 Any
: --- Affects Many People
Assignee: Ed Maste
URL: https://reviews.freebsd.org/D4296
Keywords: patch, uefi
Depends on:
Blocks: 196289 203349
  Show dependency treegraph
 
Reported: 2015-08-19 10:31 UTC by Ganael LAPLANCHE
Modified: 2016-01-07 04:06 UTC (History)
6 users (show)

See Also:


Attachments
patch-sys-boot-efi-loader-arch-amd64-elf64_freebsd.c (1.62 KB, patch)
2015-08-19 10:31 UTC, Ganael LAPLANCHE
no flags Details | Diff
patch-ExitBootServices_v2.txt (4.51 KB, patch)
2015-11-25 09:55 UTC, Ganael LAPLANCHE
no flags Details | Diff
loader.efi built from diff 10892 in review D4296 (353.28 KB, application/x-ms-dos-executable)
2015-12-07 20:52 UTC, Ed Maste
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ganael LAPLANCHE 2015-08-19 10:31:07 UTC
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
Comment 1 bpurgar 2015-08-23 12:27:35 UTC
This patch fix problem on toshiba c55-a-1nk.
Comment 2 Ed Maste freebsd_committer freebsd_triage 2015-10-27 12:54:34 UTC
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.
Comment 3 John Baldwin freebsd_committer freebsd_triage 2015-10-27 16:38:17 UTC
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.
Comment 4 John Baldwin freebsd_committer freebsd_triage 2015-10-27 16:38:45 UTC
Adding Lawrence who had a machine with this issue yesterday.
Comment 5 Ganael LAPLANCHE 2015-11-25 09:54:35 UTC
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.
Comment 6 Ganael LAPLANCHE 2015-11-25 09:55:27 UTC
Created attachment 163513 [details]
patch-ExitBootServices_v2.txt
Comment 7 Ed Maste freebsd_committer freebsd_triage 2015-11-27 02:14:10 UTC
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.
Comment 8 Ed Maste freebsd_committer freebsd_triage 2015-12-07 20:52:55 UTC
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.
Comment 9 commit-hook freebsd_committer freebsd_triage 2015-12-16 16:20:13 UTC
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
Comment 10 commit-hook freebsd_committer freebsd_triage 2015-12-20 16:07:36 UTC
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
Comment 11 commit-hook freebsd_committer freebsd_triage 2016-01-07 04:03:26 UTC
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