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

systemd-boot: allow splitting entries to xbootldr part #226692

Closed
wants to merge 1 commit into from

Conversation

colemickens
Copy link
Member

Description of changes

This allows systemd-boot to be installed to the ESP, while NixOS loader entries and files can exist on the extended boot loader partition.

This is particularly useful on dual boot systems, where Windows is allowed to install first and establishes the ESP partition. Often times it chooses a very small size that is prohibitive with regards to NixOS's boot files.

This PR adds an option that allows the user to redirect the loader entries/files to (what is hopefully their) extended boot loader partition.

Note, I use this PR on systems where this extra partition is in use, AND systems where it is not, so I am not worried about regression(s).

~
❯ exa --tree --level=3 /efi
/efi
├── EFI
│  ├── Boot
│  │  └── bootx64.efi
│  ├── Microsoft
│  │  ├── Boot
│  │  └── Recovery
│  └── systemd
│     └── systemd-bootx64.efi
├── loader
│  ├── loader.conf
│  └── random-seed
└── System Volume Information

~
❯ exa --tree --level=3 /boot
/boot
├── EFI
│  ├── Linux
│  └── nixos
│     ├── 5y4wli77bnnjfz76bfran03gb6012y6d-initrd-linux-6.1.23-initrd.efi
│     ├── ady11nphd2fx30yqchq6j2jgf645waw3-linux-6.1.24-bzImage.efi
│     ├── gf7hymimhfqn7qxjh08j04jzjp18djbm-initrd-linux-6.1.23-initrd.efi
│     ├── mnayyi1fzaa4g46lb1qg46r0hjwj15fm-initrd-linux-6.1.24-initrd.efi
│     ├── qsp0g4wa01hlw4g8q4rwqbwk3kybv3z1-initrd-linux-6.1.24-initrd.efi
│     ├── xbf2az1nsckrpshiwccdmjwy2jp8hjzi-initrd-linux-6.1.23-initrd.efi
│     └── z79mf6afiz0ywr8zxbv56s13mbd8mz2h-linux-6.1.23-bzImage.efi
└── loader
   ├── entries
   │  ├── nixos-generation-89-specialisation-legacyboot.conf
   │  ├── nixos-generation-89.conf
   │  ├── nixos-generation-90-specialisation-legacyboot.conf
   │  ├── nixos-generation-90.conf
   │  ├── nixos-generation-91-specialisation-legacyboot.conf
   │  ├── nixos-generation-91.conf
   │  ├── nixos-generation-92-specialisation-legacyboot.conf
   │  └── nixos-generation-92.conf
   └── entries.srel

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.05 Release Notes (or backporting 22.11 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.

@colemickens
Copy link
Member Author

I'm partly getting this out there, possibly still too late, just to make sure it's a known use-case regarding bootspec... I'm trying to get caught back up to NixOS-world...

@RaitoBezarius
Copy link
Member

I believe XBOOTLDR is already supported by bootspec.

@colemickens
Copy link
Member Author

I will likely abandon this whenever this is resolved on lanzaboot: nix-community/lanzaboote#173

@sdht0
Copy link
Contributor

sdht0 commented Aug 17, 2023

Hi, what is needed to get this PR over the finish line?

My EFI partition is only around 100MB thanks to Windows, and in a virtual machine, I can see that NixOS already needs 90MB from two kernels.

@colemickens
Copy link
Member Author

@sdht0 as I noted in the linked lanzaboote issue you can use this to workaround for now:

config = {
  fileSystems = {
     # ..
      "/efi/EFI/Linux" = { device = "/boot/EFI/Linux"; options = ["bind"];};
      "/efi/EFI/nixos" = { device = "/boot/EFI/nixos"; options = ["bind"];};
    };
}

@sdht0
Copy link
Contributor

sdht0 commented Aug 17, 2023

Ah neat, thanks.

@ElvishJerricco
Copy link
Contributor

I think I'd prefer to have naming closer to the spec stuff; like efiSysMountPoint isn't quite what ESP stands for, but at least its close. So entriesMountPoint should probably be something more like xbootMountPoint.

@colemickens
Copy link
Member Author

@ElvishJerricco rebased, and updated per your suggestion. It's just a rename but I can end-to-end test this. My only machine with xboot part is now using lanzaboote.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/manually-partitioned-graphical-installation-of-nixos-doesnt-boot-efi-system-partition-configured-incorrectly/33419/3

Copy link
Contributor

@sdht0 sdht0 left a comment

Choose a reason for hiding this comment

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

I did not manage to get the bind mount trick working (entries were not appearing in the bootloader), so I used an override instead by copying the two files from nixpkgs:

  disabledModules = [ "system/boot/loader/systemd-boot/systemd-boot.nix" ];
  imports = [ ./systemd-boot.nix ];

@@ -81,7 +81,7 @@ def copy_from_profile(profile: Optional[str], generation: int, specialisation: O
store_dir = os.path.basename(os.path.dirname(store_file_path))
efi_file_path = "/efi/nixos/%s-%s.efi" % (store_dir, suffix)
if not dry_run:
copy_if_not_exists(store_file_path, "@efiSysMountPoint@%s" % (efi_file_path))
copy_if_not_exists(store_file_path, "@entriesMountPoint@%s" % (efi_file_path))
Copy link
Contributor

Choose a reason for hiding this comment

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

s/entriesMountPoint/xbootMountPoint/ everywhere no?


subprocess.check_call("@copyExtraFiles@")

# Since fat32 provides little recovery facilities after a crash,
# it can leave the system in an unbootable state, when a crash/outage
# happens shortly after an update. To decrease the likelihood of this
# event sync the efi filesystem after each update.
rc = libc.syncfs(os.open("@efiSysMountPoint@", os.O_RDONLY))
rc = libc.syncfs(os.open("@entriesMountPoint@", os.O_RDONLY))
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a second syncfs command:

if "@xbootMountPoint@" != "@efiSysMountPoint@":
        rc = libc.syncfs(os.open("@efiSysMountPoint@", os.O_RDONLY))
        if rc != 0:
            print("could not sync @efiSysMountPoint@: {}".format(os.strerror(rc)), file=sys.stderr)

@@ -286,8 +286,8 @@ def main() -> None:
print("updating systemd-boot from %s to %s" % (installed_version, available_version))
subprocess.check_call(["@systemd@/bin/bootctl", "--esp-path=@efiSysMountPoint@"] + bootctl_flags + ["update"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Add an explicit boot-path?

if "@xbootMountPoint@" != "@efiSysMountPoint@":
        bootctl_flags.append("--boot-path=@xbootMountPoint@")

@colemickens
Copy link
Member Author

@sdht0 do you want to take this over maybe? You left a good review and I clearly didn't test this when I pushed it last. It might be good for you to take it over, if you want. If you want, just open a new one and CC me on it and I'll close this one in favor of it.

@sdht0
Copy link
Contributor

sdht0 commented Oct 1, 2023

I don't mind. I'll prepare the PR in the next couple of weeks, after I've added the ability to cleanup entries on a config change.

Is there anything else I need to do to get it actually merged? This PR has been open for a while.

@colemickens
Copy link
Member Author

I think I'd want another review from @ElvishJerricco, but I'm not really aware of anything else that should block it.

Also, if @RaitoBezarius can weigh in on whether or not XBOOTLDR is officially supported by bootspec, or not, and whether or not that should affect the usefulness or mergability of this PR. I'm not sure I can speak to that.

@ElvishJerricco
Copy link
Contributor

I guess one other noteworthy thing is that extraFiles and extraEntries probably should be adapted to copy to the XBOOTLDR instead of the ESP.

As a future a note, a good followup PR would be one that enables copying drivers to the $esp/EFI/systemd/drivers/ to support more file system types for XBOOTLDR (and whatever else drivers can do), along with packaging efifs for FS driver implementations.

@RaitoBezarius
Copy link
Member

Also, if @RaitoBezarius can weigh in on whether or not XBOOTLDR is officially supported by bootspec, or not, and whether or not that should affect the usefulness or mergability of this PR. I'm not sure I can speak to that.

Per se, XBOOTLDR is not formalized inside bootspec in its version 1, but you can always add it as an extension and we can document it.

I don't think anything except the builder needs the information about the entries mountpoint.

Drivers will also be necessary if the target entry mountpoint is not a supported FS for UEFI.

@RaitoBezarius
Copy link
Member

I also think it's extremely important for that PR to have tests.

@sdht0
Copy link
Contributor

sdht0 commented Oct 10, 2023

New PR is at #260241

@colemickens
Copy link
Member Author

Thanks for continuing, fixing and improving this, @sdht0, especially the tests. Closing in favor of #260241

@sdht0 sdht0 mentioned this pull request Nov 6, 2023
5 tasks
@colemickens colemickens deleted the xbootloader branch February 17, 2024 04:08
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.

5 participants