Page MenuHomeFreeBSD

The efifat files are no longer used: remove the code to build them.
ClosedPublic

Authored by bcran on Jun 8 2019, 9:19 PM.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26150
Build 24667: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Jun 8 2019, 10:45 PM

Added emaste as a sanity check for the 'not used' bit. I'd like to see this die in fire.

I approve, this should go.

I'm aware of some out-of-tree build scripts that do something like:

mkimg -s gpt -b $ROOTDIR/boot/pmbr \
  -p efi:=$ROOTDIR/boot/boot1.efifat \
  -p freebsd-boot:=$ROOTDIR/boot/gptboot \
  -p freebsd-ufs:=ufs -p freebsd-swap::1G \
  -o $image

but really they need to just generate the EFI filesystem at the same time they generate the main (UFS in this case) filesystem.

https://wiki.freebsd.org/UEFI needs an update (should probably just reference uefi(8) instead).

There's random bits of boot1.efifat lore scattered around, e.g. Google turned up https://ashish.blog/2018/06/freebsd-uefi-boot/ and https://gist.github.com/tehpeh/a5676b711db94f6c1b9af1ec3356a13c. Hopefully a relnotes entry and a mailing-list heads-up is sufficiently discoverable by authors of those sorts of docs.

stand/efi/boot1/Makefile
66

This needs an entry in OBSOLETE_FILES.
Also don't we need to actually delete them from the tree too?

stand/efi/boot1/Makefile
66

Oh, yeah it needs an entry in OBSOLETE_FILES and removed from OPTIONAL_OBSOLETE_FILES.
And yes, the .tmpl.xz files need deleted.

Removed tmpl.xz files and updated ObsoleteFiles.inc and OptionalObsoleteFiles.inc

This revision now requires review to proceed.Jun 10 2019, 2:18 AM

I approve, this should go.

I'm aware of some out-of-tree build scripts that do something like:

mkimg -s gpt -b $ROOTDIR/boot/pmbr \
  -p efi:=$ROOTDIR/boot/boot1.efifat \
  -p freebsd-boot:=$ROOTDIR/boot/gptboot \
  -p freebsd-ufs:=ufs -p freebsd-swap::1G \
  -o $image

but really they need to just generate the EFI filesystem at the same time they generate the main (UFS in this case) filesystem.

Is there a way to build the efi fs this without root? I build images in a Jenkins instance to test FreeBSD/arm64 on various simulators and would prefer to not need to give the Jenkins user sudo access.

Is there a way to build the efi fs this without root? I build images in a Jenkins instance to test FreeBSD/arm64 on various simulators and would prefer to not need to give the Jenkins user sudo access.

There's a review from July 2018 at D16438 to add msdos support to makefs, but it seems to have stalled. But otherwise, root access will be needed.

Is there a way to build the efi fs this without root? I build images in a Jenkins instance to test FreeBSD/arm64 on various simulators and would prefer to not need to give the Jenkins user sudo access.

There's a review from July 2018 at D16438 to add msdos support to makefs, but it seems to have stalled. But otherwise, root access will be needed.

I will take a renewed effort to get the makefs msdos support into the tree, but perhaps in the immediate term we can move the efifat goop into tools/? Keep the ObsoleteFiles.inc change and never install these for regular builds, but leave them around for people with special-purpose scripts?

I will take a renewed effort to get the makefs msdos support into the tree, but perhaps in the immediate term we can move the efifat goop into tools/? Keep the ObsoleteFiles.inc change and never install these for regular builds, but leave them around for people with special-purpose scripts?

I'm not sure it's worth moving them, but I can commit the OptionalObsoleteFiles.inc and ObsoleteFiles.inc changes if you think that makes sense.

I'm not sure it's worth moving them, but I can commit the OptionalObsoleteFiles.inc and ObsoleteFiles.inc changes if you think that makes sense.

Are they already unhooked from install?

Are they already unhooked from install?

It seems not - they're optionally not installed if NOFAT is defined (rS348722/rS348763). There are several ways it could be accomplished (e.g. change to #ifdef EFIFAT) but my suggestion is that by default we do not install them and move the entries to ObsoleteFiles.inc unconditionally. Folks with out-of-tree uses can just do something like (cd stand/efi/boot1 && make -DEFIFAT)

Are they already unhooked from install?

It seems not - they're optionally not installed if NOFAT is defined (rS348722/rS348763). There are several ways it could be accomplished (e.g. change to #ifdef EFIFAT) but my suggestion is that by default we do not install them and move the entries to ObsoleteFiles.inc unconditionally. Folks with out-of-tree uses can just do something like (cd stand/efi/boot1 && make -DEFIFAT)

Yea, that's recent to reuse boot1's makefile for gptboot, but have the FAT stuff not done. It's not supposed to be a user controlled knob. If you want *THAT* then you could add a MK_LOADER_EFIFAT_BLOBS and have it default to NO.

Yea, that's recent to reuse boot1's makefile for gptboot, but have the FAT stuff not done. It's not supposed to be a user controlled knob. If you want *THAT* then you could add a MK_LOADER_EFIFAT_BLOBS and have it default to NO.

Well, we can't still install these files and also move them unconditionally to ObsoleteFiles.inc (as in this review) - my suggestion is that we don't even need to go as far as the machinery for WITH_LOADER_EFIFAT_BLOBS, just make it not installed by default anywhere (and add to ObsoloeteFiles.inc), and have a temporary backdoor for folks doing odd things.

kill them with fire :)

This revision is now accepted and ready to land.Aug 16 2019, 7:31 PM
emaste requested changes to this revision.Aug 16 2019, 8:54 PM

This change is not valid, it removes the templates but leaves the Makefile rules in place to install them.

