Page MenuHomeFreeBSD

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

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

Details

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 24793
Build 23545: arc lint + arc unit

Event Timeline

bcran created this revision.Jun 8 2019, 9:19 PM
imp accepted this revision.Jun 8 2019, 10:45 PM
This revision is now accepted and ready to land.Jun 8 2019, 10:45 PM
imp added a reviewer: emaste.Jun 8 2019, 10:46 PM

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

tsoome accepted this revision.Jun 9 2019, 3:34 AM
emaste accepted this revision.Jun 9 2019, 3:39 PM

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).

emaste added a comment.Jun 9 2019, 3:43 PM

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.

imp added inline comments.Jun 10 2019, 12:31 AM
stand/efi/boot1/Makefile
64 ↗(On Diff #58417)

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

bcran added inline comments.Jun 10 2019, 12:36 AM
stand/efi/boot1/Makefile
64 ↗(On Diff #58417)

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

bcran updated this revision to Diff 58465.Jun 10 2019, 2:18 AM

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.

bcran added a comment.Jun 11 2019, 2:57 PM

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?

bcran added a comment.Wed, Jun 19, 4:48 PM

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)

imp added a comment.Wed, Jun 19, 5:34 PM

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.