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

nixos/systemd-boot: Simpler windows dual booting #344327

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

iFreilicht
Copy link
Contributor

@iFreilicht iFreilicht commented Sep 24, 2024

Description of changes

When installing NixOS on a machine with Windows, the "easiest" solution
to dual-boot is re-using the existing EFI System Partition (ESP),
which allows systemd-boot to detect Windows automatically.

However, if there are multiple ESPs, maybe even on multiple disks,
systemd-boot is unable to detect the other OSes, and you either have to
use Grub and os-prober, or do a tedious manual configuration as
described in the wiki:
https://wiki.nixos.org/w/index.php?title=Dual_Booting_NixOS_and_Windows&redirect=no#EFI_with_multiple_disks

This commit automates and documents this properly so only a single line
like

boot.loader.systemd-boot.windows."Windows 10".efiDeviceHandle = "HD0c2";

is required.

In the future, we might want to try automatically detecting this
during installation, but finding the correct device handle while the
kernel is running is tricky.

This is not something we can rely on systemd-boot to fix, the issue about this has been open for years.


In addition, this PR also adds an option to add a boot-entry for edk2-uefi-shell, which is required to find the correct value for efiDeviceHandle and is generally useful for debugging and tinkering.

I also took the liberty of autoformatting, which was done in a separate commit to make review easier.

I'm uncertain if this change is significant enough to warrant a release-note.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 24, 2024
@iFreilicht iFreilicht force-pushed the simpler-windows-dual-booting branch from b1f9b9b to f15c446 Compare September 24, 2024 23:51
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Sep 24, 2024
@iFreilicht
Copy link
Contributor Author

@JulienMalka I'm tagging you as maintainer of the memtest and netboot tests, @r-vdp as you added the sortKey option and @sdht0 as you added xbootldrMountPoint. Let me know if you know of anyone else who should know about this.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Sep 25, 2024
Copy link
Contributor

@r-vdp r-vdp left a comment

Choose a reason for hiding this comment

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

Diff LGTM!

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 26, 2024
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/1980

@iFreilicht iFreilicht force-pushed the simpler-windows-dual-booting branch 2 times, most recently from 3fc05ed to e713dd1 Compare October 8, 2024 07:28
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Oct 9, 2024
@Scrumplex
Copy link
Member

Scrumplex commented Oct 9, 2024

@ofborg build systemd-boot.edk2-uefi-shell systemd-boot.windows

Edit: whoops wrong command 👀

Copy link
Member

@Scrumplex Scrumplex left a comment

Choose a reason for hiding this comment

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

Works on my machine

LGTM

@Scrumplex
Copy link
Member

@ofborg test systemd-boot.edk2-uefi-shell systemd-boot.windows

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/2031

Copy link
Member

@Atemu Atemu left a comment

Choose a reason for hiding this comment

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

Diff generally LGTM and having the option makes sense.

Btw, whatever autoformatter you used, it's not nixfmt-rfc-style. I'd recommend you fix that as that will be the only valid one in the not too distant future.

We already have a edk2-uefi-shell package in nixpkgs, but adding it to
systemd-boot was somewhat tedious. Now it's a single line of nix.
When installing NixOS on a machine with Windows, the "easiest" solution
to dual-boot is re-using the existing EFI System Partition (ESP), which
allows systemd-boot to detect Windows automatically.

However, if there are multiple ESPs, maybe even on multiple disks,
systemd-boot is unable to detect the other OSes, and you either have to
use Grub and os-prober, or do a tedious manual configuration as
described in the wiki:
https://wiki.nixos.org/w/index.php?title=Dual_Booting_NixOS_and_Windows&redirect=no#EFI_with_multiple_disks

This commit automates and documents this properly so only a single line
like

    boot.loader.systemd-boot.windows."10".efiDeviceHandle = "HD0c2";

is required.

In the future, we might want to try automatically detecting this
during installation, but finding the correct device handle while the
kernel is running is tricky.
@iFreilicht iFreilicht force-pushed the simpler-windows-dual-booting branch from e713dd1 to 73011ba Compare October 11, 2024 08:56
@iFreilicht
Copy link
Contributor Author

Btw, whatever autoformatter you used, it's not nixfmt-rfc-style

Yeah I don't remember tbh, but I reformatted it with the latest version of nixfmt-rfc-style now.

@Atemu
Copy link
Member

Atemu commented Oct 11, 2024

@ofborg test systemd-boot.edk2-uefi-shell systemd-boot.windows

@Atemu Atemu merged commit 12ef18d into NixOS:master Oct 11, 2024
28 of 29 checks passed
@iFreilicht iFreilicht deleted the simpler-windows-dual-booting branch October 11, 2024 20:05
"windows_${winVersion}.conf" = ''
title ${cfg.title}
efi /${edk2ShellEspPath}
options -nointerrupt -nomap -noversion ${cfg.efiDeviceHandle}:EFI\Microsoft\Boot\Bootmgfw.efi
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this have been bootmgfw.efi, lowercase? I know the ESP is usually case insensitive, but there have definitely been cases where that wasn't true, whether that's because of buggy firmware or custom UEFI FS drivers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, on my PC at least the path is created by Windows exactly like this, in capital case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, interesting. On mine it's lower case. I wonder why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might it be mounted case-insensitively on your machine and case-sensitively on mine?

Copy link
Contributor

Choose a reason for hiding this comment

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

It definitely is mounted case-insensitively on my machine, but I'm pretty sure that doesn't affect the case of the file names you see when listing directories.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you add boot.loader.systemd-boot.edk2-uefi-shell.enable = true; to your config and boot into the edk2 shell, and then run map -c first and then for each of the device handles run ls HD0c1:\EFI, do the paths show up as case-sensitive then?

Copy link
Contributor

@ElvishJerricco ElvishJerricco Nov 4, 2024

Choose a reason for hiding this comment

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

"Show up as case-sensitive" doesn't really make sense. FAT does store file names with their casing, and listing a directory will show the case that was used to create the files in it. It's just that accessing files will match file names case insensitively. And indeed, listing the \EFI\Microsoft\Boot\ directory shows bootmgr.efi as all lower case, meaning that is how the file is actually stored, but I can also access the file no matter the case I use for the whole path.

Like I said, it's unlikely to be an issue because accessing FAT files is case insensitive. But some implementations don't do that correctly, and it's even possible to use case-sensitive file system drivers with EFI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I didn't know that was the case.

iFreilicht added a commit to iFreilicht/.dotfiles that referenced this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants