Split boot1 in half and use the useful half for gptboot.efi
ClosedPublic
Actions

Authored by imp on Jun 7 2019, 2:24 AM.

Details

Reviewers

bcran
tsoome
kevans

Commits

rS348812: Create gptboot.efi
rS348811: Break out the disk selection protocol from the rest of boot1.

Summary

Break out the disk selection protocol from the rest of boot1.

Segregate the disk probing and selection protocol from the rest of the
boot loader.

Create gptboot.efi

This is a primary boot loader that is intended to implement the
gptboot partition selection algorithm just like we did for BIOS
booting. While the preferred method for UEFI is to use the UEFI Boot
Manager protocol, there are situations where that can't be done: some
BIOS makers interfere with the protocol in unhelpful ways, there's a
new standard for a zero variable write from the client OS, and finally
for USB drives that might be mobile between systems with multiple
partitions there needs to be a media stable way to select.

Test Plan

Integrate it into Netflix's OCA firmware, do some upgrades, watch it boot the right partition :)

Yes, I know that some people may not like this.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 24741
Build 23502: arc lint + arc unit

Event Timeline

imp created this revision.Jun 7 2019, 2:24 AM

Herald added a subscriber: bcran. · View Herald TranscriptJun 7 2019, 2:24 AM

Harbormaster completed remote builds in B24741: Diff 58344.Jun 7 2019, 2:24 AM

imp edited the test plan for this revision. (Show Details)Jun 7 2019, 2:27 AM

imp added reviewers: bcran, tsoome, kevans.

bcran added inline comments.Jun 8 2019, 2:35 AM

stand/efi/boot1/boot1.c
232	It's possible for platforms to not produce the `EFI_BLOCK_IO_PROTOCOL` protocol, so we should check for errors here.
stand/efi/boot1/proto.c
43	Can we depend on `OpenProtocol` being available? The UEFI Specification says that should be used in preference to `HandleProtocol`: EFI 1.10 Extension The HandleProtocol() function is still available for use by old EFI applications and drivers. However, all new applications and drivers should use EFI_BOOT_SERVICES.OpenProtocol() in place of HandleProtocol(). The following code fragment shows a possible implementation of HandleProtocol() using OpenProtocol(). The variable EfiCoreImageHandle is the image handle of the EFI core.
stand/efi/gptboot/Makefile
1	Are `MK_LOADER_ZFS=no` and `NOFAT=yes` supposed to be left in?
stand/efi/gptboot/proto.c
114	Typo ("parittion").
150	Not sure the final "block" is needed?
171	Is "WTF?" suitable for end-users to see?
194	I do wish style(9) required brackets even when they're not needed - I don't like not having them here and in other places. Especially after the "goto fail" saga.

imp added inline comments.Jun 8 2019, 5:28 AM

stand/efi/boot1/boot1.c
232	This loader is useless on such platforms, but having an error message is better.
stand/efi/boot1/proto.c
43	Beyond the scope of this change. This code is copied from boot1. We use OpenProtocol in one place and HandleProtocol in 22.
stand/efi/gptboot/Makefile
1	Yup. We absolutely do not support ZFS with this, and never will. Also, we don't need that hacky boot1.efifat thing for this. That stuff should die there and I added NOFAT so we wouldn't build it here.
stand/efi/gptboot/proto.c
150	No, it's not.
171	Fair point. Will revise.
194	Yea, I guess we'll disagree here. I find the extra {} confusing for simple things like this.

imp added inline comments.Jun 8 2019, 5:34 AM

stand/efi/boot1/boot1.c
232	also, this is just code motion, I'd like to do that as a followup.

tsoome accepted this revision.Jun 8 2019, 5:34 AM

tsoome added inline comments.

stand/efi/boot1/proto.c
43	I agree that we should move towards OpenProtocol but not necessarily here. The obvious question is about obsoleting the support for pre EFI 1.1, but as we already have place with OpenProtocol.... :)