Add a new efi-update-loader script and associated man page

[bcran]

Summary

The efi-update-loader script looks for efi type partitions on each disk associated with the root filesystem, looking at UFS and ZFS mirror, stripe, raid etc. configurations.
Its primary use is expected to be during 'installworld' and 'freebsd-update' to automatically update the loader that the UEFI system firmware uses to boot FreeBSD.

Unless the user specifies a device to use, it will use all such ESPs, and if there's an existing /EFI/FreeBSD/loader.efi it will copy /boot/loader.efi (or the otherwise
specified loader file) to /EFI/FreeBSD/loader.efi and backup any existing file as loader-old.efi if the new loader.efi isn't the same as the old one and there's available space to do so.
If there's a FreeBSD loader/boot1 as /EFI/BOOT/BOOT{arch}.efi it will back it up to /EFI/FreeBSD/BOOT{arch}-old.efi (if there's space) and copy the new loader to /EFI/BOOT/BOOT${arch}.efi.

Test Plan

Tested on a couple of amd64 systems, both against the real root filesystem (ZFS) and against a md configuration of zfs mirror, raidz, ufs mirror and non-redundant configurations.
Tested on an armv6 board (BeagleBone Black) and an aarch64 system (Overdrive 1000).

Note that on armv6 and other installations with MBR disk layouts, the autodetection will fail to find any devices because the partition type isn't 'efi'.

[bcran]

I'm thinking that instead of looking at all disks in the system it should perhaps only check the disk that contains the root filesystem.
That would avoid the case where for example the script would try and update the install media plugged into a USB port.

[tsoome]

In D19588#419418, @bcran wrote:

I'm thinking that instead of looking at all disks in the system it should perhaps only check the disk that contains the root filesystem.
That would avoid the case where for example the script would try and update the install media plugged into a USB port.

I think, what you need to establish first is what this script is for and what are the use cases. If we are running it from installer, it must only install boot programs on target disk(s). If it is meant to be used as generic update current boot disks, then it has to discover disks related to current root file system. The least we want to happen is that some random disk will get updated.

[bcran]

I think, what you need to establish first is what this script is for and what are the use cases. If we are running it from installer, it must only install boot programs on target disk(s). If it is meant to be used as generic update current boot disks, then it has to discover disks related to current root file system. The least we want to happen is that some random disk will get updated.

I forgot to explain that. This script would be run from make installworld or freebsd-update to update an ESP that's expected to already have a FreeBSD boot1.efi or loader.efi on it.
I need to update the diff to look at the output of gmirror status, zpool status etc. to work out which ESP(s) to update. In a mirror situation there may be multiple ESPs which should be updated.

[tsoome]

In D19588#419505, @bcran wrote:

I think, what you need to establish first is what this script is for and what are the use cases. If we are running it from installer, it must only install boot programs on target disk(s). If it is meant to be used as generic update current boot disks, then it has to discover disks related to current root file system. The least we want to happen is that some random disk will get updated.

I forgot to explain that. This script would be run from make installworld or freebsd-update to update an ESP that's expected to already have a FreeBSD boot1.efi or loader.efi on it.
I need to update the diff to look at the output of gmirror status, zpool status etc. to work out which ESP(s) to update. In a mirror situation there may be multiple ESPs which should be updated.

