This is better expressed as follows

get_uefi_bootname() {
case ${TARGET:-$(uname -m)}
amd64) echo bootx64.efi ;;
arm64) echo bootaa64.efi ;;
i386) echo bootia32.efi ;;
arm) echo bootarm.efi ;;
*) panic bogus ;;
esac
}

generally shell scripts echo things rather than set global variables. Also, using the alternative value variable substitution lets you omit the repetition.

I'd add efibootfile here too, since it's just used in this function.

Are these two locals? I'd add them to the local list if so. I don't see where else they are used.

I'd also match ? here. getopts returns ? when there's no args in the getopt list that match.

Picking one: it may make sense to define a global variable for this value, and use it throughout.

I'd be tempted to commit all the changes to this file separately.

You don't need this space. The shell will just eat it when you use this later.

Such an approach looks beautiful but may have performance and correctness implications since it runs the function in a subshell environment. I think setting variables (either named via a parameter or with a hard-coded name) is a perfectly acceptable approach.

It might make sense to use mktemp to create a better name. That would allow multiple copies of this script to be run at the same time by different users (or the same one).
Both here and below (not specifically called out).

where is VMBASE added? This just makes an esp, but doesn't seem to create the resulting image in VMIMAGE.

This function is called once, and in the context my suggestions are fine. They won't cause any sort of regression in performance, and it's more likely to be correct (though I didn't change the { to ( to enforce the new sub-shell out of a desire to be efficient).

don't know if we have a 'panic' routine :)

I'd be tempted to create constants for these magic numbers. Then the user of this file can also use them to prevent their proliferation.

here, and elsewhere, should be lower-case k.

how can this ever be true? We only ever set X86_BOOTMETHOD foor amd64 and i386.

Good point - not sure how I removed the mkimg command.

I'm checking if the machine is arm64 _or_ the x86 bootmethod is UEFI.
So it's going to be true if running under arm64 or a amd64 or i386 system is booted using UEFI.

No, but we do have die

I'd think should do the ${espfilename} dance too, with the rm after... I'm not thrilled about lua here, but I'm not sure what would be better given the context this script runs in.

December :)

Add a comment that says what units these are in?

Yeah, me neither.

I would be tempted to go a slightly more conservative route of renaming it instead. That would allow someone to recover back to this if they have a bad new loader.

This looks weird... What are you trying to accomplish?

This doesn't look right at all. srcroot seems to be /usr/src, while DESTDIR is where we install into... Is that intentional? If so, that seems like a poorly named variable.

Does this need protection against multiple invocations?

The problem is that many systems will o my have 800KB available on the ESP. So in those situations it’s not possible to have more than one copy.

Oh that needs a comment.
efibootmgr doesn’t mark new entries active, so this code figures out what number it is and runs the command to activate it.

Here, srcroot is where we’re copying files from, to install onto the ESP.

I'm trying to see if there's an existing boot entry - filtering out other OSes that use different paths to their loader. If there is, we don't need to create a new one.

I don't think so: from what I've seen bsdinstall isn't designed to have multiple copies running at the same time.