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

Jump to bottom

raspberrypi-bootloader: support new firmware partition concept #67902

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

raspberrypi-bootloader: support new firmware partition concept #67902

wants to merge 2 commits into from

Conversation

lopsided98
Copy link
Contributor

Motivation for this change

#62462 places the U-Boot configuration in the SD image's ext4 NIXOS_ROOT partition, and only leaves the Raspberry Pi firmware in the FAT FIRMWARE partition. This partition is mounted by default at /boot/firmware.

The boot.loader.raspberryPi was not updated to account for this change, and assumes that the firmware partition is mounted at /boot and that both the U-Boot/extlinux config and the firmware files should be placed there.

This PR adds a new option, boot.loader.raspberryPi.firmwareDir, which allows the user to specify where the firmware partition is mounted. For backwards compatibility with existing installations, the default value is /boot. If using a recent SD card image, it should be set to /boot/firmware. At some point, the default should probably be changed.

If boot.loader.raspberryPi.uboot.enable = false, kernels are also copied to the firmwareDir so they can be directly booted by the Raspberry Pi bootloader.

This PR also changes the definition of the boot.loader.raspberryPi.firmwareConfig, making it possible to override the entire contents of config.txt, as well as easily retrieve the entire contents of the file from the module system (I use that here).

I also removed enable_uart=1 from the default config.txt contents. The comment next to it claims that it used to be necessary for U-Boot to function, but this does not seem to be the case anymore. It seems like a good idea to remove this option, as it locks the VPU frequency to 250 MHz, which may not be desired in many cases. On the other hand, users may notice that the serial console no longer works by default.