Okay, thats cool - yes, in those cases we need to determine the targets. Also there are not only mirror, but raidz too (I'm using raidz on my boot pool;).

[bcran]

Changed the autodetect code to look for all ESPs associated with the root filesystem.

It now understands zfs (stripe, raidz etc.), and ufs including mirror, raid, stripe etc.

[bcran]

• Fix usage output, try and improve some variable names

[manu]

usr.sbin/efi-update-loader/efi-update-loader.sh

174

You could test directly EFI/FreeBSD and mkdir -p

181

This will break system that don't have persistant variable, for those we need to use EFI/BOOT/boot${arch}.efi

221

On some system you will not have efirt for variable, can efibootmgr test this ?

[imp]

partial review.... more to come

usr.sbin/efi-update-loader/efi-update-loader.8

36

It might be nice to also accept an ESP directly to install it into, as well as auto detect.
As well as having the full unix path for ESPs that are already mounted.

45

You need markup on loader.efi

62

You need markup on these file names.

usr.sbin/efi-update-loader/efi-update-loader.sh

45

I'd be tempted to line the echos up with the echo below.

78

Does this cope with boot1.ef?

79

You can return 0 here.

82

and return 1 here.

178

If you make the changes I suggested, this can turn into a simple

if bootefi_is_freebsd; then

which seems less convoluted to me.

[imp]

usr.sbin/efi-update-loader/efi-update-loader.sh

221

It doesn't today, but we can do the right thing with efivar if uboot's EFI rejects permanent changes to variables.

[kevans]

usr.sbin/efi-update-loader/efi-update-loader.sh

221

I think we also need to test this with bhyve+UEFI, but I suspect (IIRC) both implementations accept the UEFI vars and they just don't persist.

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.sh

221

So in that case we definitely do _not_ want to remove the EFI/BOOT/BOOT{$arch}.efi file.
And for new installs, I also need to check the changes I committed a few weeks ago to install /EFI/FreeBSD/loader.efi only. I seem to recall @imp was against having the BOOT${arch}.efi file since it's supposedly only for removable media, but there are obviously several situations where it's needed for booting from fixed media too.

[imp]

usr.sbin/efi-update-loader/efi-update-loader.sh

34

[ -n ${verbose} ] && echo $*

should suffice. You don't need the extra stuff $@ does, nor the extra quotes, nor even the if.

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.sh

78

Was "boot1.ef" a typo? It copes with boot1.efi, but not currently loader.efi. Since we'll want to keep this file, I'll update it to check for strings that are in loader.efi too.

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.sh

34

Except since the script is set -e, that then needs a || true to avoid the statement causing an error.

[bcran]

Address review comments.
Only ever update /EFI/BOOT/BOOT${arch}.efi and /EFI/FreeBSD/loader.efi if they already exist and are
the BOOT${arch}.efi file is a FreeBSD boot1.efi or loader.efi.

Remove the lines about running efibootmgr.
Rework the copyloader function to better work with situations where the ESP is low on free space - for example
this will be the case with most existing installations which only have an 800KB filesystem. Since the loader.efi
is over 400KB, it won't be possible to do backups. The script now makes backups by copying the existing files to
/tmp/espback.XXXXXX and then, if there's space left, copying them back onto the ESP.

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.sh

181

In the latest revision of the script I've changed it so that if /EFI/BOOT/BOOT${arch}.efi exists, it will copy loader.efi to that file.
In addition, if EFI/FreeBSD/loader.efi exists, it will also copy loader.efi to that file. I think that should work to retain any boot functionality.

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.8

36

The script already does accept an ESP directly to install into - via the -d option.
I agree having an option to specify a unix path for an already mounted ESP would be nice to have too.

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.sh

45

In the latest revision they look lined up in QtCreator, but for some reason they're wrong here - I'm using tabs, not spaces.

[kevans]

usr.sbin/efi-update-loader/efi-update-loader.sh

163

This should take into an account an ESP we're copying to already having been mounted. For some images, it'll be premounted at /boot/msdos (or something relevant, like /boot/efi or /efi) and we probably should just use that mountpoint rather than mounting/unmounting it. We'll need to be careful about not unmounting it in cleanup, too, if that's the case.

291

This block should be redundant now; we've guaranteed just above that we either have non-zero user supplied ${esps} or we failed to autodetect ESP and bailed out with the "Error: could not detect ESP containing FreeBSD loader to update"

[bcran]

• Allow an ESP that's already mounted to be updated.

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.sh

163

Good point. I've fixed this in the latest revision and tested on a Beaglebone Black where the ESP is mounted on /boot/msdos.

291

Thanks, I've fixed it in the latest diff.

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.8

36

I've added a -p path option to the latest revision of the script and man page.

[bcran]

• Bug fixes

[kevans]

usr.sbin/efi-update-loader/efi-update-loader.sh

183

I think with this approach, you'll need to also set some local indicator that esppath should be cleared out just after you don't unmount/rmdir the mntpt. Otherwise, subsequent copyloader will suddenly fail as we can't clobber the already-mounted ESP that we don't want to touch.

This is likely really really edge-casey, though...

[tsoome]

usr.sbin/efi-update-loader/efi-update-loader.sh

184

what if it was mounted RO?

[kevans]

usr.sbin/efi-update-loader/efi-update-loader.sh

184

I guess we should keep track of auto-detected ESP vs. user-specified ESP. If user-specified it, we should probably re-mount it R/W and then return it to R/O when we're done, no?

[bcran]

usr.sbin/efi-update-loader/efi-update-loader.sh

183

Good point.

184

Yeah, we probably should do that, instead of erroring out if it's mounted RO.

[bcran]

• Allow use of an ESP that's mounted read-only.

[bcran]

Fix bugs caused by setting ${esppath} and not unsetting ${espdev}.

[markmi_dsl-only.net]

I frequently cross-build amd64->aarch64 and amd64->armv7 and also do
installworld's of the aarch64 and armv7 worlds into directories trees on the
amd64 machine ( same file system as the amd64 world is on, but in something
like, for example, /usr/obj/DESTDIRs/clang-cortexA53-installworld-poud ).
(Later such are used via qemu-user-static and poudriere-devel , for example.)

I may have missed it, but will the script or its use during installworld avoid
updating the amd64 EFI area when cross-installing a world that is not
intended to be bootable? If yes, how?

[bcran]

In D19588#421756, @markmi_dsl-only.net wrote:

I may have missed it, but will the script or its use during installworld avoid
updating the amd64 EFI area when cross-installing a world that is not
intended to be bootable? If yes, how?

I was planning to avoid it by checking ${DESTDIR}, but others have suggested not adding it to installworld at all, but having users run it as an additional manual step instead.