Again, what I am suggesting is that we instead:

  • stop installing the EFI FAT fs images
  • add to obsoletefiles etc.
  • leave the templates and Makefile bits in place here or moved to tools/ for those who have out-of-tree needs, until msdos makefs support is in
This revision now requires changes to proceed.Aug 16 2019, 8:54 PM

Since makefs fat support has been committed, proceed with removing all the EFI FAT code.

  • leave the templates and Makefile bits in place here or moved to tools/ for those who have out-of-tree needs, until msdos makefs support is in

Sorry for the long delay in updating this review. Could you take a look at the latest changes please? I'm not sure if other work needs committed before we can remove the boot1/efifat stuff altogether? If so, I'll keep this on hold and proceed with that.

ObsoleteFiles.inc
41

This file is not obsolete.

stand/efi/Makefile
12 ↗(On Diff #61267)

This is not right. It boot1.efi needs to still be built.

bcran added inline comments.
stand/efi/Makefile
12 ↗(On Diff #61267)

I thought it wasn't needed since we use loader.efi now. I've updated the patch to continue building it.

Is stand/efi/boot1/Makefile.depend supposed to be deleted?

Is stand/efi/boot1/Makefile.depend supposed to be deleted?

I deliberately deleted it since it claims to be auto-generated, and appeared to be unused. But now I see many other directories have Makefile.depend files too. So to be safe, I'll restore it.

This revision is now accepted and ready to land.Aug 30 2019, 1:33 AM

I've stumbled upon this just few days ago when make delete-old suggested to remove the old trusty /boot/boot1.efifat (which I had to refuse). I know this differential is closed, but I have some questions and don't want to spam the mailing lists.

@imp wrote:

I'd like to see this die in fire.

May I ask what's wrong with it? I've been dding this image to my p1 ever since I had switched to GPT/UEFI on my drives, and it always worked just fine.

@emaste wrote:

https://wiki.freebsd.org/UEFI needs an update (should probably just reference uefi(8) instead).

I've checked with the wiki, looks like I now have to create partition manually with newfs_msdos -F 32 -c 1 and then copy loader.efi as /mnt/EFI/BOOT/BOOTX64.efi. This is not the same as my current layout created via boot1.efifat, which lists two files:

% ls -l /mnt/tmp/efi/boot/
total 385
-rwxr-xr-x  1 root  wheel  393216 Apr 16  2018 BOOTx64.efi
-rwxr-xr-x  1 root  wheel      12 Apr 16  2018 startup.nsh

Also, example in the wiki creates a whopping 40M partition to hold 411K loader file, why is that? /boot/boot1.efifat was of much more reasonable 800K.

I've stumbled upon this just few days ago when make delete-old suggested to remove the old trusty /boot/boot1.efifat (which I had to refuse). I know this differential is closed, but I have some questions and don't want to spam the mailing lists.

@imp wrote:

I'd like to see this die in fire.

My I ask what's wrong with it? I've been dding this image to my p1 ever since I had switched to GPT/UEFI on my drives, and it always worked just fine.

It is bad for multiboot setups. With mount ESP and copy files we can handle both single OS and multiple OS cases. But the pain point is, FreeBSD does not have decent way to keep boot programs up to date automatically and yes, mount + copy is more pain than single dd command.

@emaste wrote:

https://wiki.freebsd.org/UEFI needs an update (should probably just reference uefi(8) instead).

I've checked with the wiki, looks like I now have to create partition manually with newfs_msdos -F 32 -c 1 and then copy loader.efi as /mnt/EFI/BOOT/BOOTX64.efi. This is not the same as my current layout created via boot1.efifat, which lists two files:

% ls -l /mnt/tmp/efi/boot/
total 385
-rwxr-xr-x  1 root  wheel  393216 Apr 16  2018 BOOTx64.efi
-rwxr-xr-x  1 root  wheel      12 Apr 16  2018 startup.nsh

Also, example in the wiki creates a whopping 40M partition to hold 411K loader file, why is that? /boot/boot1.efifat was of much more reasonable 800K.

This is because there are systems which will not read ESP with FAT12/FAT16. Welcome to hell of x86. FAT32 needs minimum ~ 32MB. Note that vendors recommend 128-256MB (ish), and that is partly because enough space will make it possible to implement fall back situations and some hw vendors do provide firmware updates (and tools) as EFI applications, to be installed to ESP.

With "manual" creation of ESP you can still have the 800KB, but you can also have larger one… Note:

-r-xr-xr-x 1 root sys 518144 okt 11 21:41 loader32.efi
-r-xr-xr-x 1 root sys 577536 okt 11 21:41 loader64.efi

The 32 and 64 bit variants sum above 800K and if I want to keep also the "known good" backup copy...

Hope this helps.

@tsoome wrote:

Hope this helps.

It does, thanks! However, I'm still not sure if I need to create startup.nsh or not. Quick googling suggests that one might get boot problems in some environments if this file is missing. Wiki page does not mention it.

@tsoome wrote:

Hope this helps.

It does, thanks! However, I'm still not sure if I need to create startup.nsh or not. Quick googling suggests that one might get boot problems in some environments if this file is missing. Wiki page does not mention it.

Yes it is situational, I have never needed it myself.

@tsoome wrote:

Hope this helps.

It does, thanks! However, I'm still not sure if I need to create startup.nsh or not. Quick googling suggests that one might get boot problems in some environments if this file is missing. Wiki page does not mention it.

Yes it is situational, I have never needed it myself.

It used to help in some weird situations exacerbated by bugs in the bios and in boot1.efi. things have changed enough that it's likely never needed.

@imp wrote:

It used to help in some weird situations exacerbated by bugs in the bios and in boot1.efi. things have changed enough that it's likely never needed.

I see; I've already found the commit when it was added, rS297871.