I am using this on most of my Raspberry Pis, but they are all configured somewhat differently than an out of the box NixOS install, so I would like to test that an old SD image (from before #62462) can be upgraded after applying this PR and still boot correctly.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @samueldr

@lopsided98 lopsided98 force-pushed the raspberrypi-bootloader-firmware-partition branch from a744176 to 7b5d69d Compare September 28, 2019 21:05
@kwohlfahrt kwohlfahrt mentioned this pull request Feb 19, 2020
Open
@matthewbauer
Copy link
Member

@lopsided98 This looks like an improvement over the current situation. Anything else needed to test it?

@lopsided98
Copy link
Contributor Author

It works fine for me. I wanted to verify that old installations (with /boot on a separate partition) would not be broken, but have never gotten a chance to test this.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/raspberry-pi-3b-doesnt-boot-without-a-screen/6802/9

@lheckemann
Copy link
Member

This warrants a release notes entry, since it will start updating the firmware on devices where it was previously not updated.

@lopsided98 lopsided98 force-pushed the raspberrypi-bootloader-firmware-partition branch from 7b5d69d to 7d9fc1b Compare June 29, 2020 17:03
@lopsided98
Copy link
Contributor Author

No, it currently preserves the old behavior by default. You need to set boot.loader.raspberryPi.firmwareDir = "/boot/firmware" for this to work on newer images. I don't think the default should be changed so that devices that still use the old setup don't suddenly break.

I fixed the conflicts, but I haven't tested the new version at all, so this PR should not be merged yet.

@lopsided98 lopsided98 force-pushed the raspberrypi-bootloader-firmware-partition branch from 7d9fc1b to 2c78c41 Compare August 1, 2020 19:56
@lopsided98
Copy link
Contributor Author

I've tested this on a Raspberry Pi Zero W with boot.loader.raspberryPi.uboot.enable = true and it works.

I also added another commit that gets rid of the boot.loader.raspberryPi.uboot.configurationLimit option, and instead uses the config under boot.loader.generic-extlinux-compatible, which has that same option as well as others. This prevents unnecessary duplication of the options and allows more configuration.

@lopsided98 lopsided98 force-pushed the raspberrypi-bootloader-firmware-partition branch 4 times, most recently from d4d0175 to 76f4c09 Compare August 2, 2020 00:40
lheckemann
lheckemann approved these changes Aug 16, 2020
View reviewed changes
Copy link
Member

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

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

LGTM, I'm currently far away from my nearest rpi and unable to test it though.

@lopsided98 lopsided98 force-pushed the raspberrypi-bootloader-firmware-partition branch from 76f4c09 to 45c26e9 Compare October 4, 2020 05:29
@lopsided98 lopsided98 force-pushed the raspberrypi-bootloader-firmware-partition branch from 45c26e9 to 9471821 Compare July 7, 2021 01:38
@bendlas
Copy link
Contributor

bendlas commented Nov 6, 2021

I tried this on my rpi4 installation, hoping that it would resolve the issue. Unfortunately, time seems to have passed this by:

  • I had to set boot.loader.uboot.enable to false, in order to avoid an error message
  • It generated files into /boot, even though my raspi boots from a partition mounted into /boot/firmware

I could confirm that the generated config had my added lines though.

@bendlas bendlas self-requested a review November 6, 2021 22:41
bendlas
bendlas requested changes Nov 6, 2021
View reviewed changes
Copy link
Contributor

@bendlas bendlas left a comment

Choose a reason for hiding this comment

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

Overall the diff seems rather large could we maybe split this into the fix for config.txt and the refactoring?

nixos/modules/system/boot/loader/raspberrypi/firmware-builder.nix
else
pkgs.ubootRaspberryPi3_32bit
else
throw "U-Boot is not yet supported on the Raspberry Pi 4.";
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the error message I saw. So probably this part has gone out of sync.

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, but apparently this still shouldn't work. Now I'm really curious why this works on my current config (based on unstable)

Copy link
Member

Choose a reason for hiding this comment

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

Like a BIOS update would, the update needs to be applied mindfully by the owner.

@Gerschtli wrote good instructions for doing the update:

Hopefully we can figure out a good "user story" for updating the platform firmware, one that does not require making a plethora of platform-specific configurations in the options.

(Relatedly, I wrote about planning for a better NixOS on ARM, and one of the goals is to push managing the platform firmware outside of the direct OS update lifecycle, for all distros.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, and thanks for the pointers!

I was actually wondering if I could use nixos-install to refresh the firmware (and config.txt along with it) ...

Copy link
Member

Choose a reason for hiding this comment

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

Nope, nixos-install is only a fancy wrapper around nixos-rebuild. If it can't be done with nixos-rebuild it can't be done with nixos-install.

The reason the SD image has the Platform Firmware (U-Boot) available at the front of the disk is because it's being put there during the image build. The Platform Firmware partition for the Raspberry Pi is totally decoupled from the actual in-use system.

@samueldr samueldr mentioned this pull request Nov 24, 2021
Closed
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 19, 2022
@lopsided98 lopsided98 force-pushed the raspberrypi-bootloader-firmware-partition branch from 9471821 to a2575b9 Compare September 23, 2022 22:41
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 23, 2022
@lopsided98
raspberrypi-bootloader: use generic-extlinux-compatible config with U…
16896ef 
…-Boot

Allows the generic-extlinux-compatible bootloader options to control the
extlinux.conf generated by this module when U-Boot is enabled. Previously, this
module had a more limited, parallel set of options.
@lopsided98 lopsided98 force-pushed the raspberrypi-bootloader-firmware-partition branch from a2575b9 to 16896ef Compare September 23, 2022 22:51
@jaen
Copy link
Contributor

jaen commented Oct 9, 2022

Just checking, is this supposed to fix issues like this (#82455 (comment)) or will it still require more work on top of this PR? The comments on the throw above suggests it won't, but would welcome some clarification.

@lopsided98
Copy link
Contributor Author

lopsided98 commented Oct 14, 2022 •
edited

This PR would allow the existing Raspberry Pi firmware updating support present in nixpkgs to be used with the new (since mid-2019) SD image partition organization. Enabling boot.loader.raspberryPi will cause your firmware to be updated to latest version when the system is rebuilt.

The problem that @samueldr refers to above is that firmware updating shouldn't be tied to rebuilding the system. Rebuilding doesn't update the BIOS on x86, so it shouldn't update the firmware on the Raspberry Pi. The biggest practical disadvantage of doing so is that there is no easy way to rollback the firmware update if it breaks something.

I agree with this in principle, but in practice it is often useful to ensure the firmware is from the version of nixpkgs used to build the current system. This avoids differences between updating an existing system and flashing a fresh SD card image. It is also very useful to be able to specify the contents of config.txt as part of the NixOS config.

This module may better belong outside nixpkgs, perhaps in nixos-hardware.

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank marked this pull request as draft March 20, 2024 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bendlas bendlas bendlas requested changes

@lheckemann lheckemann lheckemann approved these changes

@colemickens colemickens Awaiting requested review from colemickens

@samueldr samueldr Awaiting requested review from samueldr

@bkchr bkchr Awaiting requested review from bkchr

@dasJ dasJ Awaiting requested review from dasJ dasJ is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
2.status: merge conflict 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@lopsided98 @matthewbauer @nixos-discourse @lheckemann @bendlas @jaen @samueldr @ryantm @SuperSandro2000 @wegank