Page MenuHomeFreeBSD

Dual-purpose loader.efi

Authored by on Dec 15 2017, 3:36 AM.



This patch augments the logic for detecting the default filesystem, allowing loader.efi to function both as a replacement for boot1.efi as well as in its legacy capacity. This is intended to be the final fallback mechanism for loader.efi in the future, and the legacy compatibility is designed to facilitate a smooth transition.

This patch also allows GELI integration to proceed.

Test Plan

Testing has been done with both legacy and new operation on UFS and ZFS.

Diff Detail

rS FreeBSD src repository
Lint Skipped
Unit Tests Skipped

Event Timeline

imp accepted this revision.Dec 15 2017, 5:24 AM

As you may have guessed, I have issues with the extensive searching contemplated by this patch. I don't think it's a good idea. I've never had a system that was "saved" by it and had several where the wrong partition was "guessed" in an over zealous attempt to be bootable at all cost, but it wound up choosing something bad. I really do not wish to preserve it and wish to actively and specifically break anybody that relies on it.
There's parts I like that are nicely separate that I'll commit. Thanks for that cleanup.

97 ↗(On Diff #36621)

This can fail, and will cause a fault in the next line.


space star, not star space, is the general convention.


I don't like searching like this. Load the file or don't, but don't guess at the system might be OK before trying.


Again, I hate these checks for an installed system. They aren't needed.


{ goes on the next line.


And this is a third copy of the same thing.


Can't we just load the kernel as the check? I know that will require that we rearrange things, but I do not think we should probe like this. We should try to load the kernel or have it fail.


I do not like this searching of all the devices. It actively gets in the way of properly implementing EFIBOOTMGR stuff, which is next on my hit parade.


you can't assume GPT partitioning.


and this is an even worse version of the above since it's simply a copy that doesn't have the calls to check_preferred.


I like this cleanup.


Again, I don't like this structure of searching. At most we should search for other partitions on the disk we booted from, and then stop.


We can likely lose this.


here is where we should look for BootCurrent and then BootXXXX. If we find BootXXXX, we should see if it contains additional paths. If so, we should jump to the additional paths straight away.


At the end of this loop, we should see if we have an arg that specifies a kernel to boot directly.


I think expanding the searching is not a good idea. It will get in the way of properly implementing the efibootmgr protocol where the searching is done in the BIOS not in the loader and if we can't boot this device, we should go to the next one. I know that's not what FreeBSD has done traditionally, but the traditional behavior is (a) not useful and (b) violates the standard.

This revision is now accepted and ready to land.Dec 15 2017, 5:24 AM
imp requested changes to this revision.Dec 15 2017, 5:25 AM

Sorry, that last one is 'request changes'

This revision now requires changes to proceed.Dec 15 2017, 5:25 AM
imp added a comment.Dec 15 2017, 5:37 AM

I just read my comments, and I think they are overly critical. I think this is a good next step, but with less searching...

The idea here is to implement what will eventually be a last-resort fallback mechanism (in the case of a blank install, or someone's EFI vars getting wiped, or something) as a means of starting the transition away from boot1. The legacy search behavior *should* be subsumed into the find_currdev_all path, but I'm unwilling to remove the legacy path completely at this point.

I'm personally most interested in getting GELI integrated; thus I didn't work on the EFI vars here, and I don't plan to. My aim here is to get something in place which can start the transition away from boot1 without breaking current functionality, which can be useful in some form in the long run, and which will allow GELI integration to proceed. marked 6 inline comments as done.Dec 15 2017, 1:29 PM added inline comments.

This is the existing behavior, which is here to avoid breaking anyone's system. I think this needs to stay in place until the next major version.


I'm not going to do EFI vars handling in this changeset, but I agree, that ought to be the way it works eventually.

imp added inline comments.Dec 15 2017, 3:16 PM

Normally I'd be right there with you on compatibility.

However, compatibility breaks things, including any possibility of efibootmgr failing over to the next thing. I'm not at all comfortable retaining a feature that's little used, causes actual problems today, is of at best dubious value and demonstrably gets in the way of bringing in standards compliant behavior.


Yea, I'm actually working on making that work now that I have efibootmgr in the tree and generating proper paths. I'll see if I can fast track it, as well as a trimmed down searching that goes with it. That will get you onto the GELI stuff I broke :) marked 2 inline comments as done.Dec 16 2017, 12:27 AM

I'm taking the broader architecture discussion here to -arch marked an inline comment as done.Dec 16 2017, 12:38 AM added inline comments.

That's not really possible given the architecture of loader. It expects you to set the variables, then run a load procedure. The purpose here is to get a good guess of the boot partition.

imp added inline comments.Dec 16 2017, 12:47 AM

That's possible through UEFI environment variables. Failure to do so and looking too hard violates the UEFI spec. marked 8 inline comments as done.Dec 16 2017, 12:52 AM added inline comments.

Anyone with a legacy setup isn't going to have EFI vars set. If this is done as a fallback, it shouldn't cause problems.


This search will end up being a fallback mechanism. EFI bootmgr should take precedence, if the variables are actually defined.


tsoome will almost certainly complain about that, on behalf of everyone who boots from different disks from their ESP


The plan is this will only run if no previous mechanism (command-line args, efibootmgr) has succeeded.

cy added a subscriber: cy.Dec 16 2017, 1:47 AM marked 4 inline comments as done. edited the test plan for this revision. (Show Details)

Simplified search procedure. marked 5 inline comments as done.Dec 19 2017, 2:49 AM

Fixed efi_zfs_is_preferred so ZFS preferred volumes are correctly detected again.

Accidental early rebase