nixos/iso-image: add support for boot.loader.efi.installDeviceTree

[jfly]

There are 2 commits here: I first rename rename boot.loader.systemd-boot.installDeviceTree to boot.loader.efi.installDeviceTree, and then I add support for boot.loader.efi.installDeviceTree to iso-image.nix.

Many thanks to @jmbaur for introducing me to the embedded world!

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

---

Add a 👍 reaction to pull requests you find important.

[jmbaur]

@ofborg test systemd-boot.basic

[jmbaur]

@ofborg test systemd-boot.specialisation

[jmbaur]

@jfly and I have tested that this works on a few different aarch64 devices booting via the ESP in the ISO. We considered moving the option either at boot.loader.installDeviceTree or at boot.loader.efi.installDeviceTree, and were convinced that the latter was the best option for now since it is currently only being used in a UEFI context.

[qbisi]

Can we first let grub module support specifying deviceTree and then make use of that option when making iso-image.
See last two commits in #403746

[qbisi]

This pull request implements limited DeviceTree loader support in GRUB when building ISO images.
boot.loader.efi.installDeviceTree does not work for a common nixos configuration using grub bootloader.

NixOS also has the EFI bootloader Limine module which does not have deviceTree specification yet, renaming boot.loader.systemd-boot.installDeviceTree to a more generalized boot.loader.efi.installDeviceTree seems premature.

Also, there's case for grub/limine not installing kernel/dtb to EFI partition.

[jmbaur]

@qbisi thanks for the review!

I don't think the grub PR necessarily needs to land prior to this PR. The two implementations of using grub are and have always been entirely separate. I believe @ElvishJerricco might be doing some work to move ISOs for the ESP eltorito stuff to using systemd-boot, while reusing the code in the systemd-boot module (which already supports devicetree), but that probably requires some restructuring of how the boot loader install scripts work as a whole.

As for the common nixos option, there are other examples of generic nixos options that aren't supported by all boot loader install programs. The prime example I can think of is the extlinux boot loader install scripts not supporting specialisations. While I think it is nice if every bootloader install program had the same feature set for shared options, we already haven't been following that. I think what would make the most sense here is to change the grub option to assert that this new option presented in this PR is set to false, since it is not yet capable of using it. Interesting to see what your thoughts are.

[qbisi]

My concern is whether we should rename boot.loader.systemd-boot.installDeviceTree to boot.loader.efi.installDeviceTree. In my opinion, for the grub/limine implementation, it’s not essential for bootloader to install dtb into the EFI partition.
We can have a minimal 4 MB EFI partition containing only grub.efi, while keeping /boot and /nix/store on a single Btrfs partition. In that case, we don’t install the device tree into EFI but instead load the DTB directly from the Nix store.
Having option boot.loader.efi.installDeviceTree really confuse me for grub deviceTree loader implemention in #403746

[jmbaur]

It sounds like the concern here is with putting the option in the efi option attr path. Would boot.loader.installDeviceTree work out better in your opinion? Since it is not strictly tied to efi. I haven't read over the entire grub dtb PR yet, but it sounds like if you install the dtb to /boot instead of depending on /nix/store being available at this point in time (same as what's done for kernel and initrd, IIRC), then the option being at that generic attr path fits better.

[qbisi]

systemd-boot always need to install kernel/initrd/dtb to EFI partion, so installDeviceTree essentially means loadDeviceTree for systemd-boot. For grub/limine implemention, i think the word loadDeviceTree would be more appropriate.
Now boot.loader.systemd-boot.installDeviceTree default to with config.hardware.deviceTree; enable && name != null,
it's a intermediate variable for explicitly enable/disable deviceTree override support for efi bootloader.
I would suggest using the name boot.loader.efi.loadDeviceTree.

[jmbaur]

systemd-boot always need to install kernel/initrd/dtb to EFI partion, so installDeviceTree essentially means loadDeviceTree for systemd-boot. For grub/limine implemention, i think the word loadDeviceTree would be more appropriate. Now boot.loader.systemd-boot.installDeviceTree default to with config.hardware.deviceTree; enable && name != null, it's a intermediate variable for explicitly enable/disable deviceTree override support for efi bootloader. I would suggest using the name boot.loader.efi.loadDeviceTree.

