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 conditionals for all architecture dependent initrd modules #109280

Open
primeos opened this issue Jan 13, 2021 · 11 comments
Open

Use conditionals for all architecture dependent initrd modules #109280

primeos opened this issue Jan 13, 2021 · 11 comments

Comments

@primeos
Copy link
Member

primeos commented Jan 13, 2021

This is a meta issue to track the changes that are required since we've merged #78430 (resulted in some channel blockers: #109252).

This is important for everything that is part of modulesClosure (allowMissing is now false):

  # Determine the set of modules that we need to mount the root FS.
  modulesClosure = pkgs.makeModulesClosure {
    rootModules = config.boot.initrd.availableKernelModules ++ config.boot.initrd.kernelModules;
    kernel = modulesTree;
    firmware = firmware;
    allowMissing = false;
  };

But for missing firmware we only print a warning: 49130f9

Relevant commits:

Open/unresolved issues to consider:

  • This breaking change can be problematic for users
    • E.g. nixos/stage-1: Do not allow missing kernel modules in initrd #78430 (comment): "This makes it really annoying to use NixOS with a custom built kernel. There is a whole bunch of modules that get added to the initrd "by default" in nixos/modules/system/boot/kernel.nix and can't easily be removed:"
    • Can break on uncommon architectures (AFAIK we only test AMD64 and AArch64 on Hydra)
    • Can break with a custom kernel configuration
    • Can break with existing configs (that include typos or modules that aren't available on $arch)

Potential improvements:

  • Should we add a NixOS option for allowMissing that defaults to false?
    • Reasoning for this idea: We can fix regressions on Hydra and users could contribute fixes for other architectures but we cannot consider all custom kernel configurations (and minimal kernel configs will miss a lot of modules!)
  • The error message is not great
    • Why does it suddenly fail and not before -> maybe we could/should add an explanation and instructions (or link to this issue)?
@primeos primeos changed the title Use conditionals for all architecture dependent Linux kernel modules in Nixpkgs Use conditionals for all architecture dependent initrd modules Jan 13, 2021
@xaverdh
Copy link
Contributor

xaverdh commented Jan 13, 2021

So a quick way to find places where issues will pop up, is to look at previously successfully builds (The log of modules-shrunk.drv). Every FATAL error message from modinfo in these logs corresponds to a missing module, that will now cause the build to abort.

@xaverdh
Copy link
Contributor

xaverdh commented Jan 13, 2021

Should we add a NixOS option for allowMissing that defaults to false?
Reasoning for this idea: We can fix regressions on Hydra and users could contribute fixes for other architectures but we cannot consider all custom kernel configurations (and minimal kernel configs will miss a lot of modules!)

We should consider this as a temporary stop gap measure, but what we really want to do here is make the module system less opinionated on the list of default modules.

@vcunat
Copy link
Member

vcunat commented Jan 13, 2021

Perhaps have two lists? One where failure is OK and another one where failure is fatal?

@xaverdh
Copy link
Contributor

xaverdh commented Jan 13, 2021

Perhaps have two lists? One where failure is OK and another one where failure is fatal?

The thing is, we actually do want the build to fail, if somebody makes a typo in their config. So if we have a list of "optional" modules, we should somehow distinguish those cases.
Moreover if somebody explicitly adds a module to their config, we should detect if its not there.
But this can probably be computed in the module system. Taking the difference of the "optional default modules" minus the explicitly given modules, and only allow those to be missing.

@xaverdh
Copy link
Contributor

xaverdh commented Jan 13, 2021

@delroth since you have a custom kernel setup, would it help if the list in nixos/modules/profiles/all-hardware.nix could be separately switched off?

@delroth
Copy link
Contributor

delroth commented Jan 13, 2021

I think all-hardware.nix is only really used in rare situations e.g. building installer images or livecd or such, I personally don't really care about it.

Really the main issue is that the NixOS module system allows for modules merging things into a list but not for modules filtering things out of a list -- given that I have a "weird" config due to having to use a custom built kernel, I would be OK with having to manually filter out the default modules I don't have, but that's not something that can easily be done :(

(mkForce overrides the list completely, so that breaks the "other modules merging things into the list" part of the problem.)

@vcunat
Copy link
Member

vcunat commented Jan 13, 2021

One way I can imagine is to convert the lists into attrsets. That's seems the usual approach to cases when you have a list in NixOS config and you actually need to remove parts.

@samueldr
Copy link
Member

samueldr commented Feb 2, 2021

The image builder profile adds modules to the available kernel modules:

This profile is also used by the Raspberry Pi 4 sd image, which uses an alternate kernel and configuration. In turn, see #111683, this module is not present on that specialized configuration.

The solution should make it possible for users of the NixOS options to "maybe add modules" too. Just saying in case it wasn't the foregone and obvious conclusion :).

@samueldr
Copy link
Member

An update to my last comment:

The image builder does not do this anymore. The all-hardware.nix profile is used instead.

  • ++ lib.optionals (!platform.isAarch64 && !platform.isAarch32) [ # not sure where else they're missing
    "vmw_vmci" "vmwgfx" "vmw_vsock_vmci_transport"
    # Hyper-V support.
    "hv_storvsc"
    ] ++ lib.optionals (pkgs.stdenv.isAarch32 || pkgs.stdenv.isAarch64) [
    # Most of the following falls into two categories:
    # - early KMS / early display
    # - early storage (e.g. USB) support
    # Allows using framebuffer configured by the initial boot firmware
    "simplefb"
    # Allwinner support
    # Required for early KMS
    "sun4i-drm"
    "sun8i-mixer" # Audio, but required for kms
    # PWM for the backlight
    "pwm-sun4i"
    # Broadcom
    "vc4"
    ] ++ lib.optionals pkgs.stdenv.isAarch64 [
    # Most of the following falls into two categories:
    # - early KMS / early display
    # - early storage (e.g. USB) support
    # Broadcom
    "pcie-brcmstb"
    # Rockchip
    "dw-hdmi"
    "dw-mipi-dsi"
    "rockchipdrm"
    "rockchip-rga"
    "phy-rockchip-pcie"
    "pcie-rockchip-host"
    # Misc. uncategorized hardware
    # Used for some platform's integrated displays
    "panel-simple"
    "pwm-bl"
    # Power supply drivers, some platforms need them for USB
    "axp20x-ac-power"
    "axp20x-battery"
    "pinctrl-axp209"
    "mp8859"
    # USB drivers
    "xhci-pci-renesas"
    # Misc "weak" dependencies
    "analogix-dp"
    "analogix-anx6345" # For DP or eDP (e.g. integrated display)
    ];

We also do not have the Pi 4 SD image files anymore.

@stale
Copy link

stale bot commented Nov 9, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 9, 2021
@samueldr
Copy link
Member

Workaround from here (actually from @cleverca22):

{
/* ... */
overlays = [
  (final: super: {
    makeModulesClosure = x:
      super.makeModulesClosure (x // { allowMissing = true; });
  })
];
/* ... */
}

Hopefully I'll find it quicker next time if I mention it here.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants