Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sd-image: remove populateFirmwareCommands in favor of firmwarePartitionContents #261857

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sorki
Copy link
Member

@sorki sorki commented Oct 18, 2023

This converts sdImage.populateFirmwareCommands into a
derivation that can be used to obtain firmware partition contents.

For example for sd-image-armv7l-multiplatform,
given a test_firmware.nix file containing

{
  imports = [
    <nixpkgs/nixos/modules/installer/sd-card/sd-image-armv7l-multiplatform.nix>
  ];
  nixpkgs = {
    crossSystem.system = "armv7l-linux";
    overlays = [
    ];
  };
}

we can now do

nix-build '<nixpkgs/nixos>' \
  -A config.sdImage.firmwarePartitionContents \
  -I nixos-config=./test_firmware.nix

Description of changes

See above. Prior discussion at #241534 (comment)

Resolves #108048

CC @samueldr @SFrijters

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@SFrijters
Copy link
Member

Cool, looks promising. I also tried it with aarch64-linux and it generates some familiar stuff!
But I'm not good enough with this to put down a firm approval.

If I understand it correctly, the 'manual' update process would now be to build this derivation and copy everything to the boot/firmware partition?

@SFrijters
Copy link
Member

Somewhat of a side note: if you are going to move towards a proper update mechanism, would it make sense to add an equivalent to boot.loader.raspberryPi.firmwareConfig again - i.e. some parameter that can append to config.txt?
Although I (hopefully) have my RPi LED customization sorted with device tree overlays, the dtparams approach was a lot more user friendly (and more widely documented if you search for a way to do it).

@sorki
Copy link
Member Author

sorki commented Oct 18, 2023

If I understand it correctly, the 'manual' update process would now be to build this derivation and copy everything to the boot/firmware partition?

Exactly. We can try adding a (semi-)automated update mechanism that would do that for us - i.e. something like

mkdir -p /boot/firmware
mount /boot/firmware
... copy (if newer / diffferent hashes or so) ...
umount /boot/firmware

@sorki
Copy link
Member Author

sorki commented Oct 18, 2023

Somewhat of a side note: if you are going to move towards a proper update mechanism, would it make sense to add an equivalent to boot.loader.raspberryPi.firmwareConfig again - i.e. some parameter that can append to config.txt? Although I (hopefully) have my RPi LED customization sorted with device tree overlays, the dtparams approach was a lot more user friendly (and more widely documented if you search for a way to do it).

Possibly, I'm not a fan of these RPi specific mechanisms but sometimes it seems necessary to adjust config.txt by hand and this would overwrite the changes. You can already provide a custom sdImage.populateFirmwareCommands (or sdImage.populateFirmware derivation with this patch) if you only import sd-image.nix module.

This looks a bit weird tho - why would you have sd-image.nix imported and configure sdImage options on installation where there's no SD at all - so this is a good point to consider. Seems like a more generic module / namespace is needed (and sd-image-* could default to that).

We sure can get there but ideally in backwards compatible manner.

@SFrijters
Copy link
Member

Yeah, these things would make more sense to me if they're not referencing anything SD card related at all.
Then something like foo.firmware.populateFirmwareCommands could use the option foo.firmware.extraFirmwareConfig by default to append something (nothing by default), so if you use extraFirmwareConfig you still get the basic stuff for free, but if you know what you're doing you can override populateFirmwareCommands in its entirety.

I don't know too much about (the implementation of) modules - would it otherwise be possible to use an overlay (nix overlay, not device tree overlay) to use prev.populateFirmwareCommands and then append to that? It would be less user friendly than having the extra config option, but if we're trying to discourage it anyway...

Alternatively, just updating the documentation/NixOS on RPi wiki would help (I can probably do something with that). But it would be good to settle on the desired way forward first.

@samueldr
Copy link
Member

Long infodump, sorry...

Don't hesitate to ask for more precisions.

About the SD card build API

The SD card building API should be considered semi-private and fine enough to break (with proper mkRemovedOption use).

So this change here is generally acceptable, as long as it's not plumbed for any automatic updating, nor recommended to automatically plumb by end-users.

Having better semantics for the different bits involved here is a general win, I guess.

About automatically updating firmware

I've had the discussion a couple of times about automatically updating the firmware, and the situation is kinda hard.

This is something that lives outside of the lifecycle of generations; in other words, you cannot use the generations menu to go back to a previously working generation, when things break.

“But the bootloader has that problem too!” might be your first instinct. It's true. But it's also the boundary at which NixOS operates. Anything that runs prior to or the bootloader depends on outside of the lifecycle of generations is a no-go zone. And the bootloader is managed only because of the implicit requirements of it existing for the lifecycle of generations to even be a thing.

Now, all of that is thinking generally, this includes your boring standard Laptop UEFI firmware, and other spicier options.

The overall way we should support such things is by supporting standard (enough) methods to update firmware. The example here is fwupd/LVFS.

About board-specific hacks and workarounds

It would be undesirable to add again specific patterns for a given board. Here the Raspberry Pi.

Like alluded to previously, we should aim to support "standard" methods to manage those bits. So no, we shouldn't add any at this point in time. The aim should be to, at some point, drop this, but it's not planned for the near future.

As hinted previously (if you read between the lines) we should aim to support composing the "base" NixOS system on top of external projects (any one of them!) rather than pre-bake device-specific images.

This holds for all ecosystems, not specific to Raspberry Pi, nor for ARM.

About this particular example and the future

Things are in motion with Tow-Boot to make it well-supported generally with fwupd/LVFS.

This would include the Raspberry Pi family of devices, in the future.

So, one way to have this all work will be a future where NixOS has no knowledge of the Raspberry Pi firmware or that it needs to be updated, but instead relies on an external Platform Firmware provider, and then fwupd/LVFS to update the platform firmware, irrespective of the board. (But not limited to a specific one!)

Copy link
Member

@samueldr samueldr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some notes

nixos/modules/installer/sd-card/sd-image.nix Show resolved Hide resolved
nixos/modules/installer/sd-card/sd-image.nix Outdated Show resolved Hide resolved
nixos/modules/installer/sd-card/sd-image.nix Outdated Show resolved Hide resolved
@SFrijters
Copy link
Member

This is something that lives outside of the lifecycle of generations; in other words, you cannot use the generations menu to go back to a previously working generation, when things break.

“But the bootloader has that problem too!” might be your first instinct. It's true. But it's also the boundary at which NixOS operates. Anything that runs prior to or the bootloader depends on outside of the lifecycle of generations is a no-go zone. And the bootloader is managed only because of the implicit requirements of it existing for the lifecycle of generations to even be a thing.

I would like to see this communicated more clearly to the user at the very least. I assume this information is buried in the documentation somewhere, but I generally expected that if something is an option in a NixOS module, that a change will actually get applied.

As it is, I get a warning that boot.loader.raspberryPi is going to be removed, but I think it is equally important to warn that changing the relevant options to conform to the modern way of doing things will not actually get you to the desired state (unlike in virtually every other case in NixOS?).

@sorki sorki changed the title sd-image: introduce populateFirmware sd-image: remove populateFirmwareCommands in favor of firmwarePartitionContents Oct 19, 2023
@sorki
Copy link
Member Author

sorki commented Oct 19, 2023

The SD card building API should be considered semi-private and fine enough to break (with proper mkRemovedOption use).

So this change here is generally acceptable, as long as it's not plumbed for any automatic updating, nor recommended to automatically plumb by end-users.

Having better semantics for the different bits involved here is a general win, I guess.

Thanks for the pointers, updated to deprecate the old option. I'm not sure if it warrants a changelog entry since the deprecation message you get is pretty good I think.

About automatically updating firmware

I've had the discussion a couple of times about automatically updating the firmware, and the situation is kinda hard.

This is something that lives outside of the lifecycle of generations; in other words, you cannot use the generations menu to go back to a previously working generation, when things break.

“But the bootloader has that problem too!” might be your first instinct. It's true. But it's also the boundary at which NixOS operates. Anything that runs prior to or the bootloader depends on outside of the lifecycle of generations is a no-go zone. And the bootloader is managed only because of the implicit requirements of it existing for the lifecycle of generations to even be a thing.

Good point, I've stripped the commit message of hints about automated firmware updates as we can figure that out later. It is probably better to not try to have a fully automated solution and instead opt for semi-automated one - i.e. having a script in path that can be called manually to perform the update. Not quite sure where should this live for now (getting back to this it surely is nixos-hardware in case of RPi or specific board support projects living outside of nixpkgs).

Now, all of that is thinking generally, this includes your boring standard Laptop UEFI firmware, and other spicier options.

I'm currently trying to do something similar for old Lenovo X200 running coreboot where the update mechanism is using flashrom 😃 so I'll think about that some more for sure.

The overall way we should support such things is by supporting standard (enough) methods to update firmware. The example here is fwupd/LVFS.

About board-specific hacks and workarounds

It would be undesirable to add again specific patterns for a given board. Here the Raspberry Pi.

Like alluded to previously, we should aim to support "standard" methods to manage those bits. So no, we shouldn't add any at this point in time. The aim should be to, at some point, drop this, but it's not planned for the near future.

Completely agree although it seems hard to draw a line of what should be in nixpkgs and what should go to e.g. nixos-hardware. Like we can't drop raspberrypifw as sd-image builder depends on it. Reading the discourse post, I see where this goes and this also sounds to good to me (so not as hard after all!).

It is pretty weird that our multiplatform/generic images are at the same time RPi specific. I wouldn't go as far as removing SD card images and the tooling since it is very useful (and widely used I think) but removing RPi specifics from generic images is a good idea. We can do this in discrete steps

  • move the RPi specifics from generic images into RPi specific files
  • move these into nixos-hardware
  • drop (move into nixos-hardware) the rest of the RPi specific cruft like raspberrypifw, device-tree_rpi and RPi kernels
  • profit by not having to support (or explain to users why it is bad) all the weird mixtures of unsupported configurations like using mainline kernel with device-tree_rpi

As hinted previously (if you read between the lines) we should aim to support composing the "base" NixOS system on top of external projects (any one of them!) rather than pre-bake device-specific images.

This holds for all ecosystems, not specific to Raspberry Pi, nor for ARM.

👍

The situation with RPi is quite unfortunate and it shouldn't have a special treatment because of its popularity. The RPi foundation reinvented a lot stuff to make things seemingly more user friendly at the expanse of creating a massive confusion about how the things should be done (were done previously) in a standard manner (dtparams, GPIO, booting..).

About this particular example and the future

Things are in motion with Tow-Boot to make it well-supported generally with fwupd/LVFS.

This would include the Raspberry Pi family of devices, in the future.

So, one way to have this all work will be a future where NixOS has no knowledge of the Raspberry Pi firmware or that it needs to be updated, but instead relies on an external Platform Firmware provider, and then fwupd/LVFS to update the platform firmware, irrespective of the board. (But not limited to a specific one!)

Quite interesting and I certainly support your efforts and can possibly help with some stuff despite the fact I'm not really into UEFI - I don't have single computer running UEFI at the moment as I don't see any advantage in doing so (no need for secure boot, only reason I found it interesting was that it would allow me running systemd-boot which would allow for bootcounting and automated rollbacks but even that seems possible with just uboot nowadays).

I'll soon get my hands on Pi5 and more Pi4s so I can try UEFI/Tow-Boot!

@sorki
Copy link
Member Author

sorki commented Oct 19, 2023

Alternatively, just updating the documentation/NixOS on RPi wiki would help (I can probably do something with that). But it would be good to settle on the desired way forward first.

That is more than welcome @SFrijters and I want to do the same after we figure out the way forward.

@samueldr
Copy link
Member

It is pretty weird that our multiplatform/generic images are at the same time RPi specific. I wouldn't go as far as removing SD card images and the tooling since it is very useful (and widely used I think) but removing RPi specifics from generic images is a good idea.

There is one main reason for this: it's because it is otherwise really inconvenient to add the Platform Firmware for the Raspberry Pi after the fact. Most other platforms, at worse, "just" look at specific offsets on the same storage device, so you can "just" splat the platform firmware there if you don't have better options... But the Raspberry Pi requires a "real" partition. So adding it after the fact requires steps that are way harder to get right.

But you're right, and this is where I want to go: Have it all be agnostic.

Though you might be thinking about it thinking that "we want a generic SD card image"... We might not (but still provide useful tooling for end-users).

The ideal solution here is to always rely on a "manual" install method to the target storage, rather than pre-baked filesystems, as those have various issues down the line. So the actual goal is to "stop providing generic SD images" altogether, and instead rely solely on generic standards-based boot methods, e.g. UEFI.

So the "ideal" (realistic) future for Raspberry Pi (and other platforms without dedicated storage for the platform firmware) would be:

  • Acquire a platform firmware [EDKII, Tow-Boot, U-Boot, unimportant]
  • Write SD card with that platform firmware
  • Write USB drive with UEFI iso
  • Boot with both inserted
  • Install to SD card taking care of leaving the platform firmware stuff intact

