Page MenuHomeFreeBSD
Differential D29276

smbios: support getting address from EFI
ClosedPublic
Actions

Authored by val_packett.cool on Mar 15 2021, 8:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Sep 11, 10:10 AM
Unknown Object (File)
Fri, Sep 11, 5:05 AM
Unknown Object (File)
Wed, Sep 9, 8:06 AM
Unknown Object (File)
Tue, Sep 1, 8:53 PM
Unknown Object (File)
Tue, Sep 1, 7:42 PM
Unknown Object (File)
Tue, Sep 1, 7:35 PM
Unknown Object (File)
Tue, Sep 1, 7:28 PM
Unknown Object (File)
Tue, Sep 1, 7:07 PM
View All Files
Subscribers
ambrisko
andrew
dab
emaste
imp
vangyzen

Details

Reviewers
allanjude
rpokala
manu
vangyzen
dab
Group Reviewers
Contributor Reviews (src)
Commits
rG4cfd4d54822b: smbios: support getting address from EFI
rGa29bff7a5216: smbios: support getting address from EFI
Summary

On some systems (e.g. Lenovo ThinkPad X240, Apple MacBookPro12,1) the SMBIOS entry point is not found in the <0xFFFFF space.

Follow the SMBIOS spec and use the EFI Configuration Table for locating the entry point on EFI systems.

I'm not sure if I need any other conditionals to like, exclude kernels built without EFI support… (if anyone cares about that)

Test Plan

Boot on various systems that didn't attach smbios before, observe smbios attaching with fancy high addresses.

Thinkpad:

[17] smbios0: <System Management BIOS> at iomem 0xbcd3d000-0xbcd3d01e
[17] smbios0: Version: 2.7, BCD Revision: 2.7

MacBook:

[91] smbios0: <System Management BIOS> at iomem 0x8afad000-0x8afad01e
[91] smbios0: Version: 2.7

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

val_packett.cool requested review of this revision.Mar 15 2021, 8:53 PM
val_packett.cool created this revision.
val_packett.cool updated this revision to Diff 85795.Mar 15 2021, 9:02 PM

oops, forgot efi.h change

sys/sys/efi.h
41

These don't seem to be used anywhere in the tree.

rpokala added a subscriber: ambrisko.Mar 15 2021, 9:10 PM

This looks grossly correct to me, but it's been a long time since I've thought about this stuff. IIRC @ambrisko did some stuff with SMBIOS a ways back; Doug?

val_packett.cool added a child revision: D29277: smbios: expose useful DMI information as sysctl and dmesg.Mar 15 2021, 9:16 PM
ambrisko added a comment.Mar 16 2021, 5:11 PM

Has this been testing on systems without UEFI BIOS? I'm not sure what happens if efirt is loaded on a non-UEFI system? Please include testing on non UEFI system and UEFI system. QEMU can be used to test in legacy BIOS mode. Please include testing results from both environments. FYI, the loader should set hint.smbios.0.mem in UEFI mode so that can be used a quick check on the address. It is good to verify the address is not in the BIOS range when testing. Some UEFI systems have a combined legacy and UEFI env.

vangyzen added a subscriber: vangyzen.Mar 31 2021, 3:46 PM
In D29276#655921, @ambrisko wrote:

I'm not sure what happens if efirt is loaded on a non-UEFI system?

options EFIRT is in GENERIC.

sys/dev/smbios/smbios.c
37

It would be nice to spell this #if ARCH_MAY_USE_EFI so this condition doesn't proliferate. Every arch already has <machine/efi.h>, so it would be easy to add.

84

I would prefer to put this on the stack instead of permanently wasting 16 bytes.

90

This should go under the same #if as efi_smbios.

dab added a subscriber: dab.Mar 31 2021, 6:22 PM

Looks pretty good. I would agree that it should be tested on a LEGACY (non-UEFI) BIOS.

sys/dev/smbios/smbios.c
37

+1

val_packett.cool updated this revision to Diff 86636.Mar 31 2021, 6:47 PM
  • options EFIRT is not in MINIMAL, so now device smbios can't be either, so kldload smbios would be the way to go in MINIMAL
  • address feedback
Herald added a reviewer: manu. · View Herald TranscriptMar 31 2021, 6:47 PM
Herald added subscribers: emaste, andrew. · View Herald Transcript
vangyzen accepted this revision.Mar 31 2021, 10:11 PM
vangyzen added inline comments.
sys/dev/smbios/smbios.c
50

<sys/efi.h> already includes <machine/efi.h>, so this is not needed.

This revision is now accepted and ready to land.Mar 31 2021, 10:11 PM
val_packett.cool updated this revision to Diff 86683.Apr 1 2021, 1:29 PM

oops, thought I did remove that include, but somehow didn't

This revision now requires review to proceed.Apr 1 2021, 1:29 PM
vangyzen accepted this revision.Apr 2 2021, 3:24 PM

@greg_unrelenting.technology Please look back over the comments from @ambrisko . In particular, I'd like to see testing on a non-UEFI system.

@rpokala or @ambrisko Would you like to take another look?

This revision is now accepted and ready to land.Apr 2 2021, 3:24 PM
dab accepted this revision as: dab.Apr 2 2021, 4:15 PM
rpokala added a comment.Apr 5 2021, 10:16 PM

Code LGTM. ==@ambrisko WRT testing on legacy BIOS platforms.

vangyzen added a comment.EditedApr 7 2021, 7:39 PM

smbios still attaches on a bhyve VM running FreeBSD main plus this change, booting in Legacy BIOS mode. The device address did not change, of course.

smbios0: <System Management BIOS> at iomem 0xf1000-0xf101e
smbios0: Version: 2.6, BCD Revision: 2.6
Closed by commit rGa29bff7a5216: smbios: support getting address from EFI (authored by val_packett.cool, committed by vangyzen). · Explain WhyApr 7 2021, 7:53 PM
This revision was automatically updated to reflect the committed changes.
vangyzen added a commit: rGa29bff7a5216: smbios: support getting address from EFI.
yinlong.lu_dell.com mentioned this in D30007: ipmi: support getting address from EFI.Apr 27 2021, 6:23 AM
vangyzen added a commit: rG4cfd4d54822b: smbios: support getting address from EFI.Mar 3 2022, 2:21 PM
Herald added a subscriber: imp. · View Herald TranscriptMar 3 2022, 2:21 PM

Revision Contents
Changeset List

PathSize
sys/
amd64/
conf/
1 line
2 lines
include/
1 line
arm64/
include/
2 lines
dev/
smbios/
22 lines
sys/
8 lines

Diff 86683

View Options

sys/amd64/conf/MINIMAL

Loading...
View Options

sys/amd64/conf/NOTES

Loading...
View Options

sys/amd64/include/efi.h

Loading...
View Options

sys/arm64/include/efi.h

Loading...
View Options

sys/dev/smbios/smbios.c

Loading...
View Options

sys/sys/efi.h

Loading...