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

Authored by chris.longros_gmail.com on Fri, Apr 3, 10:44 PM.

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

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

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

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

Path

Size

stand/

efi/

loader/

bootinfo.c

85 lines

Diff 174955

View Options

stand/efi/loader/bootinfo.c

loader/efi: pre-allocate memory map buffer before ExitBootServicesAcceptedPublicActions