Now, that doesn't help for "large scale deployment of images", but this is the kind of problem that is not ARM-specific nor Raspberry Pi specific, and require better semantics around producing disk images within NixOS :).

@samueldr samueldr dismissed their stale review October 19, 2023 21:44

[looks fine, but I haven't ran or tested personally]

@sorki
Copy link
Member Author

sorki commented Oct 20, 2023

There is one main reason for this: it's because it is otherwise really inconvenient to add the Platform Firmware for the Raspberry Pi after the fact. Most other platforms, at worse, "just" look at specific offsets on the same storage device, so you can "just" splat the platform firmware there if you don't have better options... But the Raspberry Pi requires a "real" partition. So adding it after the fact requires steps that are way harder to get right.

If the partition stays there (even if it's empty, which currently there's no way to disable it) than it is pretty easy to grab firmware (the mechanism provided in this PR makes it less troublesome) and add it to the image manually (mounting it after dding, losetup, guestfish..).

This only affects aarch64 image which is the only one currently built by Hydra I think and even that one is kind-of semi-official since it is only linked from nixos.wiki. Removal of the RPi specific firmware would make it a bit more inconvenient as you can't just grab an image built by Hydra and dd it to sdcard but you would need to perform additional step if you want to use it on RPi, which would cause some confusion for users used to the current method.

Shouldn't be a blocker but the failure mode is not ideal a la it was working before why it suddenly doesn't. So yes, I see why you would want to get rid of the image(s) entirely.

Now, that doesn't help for "large scale deployment of images", but this is the kind of problem that is not ARM-specific nor Raspberry Pi specific, and require better semantics around producing disk images within NixOS :).

There's another reason which is more important for me (I always build my own images anyway as I have many armv7l devices) and that is the ability to fully describe the working image using Nix. I just build the image, dd, put it in the device and call it a day. The specification is fully contained in Nix and doesn't require additional steps nor documentation.

I agree about better semantics and was wondering about that as well - the sdImage module is a bit weird since it is not included in NixOS modules by default (I've did that manually yesterday to test the generated documentation and deprecation message, but I've had to comment out fileSystems and boot.postBootCommands to not break the host).

I think we can use NixOS containers as an inspiration and have a similar mechanism - sdImage*s* that would allow configuring bunch of them and have them built on nixos-rebuild. Even if they are just a script that would spit out a path to the image (but it also opens up a possibility to have a flasher script and probably much more).

Now the interesting thing is - what about having all the things - sdImage, PXE, virtualization, deployment tools (nixops, Nixus, morph) integrated.. I've had this idea couple years ago - describe all your systems within one configuration and specify what backends you want for them. This would allow us to describe a system once and have all these goods available as simple knobs.

I wonder if there were prior efforts in this direction.


After further consideration I also don't think removing/moving any of the RPi packages from nixpkgs to nixos-hardware is a good idea

  • there are no guidelines for this
  • it would require consensus of all involved parties
  • nixos-hardware doesn't really have packages (although it has an overlay for device tree to use dtmerge which is perfectly fine and RPi specifics could be provided by that as well)

The device specific SD profiles seem easier to migrate.

I thought I should state this here so it doesn't look like some conspiracy discussed in unrelated PR.


Finally related to this PR

  • I've tested all the code paths by building x86 SD images (null is handled correctly, fails when the output is not a directory, copies the paths if it is).
  • Tested the deprecation message and generated documentation, it looks like this:
       - The option definition `sdImage.populateFirmwareCommands' in `/etc/nixos/machines/duck/configuration.nix' no longer has any effect; please remove it.
       Use sdImage.firmwarePartitionContents instead.

       For example, if your sdImage.populateFirmwareCommands were:

         ''
           cp ${pkgs.myBootLoader}/u-boot.bin firmware/
         ''

       You need to conver it to a derivation creating a directory:

         pkgs.runCommand "myFirmware" {} ''
           mkdir $out
           cp ${pkgs.myBootLoader}/u-boot.bin $out/
         ''

man:

       sdImage.firmwarePartitionContents
           Directory representing /boot/firmware partition on the SD image.

           Type: null or path

           Default: null

           Example:

               pkgs.runCommand "myFirmware" {} ''
                 mkdir $out
                 cp ${pkgs.myBootLoader}/u-boot.bin $out/
               ''

           Declared by:
               <nixpkgs/nixos/modules/sd-image-test.nix>

I have not tested on a real device (only with UbootQemuX86, since cross compilation is failing on systemd and I don't want to get into that right now.

Outstanding questions:

  • Do we want a changelog entry as well?
  • 33f9c8a changes semantics of couple of options which previously had undefined type into types.lines that allows merging these (prior to this they would fail if defined multiple times I think). It seems sensible to me to allow merging for them but PTAL

@sorki
Copy link
Member Author

sorki commented Oct 21, 2023

Now the interesting thing is - what about having all the things - sdImage, PXE, virtualization, deployment tools (nixops, Nixus, morph) integrated.. I've had this idea couple years ago - describe all your systems within one configuration and specify what backends you want for them. This would allow us to describe a system once and have all these goods available as simple knobs.

Realized that this can be probably already done by passing nixosConfigurations from flakes around.

EDIT: not quite!

@SFrijters
Copy link
Member

SFrijters commented Oct 22, 2023

Just FYI, I have now actually deployed the result of this derivation to my Pi and I'm now succesfully booting with U-Boot (and the device overlays that started the whole journey for me indeed work).

One more question though: nixos-init, kernel.img, initrd, cmdline.nix are files that are still sitting in my /boot and they are not included in this firmware, and they have not been updated by the system rebuild with boot.loader.generic-extlinux-compatible (also a folder /boot/old). Am I right in assuming these were only necessary for boot.loader.raspberryPi and they are now safe to remove? I see that different initrd and image files are referenced by /boot/extlinux/extlinux.conf but I'm not sure what the other two do exactly.

Sorry if this is a stupid question, but I'd rather not mess up my migration/cleanup at the last hurdle.

@sorki
Copy link
Member Author

sorki commented Oct 22, 2023

Just FYI, I have now actually deployed the result of this derivation to my Pi and I'm now succesfully booting with U-Boot (and the device overlays that started the whole journey for me indeed work).

Cool!

One more question though: nixos-init, kernel.img, initrd, cmdline.nix are files that are still sitting in my /boot and they are not included in this firmware, and they have not been updated by the system rebuild with boot.loader.generic-extlinux-compatible (also a folder /boot/old). Am I right in assuming these were only necessary for boot.loader.raspberryPi and they are now safe to remove? I see that different initrd and image files are referenced by /boot/extlinux/extlinux.conf but I'm not sure what the other two do exactly.

Sorry if this is a stupid question, but I'd rather not mess up my migration/cleanup at the last hurdle.

Yes those are exactly the artifacts of the previous builder

if test -n "@copyKernels@"; then
copyToKernelsDir $kernel; kernel=$result
copyToKernelsDir $initrd; initrd=$result
fi
echo $(readlink -f $path) > $outdir/$generation-system
echo $(readlink -f $path/init) > $outdir/$generation-init
cp $path/kernel-params $outdir/$generation-cmdline.txt
echo $initrd > $outdir/$generation-initrd
echo $kernel > $outdir/$generation-kernel
if test "$generation" = "default"; then
copyForced $kernel $target/kernel.img
copyForced $initrd $target/initrd
for dtb in $dtb_path/{broadcom,}/bcm*.dtb; do
dst="$target/$(basename $dtb)"
copyForced $dtb "$dst"
filesCopied[$dst]=1
done
cp "$(readlink -f "$path/init")" $target/nixos-init
echo "`cat $path/kernel-params` init=$path/init" >$target/cmdline.txt
so safe to delete. All that is now handled by uBoot which loads extlinux.conf

…onContents

This converts `sdImage.populateFirmwareCommands` into a
derivation that can be used to obtain firmware partition contents.

For example for `sd-image-armv7l-multiplatform`,
given a `test_firmware.nix` file containing

```
{
  imports = [
    <nixpkgs/nixos/modules/installer/sd-card/sd-image-armv7l-multiplatform.nix>
  ];
  nixpkgs = {
    crossSystem.system = "armv7l-linux";
    overlays = [
    ];
  };
}
```

we can now do

```
nix-build '<nixpkgs/nixos>' \
  -A config.sdImage.firmwarePartitionContents \
  -I nixos-config=./test_firmware.nix
```
@sorki
Copy link
Member Author

sorki commented Jun 10, 2024

Rebased due to mdDoc removals

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update firmware partition with generic-extlinux-compatible
4 participants