Page MenuHomeFreeBSD
Differential D56249

loader/efi: pre-allocate memory map buffer before ExitBootServices
AcceptedPublic
Actions

Authored by chris.longros_gmail.com on Fri, Apr 3, 10:44 PM.
Tags
None
Referenced Files
F151257432: D56249.diff
Tue, Apr 7, 4:02 AM
F151167837: D56249.diff
Mon, Apr 6, 1:33 PM
F151156205: D56249.diff
Mon, Apr 6, 11:52 AM
Unknown Object (File)
Sun, Apr 5, 4:32 PM
Unknown Object (File)
Sun, Apr 5, 8:17 AM
Subscribers
bcran
dab
imp

Details

Reviewers
manu
imp
Summary

Move the memory map allocation before the ExitBootServices retry
loop to ensure no boot service calls occur between GetMemoryMap
and ExitBootServices. This simplifies the control flow by removing
the inner sizing loop and matches the strategy used by the Linux
EFI stub.

Original motivation:
Identified while investigating an ExitBootServices hang on
AMD AGESA 1.3.0.0a firmware (Gigabyte B650 GAMING X AX V2).

Signed-off-by: Christos Longros <chris.longros@gmail.com>

Test Plan

Tested on FreeBSD 16.0-CURRENT (amd64), bare metal and QEMU.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 71981
Build 68864: arc lint + arc unit

Event Timeline

chris.longros_gmail.com created this revision.Fri, Apr 3, 10:44 PM
Herald added a reviewer: manu. · View Herald TranscriptFri, Apr 3, 10:44 PM
Herald added subscribers: dab, bcran, imp. · View Herald Transcript
chris.longros_gmail.com requested review of this revision.Fri, Apr 3, 10:44 PM
Harbormaster completed remote builds in B71955: Diff 174897.Fri, Apr 3, 10:44 PM
chris.longros_gmail.com added a reviewer: imp.Fri, Apr 3, 10:45 PM
chris.longros_gmail.com updated this revision to Diff 174898.Fri, Apr 3, 11:18 PM
chris.longros_gmail.com retitled this revision from loader/efi: harden ExitBootServices against stale memory map keys to loader/efi: fix UEFI spec violation in ExitBootServices retry loop.
chris.longros_gmail.com edited the summary of this revision. (Show Details)
chris.longros_gmail.com removed a reviewer: imp.

Reframe as UEFI spec compliance fix: remove unproven firmware hang claims, cite UEFI spec s7.4.6 requirements.

Harbormaster completed remote builds in B71956: Diff 174898.Fri, Apr 3, 11:18 PM
chris.longros_gmail.com added a reviewer: imp.Fri, Apr 3, 11:22 PM
imp added inline comments.Sat, Apr 4, 1:16 AM
stand/efi/loader/bootinfo.c
286

This is a bad idea. If 5 minutes isn't enough, it needs to be longer. But we can't go from here until when the kernel starts and arms the watchdog w/o wdog protection.

288

This i think is good.

chris.longros_gmail.com added inline comments.Sat, Apr 4, 6:04 AM
stand/efi/loader/bootinfo.c
286

This is a bad idea. If 5 minutes isn't enough, it needs to be longer. But we can't go from here until when the kernel starts and arms the watchdog w/o wdog protection.

What timeout value would you recommend?

chris.longros_gmail.com added inline comments.Mon, Apr 6, 11:28 AM
stand/efi/loader/bootinfo.c
286

Dropped the watchdog change

chris.longros_gmail.com updated this revision to Diff 174955.Mon, Apr 6, 11:31 AM

Drop the watchdog timer change.

Harbormaster completed remote builds in B71981: Diff 174955.Mon, Apr 6, 11:31 AM
chris.longros_gmail.com edited the summary of this revision. (Show Details)Mon, Apr 6, 11:34 AM
chris.longros_gmail.com edited the test plan for this revision. (Show Details)
chris.longros_gmail.com retitled this revision from loader/efi: fix UEFI spec violation in ExitBootServices retry loop to loader/efi: pre-allocate memory map buffer before ExitBootServices.
chris.longros_gmail.com added inline comments.Mon, Apr 6, 11:50 AM
stand/efi/loader/bootinfo.c
286

sorry for disabling the watchdog. That was totally wrong ...

chris.longros_gmail.com edited the summary of this revision. (Show Details)Mon, Apr 6, 12:06 PM
imp accepted this revision.Mon, Apr 6, 3:31 PM
In D56249#1287432, @chris.longros_gmail.com wrote:

Drop the watchdog timer change.

Thanks! I was going to suggest this was a configuration error in the pre-execution timeout configured in the firmware and the right place to fix that is in the config of systems that need more time.

This revision is now accepted and ready to land.Mon, Apr 6, 3:31 PM

Revision Contents
Changeset List

PathSize
stand/
efi/
loader/
85 lines

Diff 174955

View Options

stand/efi/loader/bootinfo.c

Loading...