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

use make-disk-image instead of sd-image #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rcambrj
Copy link

@rcambrj rcambrj commented Nov 21, 2024

Following up on #88, this change converts the core of this repository's sd-image.nix copied variant to instead use make-disk-image.nix along with generic-extlinux-compatible.

As part of this change, it was necessary to include a copied variant of generic-extlinux-compatible/*, which permits additional scripts to be added to system.build.installBootloader. I think it makes sense to upstream this change eventually because I suspect that a fair few machines running extlinux require additional files, just like the Raspberry Pis.

This is a breaking change ❗

  • The make-disk-image.nix invocation should now be added to the nixos configuration (see README changes)
  • The examples have been renamed to include two variants: a direct-to-kernel and a u-boot variant. If your CI referred to rpi-example, this will now be broken.
  • You may find that nixos-rebuild switch breaks your machine's ability to boot. Back ups are encouraged.

Can I upgrade with nixos-rebuild switch?

No. Regardless of whether you use direct-to-kernel or u-boot.

$ nixos-rebuild switch --flake github:rcambrj/raspberry-pi-nix/make-disk-image#rpi-example-uboot
building the system configuration...
stopping the following units: boot-firmware.mount
activating the configuration...
setting up /etc...
reloading user units for root...
restarting sysinit-reactivation.target
reloading the following units: -.mount
[  361.674119] EXT4-fs (mmcblk0p2): re-mounted 44444444-4444-4444-8888-888888888888 r/w. Quota mode: none.
A dependency job for local-fs.target failed. See 'journalctl -xe' for details.
Job for sysinit.target canceled.
You are in emergency mode. After logging in, type "journalctl -xb" to view
system logs, "systemcGive root password for maintenance
(or press Control-D to continue):

I'm not entirely sure what causes this failure, but at the moment, the only route forward is to make a backup, re-image your SD card, then restore the backup.

I think this upgrade dead-end makes this change quite a good candidate for a long-lived branch or a separate repository, but I'm also aware that this repo's version hasn't reached 1.0 yet. Keen to hear thoughts! At the very least I thought it best to make this clear.

Some notes

  • example/default.nix has been removed in favour of example/direct.nix and example/uboot.nix. I'd like to expand this later to include various boards, alongside an effort to bring about automated testing.
  • atomic-copy-safe and atomic-copy-clobber should be DRY'd (happy to work on this inside this PR if considered a big deal or separately later).
  • There's a note about the boot partition name (ESP) in the README

Some blockers which need fixing

  • UART on pins 8+10
    • with direct-to-kernel, this tty doesn't show anything until the login prompt
    • with u-boot, this tty doesn't show anything after u-boot hands over to the Linux kernel (no login prompt)
    • tty1 (HDMI) works fine

Copy link
Member

@tstat tstat left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this PR!

I haven't built an image from this branch yet, but intend to soon.

One concern I have is that, on master, everything that makes assumptions about the partition scheme is broken off into a separate module (the sd-image one), while everything else is in the rpi module. Then, by not importing the sd image module, you can use this repo with any file system or partition scheme.

I do want to maintain that separation, as the rpi module is useful on its own.

As part of this change, it was necessary to include a copied variant of generic-extlinux-compatible/*, which permits additional scripts to be added to system.build.installBootloader.

Instead of copying and adding an option, I wonder if we can supply our own system.build.installBootloader that installs firmware files and calls generic-extlinux-compatible's installBootloader if uboot is enabled.

Additionally, the systemd script that installs the kernel, initrd, and firmware today has some care taken to not modify any files that haven't changed, and to track if the installation process was halted at a bad time (like in the middle of firmware installation). It would be good to retain that.

I'm happy to work on these changes when I find time and motivation.

I'm also happy to merge this to a branch as is if you want to retarget it.

@@ -318,13 +184,15 @@ in
};
boot = {
kernelParams =
if cfg.uboot.enable then [ ]
[ "console=serial0,115200n8" "console=tty1" ] ++
Copy link
Member

Choose a reason for hiding this comment

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

I think the "no kernel init script uart" issue is due to the order of the console params here. I experienced this issue before and had to swap the order, although the docs don't seem to mention anything about this order.

Copy link
Author

Choose a reason for hiding this comment

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

You might be right. Definitely worth experimenting with the order. Thanks for the tip.

Comment on lines +190 to +193
"root=PARTUUID=${cfg.rootGPUID}"
"rootfstype=ext4"
"fsck.repair=yes"
"rootwait"
Copy link
Member

Choose a reason for hiding this comment

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

These aren't necessary when using initrd, and we don't want to hardcode this sort of thing in the rpi module as that would prevent booting to alternative file systems, like btrfs.

Copy link
Author

Choose a reason for hiding this comment

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

Fair point! Just to be clear though, with these are you referring to:

          "rootfstype=ext4"
          "fsck.repair=yes"
          "rootwait"

?

"rootfstype=ext4"
"fsck.repair=yes"
"rootwait"
"init=/nix/var/nix/profiles/system/init"
Copy link
Member

Choose a reason for hiding this comment

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

/sbin/init is still best here I think. It's installed by the initscript bootloader and will exec the appropriate init script in the nix store.

Copy link
Author

Choose a reason for hiding this comment

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

I thought I had trouble with /sbin/init but can't remember what that was. Needs more investigation.

Comment on lines +212 to +213
# when uboot is disabled, use this module to put files into
# the boot partition as part of installBootloader
Copy link
Member

Choose a reason for hiding this comment

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

how does this work with direct-to-kernel? Where do we install and update the kernel, initrd, params, etc in that case?

Copy link
Author

Choose a reason for hiding this comment

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

generic-extlinux-compatible-pi-loader still installs the kernel and initrd in /boot/nixos - the same files as used for uboot, only the boot strategy is changed.

cmdline.txt is written to /boot in config.generic-extlinux-compatible-pi-loader.extraCommandsAfter

alsa.support32Bit = true;
pulse.enable = true;
};
fileSystems = {
Copy link
Author

@rcambrj rcambrj Dec 12, 2024

Choose a reason for hiding this comment

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

@tstat you said:

One concern I have is that, on master, everything that makes assumptions about the partition scheme is broken off into a separate module (the sd-image one), while everything else is in the rpi module. Then, by not importing the sd image module, you can use this repo with any file system or partition scheme.

I do want to maintain that separation, as the rpi module is useful on its own.

You'll notice that config.fileSystems is declared in the machine's configuration, so the file systems can be mounted as needed. I actually use this approach on my machines with custom mounts to achieve impermanence.

There's a parallel discussion in nix-community/disko#550 about using disko to customise the partition layout using make-disk-image, which faces some challenges. TLDR is: try using systemd-repart.

This doesn't mean that there aren't assumptions made - the /boot directory is critical to putting the files in the right spot, but at least the mount can be customised as needed and other filesystems/mounts added as needed too (in theory - haven't tried).

To talk a little more about this /boot assumption though... config.boot.loader.generic-extlinux-compatible-pi-loader.mirroredBoots and its original config.boot.loader.generic-extlinux-compatible.mirroredBoots allow the selection of the location for the boot files (and even multiple locations), but I've noticed that extlinux-conf-builder.sh makes an assumption about this location :( this bug would need fixing before changing the /boot path.

if ! test -e $dst; then
dstTmp=$dst.tmp.$$
cp -r $src $dstTmp
mv $dstTmp $dst
Copy link
Author

Choose a reason for hiding this comment

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

@tstat you said:

Additionally, the systemd script that installs the kernel, initrd, and firmware today has some care taken to not modify any files that haven't changed, and to track if the installation process was halted at a bad time (like in the middle of firmware installation). It would be good to retain that.

This line moves the files into the right place after they've been copied to the boot filesystem. Notice line 16 though - the file won't be copied if it already exists at the destination. This is copied from the generic-extlinux-compatible shell script. A modified version atomic-copy-clobber.sh does the same but overwrites the file, which is used for config.txt and cmdline.txt. Is this in line with what you mean?

'');
in
mkIf cfg.enable {
system.build.installBootLoader = installBootLoader;
Copy link
Author

Choose a reason for hiding this comment

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

@tstat you said:

As part of this change, it was necessary to include a copied variant of generic-extlinux-compatible/*, which permits additional scripts to be added to system.build.installBootloader.

Instead of copying and adding an option, I wonder if we can supply our own system.build.installBootloader that installs firmware files and calls generic-extlinux-compatible's installBootloader if uboot is enabled.

I'm not entirely sure I understand what you mean. I think this is what's happening on this line. The value of config.system.build.installBootLoader is different here than it would be in generic-extlinux-compatible as it now contains cfg.extraCommandsAfter. Can you please expand?

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

Successfully merging this pull request may close these issues.

2 participants