Page MenuHomeFreeBSD

libsa: smbios: Use 64-bit entry point if table below 4GB on non-EFI boot
ClosedPublic

Authored by olce on Mar 7 2025, 4:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 18, 11:45 PM
Unknown Object (File)
Tue, Apr 15, 12:40 PM
Unknown Object (File)
Sun, Apr 6, 9:48 AM
Unknown Object (File)
Mar 20 2025, 9:17 AM
Unknown Object (File)
Mar 14 2025, 8:10 PM
Unknown Object (File)
Mar 13 2025, 7:04 PM
Unknown Object (File)
Mar 13 2025, 8:57 AM
Unknown Object (File)
Mar 13 2025, 7:46 AM
Subscribers

Details

Summary

On amd64, boot blocks and the non-EFI loader are 32-bit compiled as
clients of BTX, so cannot access addresses beyond 4GB. However, the
64-bit entry point may refer to a structure table below 4GB, which we
want to use if the BIOS does not provide a 32-bit entry point. The
situation is similar for powerpc64.

Consequently, always compile-in support for the 64-bit entry point, but
ensure that it is not selected on 32-bit-compiled boot loaders if the
structure table it points to grows beyond 4GB (as it is then not
accessible).

PR: 284460

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

I have an answer to the question I left on the last revie.

stand/libsa/smbios.c
190

I'd use the pointer size here. The cheri folks have asked that we prefer that everywhere.

196

Don't you just need to test the end address?

stand/libsa/smbios.c
190

Is replacing this #ifdef by #if __SIZEOF_POINTER__ < 8 OK?

I suspect that __SIZEOF_POINTER__ includes capabilities on Cheri platforms. This makes the test above semantically imperfect (what is really wanted is the width of the address space), but it will work in practice as all Cheri platforms have 64-bit addressing. I also suspect the boot loaders are not compiled/executed in capability mode (I think this would require specific code in other existing places in the loader).

196

Mmm... yes. I just wanted to be extra paranoid in case the table is botched, but doing it that way is not very elegant, and anyway if done should apply to all cases. I'll just leave out the first test.

stand/libsa/smbios.c
190

that's the change they'd recommend to me in the past when I've done these things. True, it's imperfect here, since we're not creating a cap pointer, but it doesn't trigger their scanning scripts that look for LP32 tests. you can ask brooks or jrtc23 to see if I'm understanding their ask correctly.

markj added inline comments.
stand/libsa/smbios.c
190

In general I think using _ILP32 to test for 32-bit platforms is ok, the problem is more with using _LP64 to test for "not 32-bit", since that excludes CHERI, which is also not a 32-bit platform.

This revision is now accepted and ready to land.Mar 9 2025, 9:22 AM
jhb added inline comments.
stand/libsa/smbios.c
190

The other thing you can use is __SIZEOF_SIZE_T__ as an effective test for address size. That is the cleanest of the available options I think. I do think in terms of the diff, I would lean towards keeping the same test you had before (SIZEOF_POINTER) so that the diff is about the functional change you care about and not changing two things at once. Maybe switch to __SIZEOF_SIZE_T__ as a separate commit?

olce marked 5 inline comments as done.Mar 11 2025, 1:28 PM
olce added inline comments.
stand/libsa/smbios.c
190

As the code is currently written (we test if we have overflowed a 32-bit number), and although the #if is in practice equivalent, I feel that discriminating with __ILP32__ is more logical. I'll modify this code along with the guard (to start using __SIZEOF_SIZE_T__) in a separate commit.