Initrd improvements #97145

lheckemann · 2020-09-04T17:44:24Z

Motivation for this change

Fixing some false assumptions, such as that all u-boot images should be for ARM and compressed with gzip;
Providing a path with a filename extension corresponding to the compression, for more clarity on what the files are (just initrd is still provided for compatibility);
A number of the features which are useful for modifying from outside (especially for embedded systems) were fixed based on the platform; but using makeInitrd from one of the predefined platforms (like aarch64-multiplatform) can be significantly more convenient than defining a whole new platform — so this makes u-boot options available via the function call but optional, defaulting to the platform values;
Fixing terminology (partly) — we're generating an initramfs, not an initrd. This needs some more work, but we refer to it as initrd all over the place and this is not a battle I want to fight all the way in this context :)
Fixing use of initramfs secrets when cross-compiling
Adds tests for the initramfs secret functionality, testing all compressions

Closes #96005.

Things done

Built a number of initramfses for embedded systems
Built my NixOS system with the initramfs
Ran uefiUsb boot test
[?] Ensured that relevant documentation is up to date — I've updated (and added) documentation in make-initrd.nix itself, but not touched the relevant NixOS bits yet.
Fits CONTRIBUTING.md.

xaverdh · 2020-12-04T16:19:36Z

friendly ping

lheckemann · 2020-12-12T14:26:03Z

@xaverdh sorry for the delays and thanks for the ping! So, I've revamped it. It should now:

Support a wide range of predefined compressors
Allow specifying completely custom compressors with the function method
Support cross-compilation
Include tests for initrd secrets (which didn't exist beforehand, regardless of compression)

The missing pieces are:

getting the zstd and lz4 tests working (I've spent about 15min on this and it's not clear to me why the kernel isn't recognising the resulting initramfs but don't have much motivation to pursue this further). If you'd like to contribute to the former, I'd welcome it; if not, I'd suggest just removing the support for $motivated_person to add at a later point in time.
cleaning up the commit history.

EDIT: I would also welcome if you could give this a spin with cross-compilation, I don't currently have a device handy on which I can try this.

cc @elseym who expressed interest in this

@ofborg build nixosTests.initrd-secrets

xaverdh · 2020-12-12T16:15:17Z

First of all thanks for working on this!

* getting the zstd and lz4 tests working (I've spent about 15min on this and it's not clear to me why the kernel isn't recognising the resulting initramfs but don't have much motivation to pursue this further). If you'd like to contribute to the former, I'd welcome it; if not, I'd suggest just removing the support for `$motivated_person` to add at a later point in time.

Stupid question, did you try with a recent enough kernel (i.e. zstd is supported since version 5.9)?
Also for lz4 you need to pass the -l option (the legacy format) for the kernel to be able to decompress it.

lheckemann · 2020-12-12T17:29:13Z

Good catch! Using linuxPackages_latest makes zstd work a charm; I think I might remove the test all the same, to avoid confusion. Adding -l to lz4 does make it boot successfully, so I'll add it to the default args for lz4, but the appended secrets don't seem to take effect at all so the test still doesn't pass. Any more ideas?

xaverdh · 2020-12-12T19:59:02Z

Ok, so I now actually read the bash code in append-initrd-secrets, and I don't get whats happening there. Maybe I am stupid or something, but if I read that correctly, we create a new archive, compress that, and then literally append it to the existing compressed one. That shouldn't work, since compression and concatenation don't commute?
I would have expected the code to decompress, add the new content and recompress..

lheckemann · 2020-12-12T20:37:57Z

I believe the kernel will handle multiple concatenated and compressed files. At least this seems to be the case for most of the formats. However, I'm a lot more confused that grub seems to be giving the kernel two initramfs files…

samueldr · 2020-12-13T00:50:23Z

grub seems to be giving the kernel two initramfs files…

I have not verified this assumption, so sorry if I'm wrong.

Wouldn't there be no differences between a "pre-appended" initramfs file (cat initramfs1 initramfs2 > initramfs) and the bootloader appending in-memory?

lheckemann · 2020-12-13T19:39:57Z

I can imagine there would be other ways for a bootloader to give the kernel multiple initramfses than concatenating them.

lheckemann · 2020-12-13T19:54:51Z

Right, so it turns out that the boot menu generation scripts for grub do indeed generate a second, otherwise empty initramfs. How exactly grub passes multiple initramfses still remains unclear to me. Meanwhile, the systemd-boot scripts do pre-concatenate the files to a single initramfs. Both approaches seem to work generally, lz4 and grub is the only combination that doesn't seem to work.

xaverdh · 2020-12-14T08:11:57Z

Right, so it turns out that the boot menu generation scripts for grub do indeed generate a second, otherwise empty initramfs. How exactly grub passes multiple initramfses still remains unclear to me. Meanwhile, the systemd-boot scripts do pre-concatenate the files to a single initramfs. Both approaches seem to work generally, lz4 and grub is the only combination that doesn't seem to work.

I think the bootloader just passes the path via the kernel cmdline? And to pass several, you simply specify the option multiple times on the cmdline. At least that is how it works for early microcode loading if I remember correctly.
Ah here is the arch wiki entry on microcode loading, they indeed pass several initrd options.

samueldr · 2020-12-14T21:23:27Z

just passes the path via the kernel cmdline

There is no filesystem that the kernel is aware of at that point in time.

You're being misled :).

Here, in the list of parameters, there is confirmation:

In addition, the following text indicates that the option:
BOOT    Is a boot loader parameter.
Parameters denoted with BOOT are actually interpreted by the boot loader, and have no meaning to the kernel directly. Do not modify the syntax of boot loader parameters without extreme need or coordination with The Linux/x86 Boot Protocol.

[...]

        initrd=         [BOOT] Specify the location of the initial ramdisk

https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html

For EFI stubs, initrd= is explained:

Like most boot loaders, the EFI stub allows the user to specify multiple initrd files using the “initrd=” option. This is the only EFI stub-specific command line parameter, everything else is passed to the kernel when it boots.

https://www.kernel.org/doc/html/latest/admin-guide/efi-stub.html

I added emphasis on the section explaining how it's not the kernel that is handling the initrd parameter, but efistub.

lheckemann · 2020-12-14T21:40:08Z

I'd merge this after cleaning up the history, since it doesn't introduce any regressions AFAIU. Any opposition?

xaverdh · 2020-12-14T22:12:31Z

just passes the path via the kernel cmdline

There is no filesystem that the kernel is aware of at that point in time.

You're being misled :).

Here, in the list of parameters, there is confirmation:
In addition, the following text indicates that the option:
BOOT    Is a boot loader parameter.
Parameters denoted with BOOT are actually interpreted by the boot loader, and have no meaning to the kernel directly. Do not modify the syntax of boot loader parameters without extreme need or coordination with The Linux/x86 Boot Protocol.

Interesting, thanks for explaining!

pkgs/build-support/kernel/initrd-compressor-meta.nix

mips for example might use uImages too

IA64 (Itanium) is something completely different and certainly not what we want! x86_64 code lives in arch/x86 just like "classic" x86.

- Generate a link to the initramfs file with an appropriate file extension, guessed based on the compressor by default - Use correct metadata in u-boot images if generated, up to now this was hardcoded to gzip and would silently generate an erroneous image if another compressor was specified - Document all the parameters - Improve cross-building compatibility, by allowing passing either a string as before, or a function taking a package set and returning the path to a compressor in the "compressor" argument of the function. - Support more compression algorithms - Place compressor executable function and arguments in passthru, for reuse when appending initramfses Co-Authored-By: Dominik Xaver Hörl <hoe.dom@gmx.de>

lz4 compression is excluded because it doesn't work for a reason which remains unclear to me.

lheckemann · 2020-12-17T10:50:22Z

Will merge tomorrow, unless anybody has any objections (including wanting to review beforehand, but not having the time).

@ofborg build nixosTests.initrd-secrets nixosTests.boot

xaverdh · 2020-12-17T12:54:32Z

For what its worth, I did some basic testing and this is actually backwards compatible to some degree (i.e. if the compressor is not just an executable but includes arguments), not sure if that's intentional but nice anyway. I can't efficiently test cross sadly :-/

I have two nits concerning the documentation though:

when rendered as man page (man configuration.nix) the listing in the description of boot.initrd.compressor does not put the items on separate lines, don't know how to fix this either..
currently it says

A string representing a command available in stdenv

maybe this could include a pointer to either the meta data file or its content?

Anyway these not important points, so feel free to merge from my side.

lheckemann · 2020-12-17T21:48:13Z

not sure if that's intentional but nice anyway

fully ;)

when rendered as man page (man configuration.nix) the listing in the description of boot.initrd.compressor does not put the items on separate lines, don't know how to fix this either..

good point, this should be docbook. Will fix!

maybe this could include a pointer to either the meta data file or its content?

Yes.

lheckemann · 2020-12-17T22:01:45Z

How's this?

xaverdh · 2020-12-18T07:41:10Z

How's this?

much better :)

Ericson2314 · 2020-12-19T18:44:18Z

lib/systems/default.nix

@@ -83,7 +83,7 @@ rec {
        if final.isAarch32 then "arm"
        else if final.isAarch64 then "arm64"
        else if final.isx86_32 then "x86"
-        else if final.isx86_64 then "ia64"


ius

Apologies for reviewing a merged PR, but it appears to be the most convenient way to hilight an issue.

ius · 2021-07-30T09:56:27Z

pkgs/build-support/kernel/make-initrd.sh


 if [ -n "$makeUInitrd" ]; then
-    mv $out/initrd $out/initrd.gz
-    mkimage -A arm -O linux -T ramdisk -C gzip -d $out/initrd.gz $out/initrd
+    mkimage -A $uInitrdArch -O linux -T ramdisk -C "$uInitrdCompression" -d $out/initrd"$extension" $out/initrd.img


$uInitrdCompression is not set here due to bad name in make-initrd.nix

$out/initrd"$extension" cannot exist because the initrd is written without extension?

ius · 2021-07-30T09:56:37Z

pkgs/build-support/kernel/make-initrd.nix


-  builder = ./make-initrd.sh;
+  ${if makeUInitrd then "uinitrdCompression" else null} = uInitrdCompression;


unitrdCompression is not camelcased

ius · 2021-07-30T10:10:18Z

pkgs/build-support/kernel/make-initrd.sh

+    # Compatibility symlink
+    ln -s "initrd.img" "$out/initrd"
+else
+    ln -s "initrd" "$out/initrd$extension"


(unrelated) Other way around makes more sense (i.e. compressor suffixed file) with initrd as symlink

lheckemann requested review from Ericson2314, matthewbauer and nbp as code owners September 4, 2020 17:44

lheckemann mentioned this pull request Sep 4, 2020

initrd: cross compile fix #96005

Closed

ofborg bot added 10.rebuild-darwin: 101-500 10.rebuild-linux: 1-10 labels Sep 4, 2020

ofborg bot added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 and removed 10.rebuild-darwin: 101-500 labels Dec 10, 2020

TredwellGit mentioned this pull request Dec 16, 2020

kernel: Switch to ZSTD compression & manual-config.nix improvements #105777

Closed

10 tasks

archseer reviewed Dec 17, 2020

View reviewed changes

pkgs/build-support/kernel/initrd-compressor-meta.nix Show resolved Hide resolved

lheckemann and others added 5 commits December 17, 2020 11:10

makeInitrd: make uinitrd behaviour optional

a343ff7

makeInitrd: don't assume uImage => arm

85e0ae7

mips for example might use uImages too

makeInitrd: include dotfiles at root

3a3c9c9

lib/systems: fix kernelArch for x86_64

2ee35e1

IA64 (Itanium) is something completely different and certainly not what we want! x86_64 code lives in arch/x86 just like "classic" x86.

xaverdh and others added 2 commits December 17, 2020 11:38

nixos/initrd: add compressorArgs, make compressor option public

d4ef25d

nixos/tests: Add test for initrd secrets

3136e49

lz4 compression is excluded because it doesn't work for a reason which remains unclear to me.

lheckemann force-pushed the initrd-improvements branch from c15df3d to 3136e49 Compare December 17, 2020 10:49

nixos/initrd: docbookise "compressor" description

834cc5d

lheckemann merged commit b1fc183 into NixOS:master Dec 18, 2020

lheckemann deleted the initrd-improvements branch December 18, 2020 17:15

Ericson2314 reviewed Dec 19, 2020

View reviewed changes

xaverdh mentioned this pull request Dec 27, 2020

kernel: use zstd #107703

Merged

10 tasks

ius mentioned this pull request Jul 30, 2021

make-initrd: uImage generation is broken #132059

Closed

ius reviewed Jul 30, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initrd improvements #97145

Initrd improvements #97145

lheckemann commented Sep 4, 2020 •

edited

xaverdh commented Dec 4, 2020

lheckemann commented Dec 12, 2020 •

edited

xaverdh commented Dec 12, 2020 •

edited

lheckemann commented Dec 12, 2020 •

edited

xaverdh commented Dec 12, 2020

lheckemann commented Dec 12, 2020

samueldr commented Dec 13, 2020

lheckemann commented Dec 13, 2020

lheckemann commented Dec 13, 2020

xaverdh commented Dec 14, 2020 •

edited

samueldr commented Dec 14, 2020

lheckemann commented Dec 14, 2020

xaverdh commented Dec 14, 2020

lheckemann commented Dec 17, 2020

xaverdh commented Dec 17, 2020 •

edited

lheckemann commented Dec 17, 2020

lheckemann commented Dec 17, 2020

xaverdh commented Dec 18, 2020

Ericson2314 Dec 19, 2020

ius left a comment

ius Jul 30, 2021

ius Jul 30, 2021

ius Jul 30, 2021

ius Jul 30, 2021


		builder = ./make-initrd.sh;
		${if makeUInitrd then "uinitrdCompression" else null} = uInitrdCompression;

Initrd improvements #97145

Initrd improvements #97145

Conversation

lheckemann commented Sep 4, 2020 • edited

Motivation for this change

Things done

xaverdh commented Dec 4, 2020

lheckemann commented Dec 12, 2020 • edited

xaverdh commented Dec 12, 2020 • edited

lheckemann commented Dec 12, 2020 • edited

xaverdh commented Dec 12, 2020

lheckemann commented Dec 12, 2020

samueldr commented Dec 13, 2020

lheckemann commented Dec 13, 2020

lheckemann commented Dec 13, 2020

xaverdh commented Dec 14, 2020 • edited

samueldr commented Dec 14, 2020

lheckemann commented Dec 14, 2020

xaverdh commented Dec 14, 2020

lheckemann commented Dec 17, 2020

xaverdh commented Dec 17, 2020 • edited

lheckemann commented Dec 17, 2020

lheckemann commented Dec 17, 2020

xaverdh commented Dec 18, 2020

Ericson2314 Dec 19, 2020

Choose a reason for hiding this comment

ius left a comment

Choose a reason for hiding this comment

ius Jul 30, 2021

Choose a reason for hiding this comment

ius Jul 30, 2021

Choose a reason for hiding this comment

ius Jul 30, 2021

Choose a reason for hiding this comment

ius Jul 30, 2021

Choose a reason for hiding this comment

lheckemann commented Sep 4, 2020 •

edited

lheckemann commented Dec 12, 2020 •

edited

xaverdh commented Dec 12, 2020 •

edited

lheckemann commented Dec 12, 2020 •

edited

xaverdh commented Dec 14, 2020 •

edited

xaverdh commented Dec 17, 2020 •

edited