It is not always the case that systemd-boot should install the devicetree blob. For platforms that have the DTB (that linux would use) built into the firmware, and exposed via the UEFI configuration table, it is not necessary that we install a devicetree to the ESP at all. The name installDeviceTree is not meant to describe what happens when the bootloader is ran, but rather what happens when the bootloader install programs (that we write) run, i.e. whether or not we install the devicetree blob to the ESP. The description says this as well (https://github.com/nixos/nixpkgs/blob/231bf43f72ff6cbc22bfb4f3ba4249cba5ef8375/nixos/modules/system/boot/loader/systemd-boot/systemd-boot.nix#L251). I think this name applies to both systemd-boot and grub alike.

[qbisi]

By saying "always" i mean if systemd-boot need to use the keyword devicetree in boot entries, it always has to install dtb to EFI partition as systemd-boot can not read ext4/btrfs/xfs in rootfs partition. So the description

        Install the devicetree blob specified by `config.hardware.deviceTree.name`
        to the ESP and instruct systemd-boot to pass this DTB to linux.

make sense for systemd-boot. However, this is not the case for grub/limine which can load dtb directly from rootfs.

Both efi bootloadr (sd-boot,grub,limine) support specifying devicetree when executing the kernel. A more general option name to describle the common behavior would be boot.loader.efi.loadDeviceTree: Instruct efi bootloader to load the devicetree blob specified by config.hardware.deviceTree.name, for later use by a Linux kernel.

ref

• sd-boot: https://uapi-group.org/specifications/specs/boot_loader_specification/#type-1-boot-loader-specification-entries
• grub: https://www.gnu.org/software/grub/manual/grub/html_node/devicetree.html
• limine: https://codeberg.org/Limine/Limine/src/commit/e01e8309910dd4c2514ff3e3971a7ee15e0a61bc/CONFIG.md?display=source#L214

[jmbaur]

this is not the case for grub/limine which can load dtb directly from rootfs

In theory, systemd-boot can do this as well with XBOOTLDR, where the DTB could live on something like ext4. For the argument you're making about being able to pull the DTB directly out of the nix store, while I agree that some boot loaders have the capability of doing this, I am against this for a few reasons:

• it is inflexible for doing things like LUKS full-disk encryption, which not every boot loader supports (or wants to support, since the initrd is really the place where LUKS things should happen)
• the same argument could be made for the kernel and initrd, which are both also in the nix store and are much larger files compared to the DTB
• the DTB is a property of how the kernel is invoked, just like the kernel and initrd, and as such should live alongside the kernel and initrd

As for the naming of the option, while I would like to stay detached from the bootloader implementation of what it does with the DTB, loadDeviceTree sounds reasonable enough, so I'd like to move forward with this. I'd argue that this should go into boot.loader.loadDeviceTree, since this isn't strictly tied to UEFI, does that sound good to you?

[qbisi]

I'd argue that this should go into boot.loader.loadDeviceTree, since this isn't strictly tied to UEFI, does that sound good to you?

We have generic-extlinux-compatible module in nixos, using boot.loader.loadDeviceTree replace boot.loader.generic-extlinux-compatible.useGenerationDeviceTree. This will involve more tree-wide change. But it sounds good to me.

while I agree that some boot loaders have the capability of doing this, I am against this for a few reasons:

There is an option boot.loader.grub.copyKernels meets the need. Implemention in #403746 will also respect copyKernels when setting deviceTree blob.

    my $json_text = read_file("$path/boot.json");
    my $data = decode_json($json_text);
    my $devicetree;
    if (exists $data->{"org.nixos.grub"} && exists $data->{"org.nixos.grub"}->{devicetree}) {
        $devicetree = $data->{"org.nixos.grub"}->{devicetree};
    }
    my $fdtfile = defined $devicetree ? copyToKernelsDir(Cwd::abs_path("$devicetree")) : undef;

[jmbaur]

using boot.loader.loadDeviceTree replace boot.loader.generic-extlinux-compatible.useGenerationDeviceTree

Good find! Ok, so I think it would be good to move forward with boot.loader.loadDeviceTree. Based on what I can see, it should be a drop-in replacement for boot.loader.generic-extlinux-compatible.useGenerationDeviceTree, and we can add one more commit to this PR that does that work. With that change in place, the grub-related changes in #403746 can point to the new option. Does that sound good?

[jmbaur]

I just found #396334, which is very much related. The changes look to be very similar, though this PR does try and generalize the bootloader "use devicetree from NixOS config".

[qbisi]

the grub-related changes in #403746 can point to the new option. Does that sound good?

LGTM, should i split the grub part from #403746 to a seperate pr which does not involve changes in linux kernel. This seperate pr will not include nixosTest.device-tree.{systemd-boot,grub} as it require specifying deviceTree {blob, source} directly in deviceTree module.

[jmbaur]

the grub-related changes in #403746 can point to the new option. Does that sound good?

LGTM, should i split the grub part from #403746 to a seperate pr which does not involve changes in linux kernel. This seperate pr will not include nixosTest.device-tree.{systemd-boot,grub} as it require specifying deviceTree {blob, source} directly in deviceTree module.

I think that would be best.

@ElvishJerricco any thoughts on this PR versus #396334? It seems like we have a good way forward with this one.

[jmbaur]

@jfly this PR likely will need to change the aarch64 ISO to set the new option loadDeviceTree to false, as that ISO is meant to be generic across devices and will only work if the firmware already has a devicetree to present to linux.

[qbisi]

The default value for boot.loader.loadDeviceTree is "with config.hardware.deviceTree; enable && name != null;" , which will evaluate to false when making a generic ISO image. do we need to manualy set it to false in aarch64 ISO settings.

[jmbaur]

The default value for boot.loader.loadDeviceTree is "with config.hardware.deviceTree; enable && name != null;" , which will evaluate to false when making a generic ISO image. do we need to manualy set it to false in aarch64 ISO settings.

Yes! That's what I mentioned in the previous comment

[ElvishJerricco]

@jmbaur So, to summarize, the suggestion is mainly to rename the option to boot.loader.loadDeviceTree and share it among boot loader implementations that support device trees?

One issue I see is the default value for the option. The default for systemd-boot is not the correct default for all boot loaders. In particular, I'm thinking of generic-extlinux-compatible. Unlike systemd-boot (and presumably grub), we set up generic-extlinux-compatible with a whole directory of DTBs that u-boot can choose from at runtime. The fact that systemd-boot needs a specific one specified is why it requires hardware.deviceTree.name to be set.

So I think really the default for the option should be false, and each boot loader module should include its own config.boot.loader.loadDeviceTree = lib.mkDefault ... setting according to what it actually supports. And finally, we probably want to avoid scenarios where loadDeviceTree is set to true but the boot loader doesn't support it. We can fix that by adding a hidden and internal option like boot.loader.supportsLoadingDeviceTree that the boot loader modules set to true, and an assertion on config.boot.loader.loadDeviceTree -> config.boot.loader.supportsLoadingDeviceTree.

#403746 and #446563 will both need adjusting, of course.

[qbisi]

generic-extlinux-compatible can specify both fdtfile or fdtdir. loadDeviceTree=true is equivalent to use fdtfile in extlinux.conf.

We have mainly 5 bootloader in nixos:

1. extlinux compatible
2. grub/grub-efi
3. systemd-boot
4. limine
5. rEFInd

Only rEFInd does not support loadDeviceTree (limine and grub to be implemented), loadDevcieTree just take no effect on rEFInd. We can warn user if they set boot.loader.refind.enable = true and boot.loader.loadDeviceTree = true.

[ElvishJerricco]

generic-extlinux-compatible can specify both fdtfile or fdtdir. loadDeviceTree=true is equivalent to use fdtfile in extlinux.conf.

right but the default is to use fdtdir, so that should remain its default. Hence my whole point about letting each boot loader module set its own default for the option.

Only rEFInd does not support loadDeviceTree (limine and grub to be implemented), loadDevcieTree just take no effect on rEFInd. We can warn user if they set boot.loader.refind.enable = true and boot.loader.loadDeviceTree = true.

I don't see why this is better than what I suggested. We can just have a clear indicator of whether the option is supported by a boot loader module and then error if it isn't supported. I don't see why we need to spaghettify the logic like that.

[qbisi]

The purpose is to migrate boot.loader.systemd-boot.installDeviceTree to more generic option, the option boot.loader.loadDeviceTree instruct downstream bootloader to manually load the specified dtb file. Then downstream bootloader module implement it rather than set they prefered default. In fact, i would prefer to make boot.loader.loadDeviceTree an internal option that equals to with config.hardware.deviceTree; enable && name != null.

To be specific:

1. If hardware.deviceTree.name !=null and hardware.deviceTree.enabled = true. The user prefer to use fdtfile ${config.hardware.deviceTree.name} in their extlinux.conf file. generic-extlinux-compatible module should not modify the value boot.loader.loadDeviceTree.
2. If hardware.deviceTree.name=null and hardware.deviceTree.enabled = true. Then use fdtdir in extlinux.conf
3. If hardware.deviceTree.enabled = true but bootloader does not support loading deviceTree , then just warn user rather than introduce extra options to module system.

[ElvishJerricco]

In fact, i would prefer to make boot.loader.loadDeviceTree an internal option that equals to with config.hardware.deviceTree; enable && name != null.

No, because that's not correct for other boot loaders. Plus, at that point it's just an unnecessary alias.

Maybe it should be like this: Instead of having boot.loader.loadDeviceTree, we only have hardware.deviceTree.enable. Then we have an internal option boot.loader.supportsDeviceTree, which the boot loader module sets to true if it can support it. The hardware.deviceTree module then has assertions = [ { assertion = config.hardware.deviceTree.enable -> config.boot.loader.supportsDeviceTree; ... } ];. Now the boot loader module can interpret hardware.deviceTree.* however it wants. systemd-boot can assert assertion = config.hardware.deviceTree.name != null;, and generic-extlinux-compatible can accept more broad circumstances like it currently does.

[jfly]

Closing this to not give the illusion that I'm working on this. (Others are free to use code/ideas from this PR.)