loader.efi: smbios: Favor the v3 (64-bit) entry point
ClosedPublic
Actions

Authored by olce on Mar 7 2025, 4:57 PM.

Details

Reviewers

emaste
imp
markj
manu

Commits

rG807d51be8040: loader.efi: smbios: Favor the v3 (64-bit) entry point
rG96f77576e9ea: loader.efi: smbios: Favor the v3 (64-bit) entry point

Summary

Be consistent with what we are now doing with non-EFI boot (but with the
difference that EFI runs in 64-bit mode on 64-bit platforms, so there is
no restriction that the v3 entry point should be below 4GB).

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

olce created this revision.Mar 7 2025, 4:57 PM

Herald added a reviewer: manu. · View Herald TranscriptMar 7 2025, 4:57 PM

Herald added subscribers: dab, bcran. · View Herald Transcript

olce requested review of this revision.Mar 7 2025, 4:57 PM

Harbormaster completed remote builds in B62832: Diff 152028.Mar 7 2025, 4:57 PM

olce added a parent revision: D49291: libsa: smbios_probe(): Strictly obey specified entry point address.Mar 7 2025, 4:59 PM

olce added a child revision: D49293: loader.kboot: smbios: Add a comment about v3 entry point being favored.

I'd be tempted to save an indentation level and do continue if this vendor table isn't SMBIOS*. But it's not a clear win, so I'm mentioning it passing for your consideration.

I'd also be tempted to remove the #if !arm ifdef, but that's clearly a different change. the original was there only for early-days u-boot UEFI which was a bit rough and not very standards conformant.

Thanks for all the cleanups.

stand/efi/loader/main.c
1193	Mayb

This revision is now accepted and ready to land.Mar 7 2025, 5:47 PM

markj accepted this revision.Mar 9 2025, 9:36 AM

markj added inline comments.

stand/efi/loader/main.c
1193	Not sure if I'm completing Warner's thought here, but the need to use `__unused` here suggests that the smbios search loop would be nice to keep in its own conditionally defined subroutine. That would make control flow a bit simpler too (just return as soon as we find a 64-bit EP).

In D49292#1123971, @imp wrote:

I'd be tempted to save an indentation level and do continue if this vendor table isn't SMBIOS*. But it's not a clear win, so I'm mentioning it passing for your consideration.

Besides removing an indentation level, I think it's a small win also in cognitive burden, as one can eliminate these cases in their mind when reading the rest of the loop. I'll do it.

I'd also be tempted to remove the #if !arm ifdef, but that's clearly a different change. the original was there only for early-days u-boot UEFI which was a bit rough and not very standards conformant.

Yes, a different change, and I have no hardware to test that on. I only have amd64 machines currently. But maybe it's possible to test such things with QEMU?

Thanks for all the cleanups.

No problem. Since I had to read the spec, I thought I'd take the opportunity to review what exists and make sure we are both quite flexible when possible but also quite conservative with respect to what is mandated.

olce marked 2 inline comments as done.Mar 11 2025, 11:01 AM

olce added inline comments.