diff mbox series

[bug#55231,v1] initrd: Allow extra search paths with ‘initrd-extra-module-paths’

Message ID 87wnf3pv87.fsf@ditto.jhoto.spork.org
State New
Headers show
Series [bug#55231,v1] initrd: Allow extra search paths with ‘initrd-extra-module-paths’ | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Brian Cully May 2, 2022, 7:11 p.m. UTC
This patch allows copying of out-of-tree kernel modules to the Linux
initrd.

For out-of-tree modules to found, an extra slot has been added to
‘operating-system’: ‘initrd-extra-module-paths’, which contains a list
containing items of either FILE-LIKE or (FILE-LIKE OUTPUT) which will be
searched for modules in addition to the standard Linux module path. The
required modules can then be added to ‘initrd-modules’ as normal and all paths
will be searched for it, including for any modules depended on.

* gnu/build/linux-modules.scm (find-module-file): change DIRECTORY argument to
DIRECTORIES. Now takes a list of directories to search, rather than a single
one.
* gnu/system.scm <operating-system>: Add INITRD-EXTRA-MODULE-PATHS
field and accessor. Takes a list of file-like objects.
* gnu/system/linux-initrd.scm (flat-linux-module-directory): change LINUX
argument to PACKAGES. Now contains a list of file-likes to search for modules.
(raw-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it
to (flat-linux-extra-module-paths) along with the selected LINUX package.
(base-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it
to (raw-initrd).
---
 gnu/build/linux-modules.scm | 19 ++++++++------
 gnu/system.scm              |  5 ++++
 gnu/system/linux-initrd.scm | 52 +++++++++++++++++++++++++------------
 3 files changed, 51 insertions(+), 25 deletions(-)


base-commit: 0a64b629ae8512790d532158a72a4a25698e8157
prerequisite-patch-id: 213ce2b9743f11d45836fe0794f117f3bb84063d

Comments

M May 2, 2022, 10:03 p.m. UTC | #1
Brian Cully via Guix-patches via schreef op ma 02-05-2022 om 15:11 [-
0400]:
> This patch allows copying of out-of-tree kernel modules to the Linux
> initrd.

This needs some information in the manual -- when does the field need
to be set?  For what kernel modules?  Is this required by v4l2loopback-
linux-module and librem-ec-acpi-linux-module?  Things like that.
As-is, this functionality is hard to discover.

Greetings,
Maxime.
Kaelyn Takata May 13, 2022, 7:46 p.m. UTC | #2
I've read through the original patch and the doc patch, and based on my own tinkering with guix initrd generation, the patches look good to me. I'm also happy to see the functionality being added, and already have plans for using it with zfs (to further the goal of an encrypted multi-disk zfs root).

Cheers,
Kaelyn
M May 13, 2022, 8:25 p.m. UTC | #3
(for whatever reason, looks like this didn't end up in my inbox)
> +@item @code{initrd-extra-module-paths} (default: @code{'()})
> [...]
> The items in this may be either file-like objects (usually 
> packages), or
> +a list where the first element is a package and the second is an
> +output--e.g. @code{(list (list zfs "module"))}.


As-is, I don't think this is a good example, because
'expression->initrd' does not set #:substitutable? #false,
the 'zfs' package has the comment (*)

     `(;; The ZFS kernel module should not be downloaded since the
license
       ;; terms don't allow for distributing it, only building it
locally.
       #:substitutable? #f [...])

and IIUC, the code inside expression->initrd copies the kernel module
into
the new store item, so it looks like this accidentally suggest people
to
commit copyvios, and copyvios are currently against the law.

Greetings,
Maxime.

(*) Though I don't understand that comment: Guix is a distribution, so
by definition
it's distributing zfs -- unless it's only talking about the binaries
and not the
source code ...
Kaelyn Takata May 30, 2022, 8:14 p.m. UTC | #4
As the earlier patch discussion seemed to have focused on ZFS licensing issues that are largely orothogonal to the functionality being added, I'd like to again voice my support for the proposed patch. While ZFS is indeed a bad example due to open questions about module licensing and redistribution, it isn't the only out-of-tree module currently packaged in Guix which could be used with the extra initrd support (not counting future packages or external channels). Within the guix repo, the packages using the `linux-module-build-system` aside from zfs are:
- acpi-call-linux-module
- bbswitch-module
- ddcci-driver-linux
- rtl8812au-aircrack-ng-linux-module
- rtl8821ce-linux-module
- ttyebus-linux-module
- v4l2loopback-linux-module
- vhba-module
- wiregard-linux-compat

Of those nine, at least four look to be for specific hardware, and I can come up with theoretical use cases for why someone might want those or wiregard-linux-compat (though the cases are definitely contrived straw-men that may or may not be implementable in practice or actually useful in an initrd context, such as network booting with an older kernel and the root filesystem can only be accessed over a wireguard connection).

Cheers,
Kaelyn
Kaelyn Takata June 2, 2022, 9:23 p.m. UTC | #5
Hi,

My $0.02 on 'initrd-extra-module-paths' vs 'kernel-loadable-modules': if the initrd code is smart enough to not include the entire packages and only includes the requested modules from the packages (which I think is already true based on behavior observed some time ago), then not having to duplicate the list would be preferable.

I found this issue in the tracker after being surprised that the initrd builder ignored 'kernel-loadable-modules' with 'initrd-modules' only working for modules that are included in the primary linux kernel package. That surprise over the incompatibility between 'initrd-modules' and 'kernel-loadable-modules' is also why I've been relatively vocal about the patch. ;)

Cheers,
Kaelyn
Kaelyn Takata June 21, 2022, 12:34 p.m. UTC | #6
Hi again,

Regarding the documentation example, one (contrived) alternative is:

"""
For example, if you need the driver for a Realtek RTL8821CE wireless network adapter for mounting the root filesystem over NFS, your configuration might include the following:

@lisp
(operating-system
  ;; @dots{}
  (initrd-modules (cons "8821ce" %base-initrd-modules))
  (kernel-loadable-modules (list (list rtl8821ce-linux-module "module"))))
@end lisp
"""

While I don't have the hardware, I did verify the kernel module name by building the rtl8821ce-linux-module package.

Cheers,
Kaelyn
diff mbox series

Patch

diff --git a/gnu/build/linux-modules.scm b/gnu/build/linux-modules.scm
index 053720574b..97b7e429ea 100644
--- a/gnu/build/linux-modules.scm
+++ b/gnu/build/linux-modules.scm
@@ -225,8 +225,8 @@  (define (file-name->module-name file)
 '.ko[.gz|.xz]' and normalizing it."
   (normalize-module-name (strip-extension (basename file))))
 
-(define (find-module-file directory module)
-  "Lookup module NAME under DIRECTORY, and return its absolute file name.
+(define (find-module-file directories module)
+  "Lookup module NAME under DIRECTORIES, and return its absolute file name.
 NAME can be a file name with or without '.ko', or it can be a module name.
 Raise an error if it could not be found.
 
@@ -247,16 +247,19 @@  (define names
                            (else chr)))
                        module))))
 
-  (match (find-files directory
-                     (lambda (file stat)
-                       (member (strip-extension
-                                (basename file)) names)))
+  (match (append-map (lambda (directory)
+                       (find-files directory
+                                   (lambda (file _stat)
+                                     (member (strip-extension
+                                              (basename file))
+                                             names))))
+                       directories)
     ((file)
      file)
     (()
-     (error "kernel module not found" module directory))
+     (error "kernel module not found" module directories))
     ((_ ...)
-     (error "several modules by that name" module directory))))
+     (error "several modules by that name" module directories))))
 
 (define* (recursive-module-dependencies files
                                         #:key (lookup-module dot-ko))
diff --git a/gnu/system.scm b/gnu/system.scm
index c3810cbeeb..15ac30c933 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -103,6 +103,7 @@  (define-module (gnu system)
             operating-system-label
             operating-system-default-label
             operating-system-initrd-modules
+            operating-system-initrd-extra-module-paths
             operating-system-initrd
             operating-system-users
             operating-system-groups
@@ -232,6 +233,8 @@  (define-record-type* <operating-system> operating-system
   (initrd-modules operating-system-initrd-modules ; list of strings
                   (thunked)                       ; it's system-dependent
                   (default %base-initrd-modules))
+  (initrd-extra-module-paths operating-system-initrd-extra-module-paths ; list of file-likes
+                             (default '()))
 
   (firmware operating-system-firmware             ; list of packages
             (default %base-firmware))
@@ -1307,6 +1310,8 @@  (define make-initrd
                #:linux (operating-system-kernel os)
                #:linux-modules
                (operating-system-initrd-modules os)
+               #:linux-extra-module-paths
+               (operating-system-initrd-extra-module-paths os)
                #:mapped-devices mapped-devices
                #:keyboard-layout (operating-system-keyboard-layout os)))
 
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 4c4c78e444..50a182d7d5 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -119,13 +119,23 @@  (define builder
                               `(#:references-graphs (("closure" ,init))))
                "/initrd.cpio.gz"))
 
-(define (flat-linux-module-directory linux modules)
+(define (flat-linux-module-directory packages modules)
   "Return a flat directory containing the Linux kernel modules listed in
-MODULES and taken from LINUX."
+MODULES and taken from PACKAGES."
   (define imported-modules
     (source-module-closure '((gnu build linux-modules)
                              (guix build utils))))
 
+  (define (package+out->input package out)
+    (gexp-input package out))
+
+  (define package-inputs
+    (map (lambda (p)
+           (match p
+             ((p o) (package+out->input p o))
+             (p     (package+out->input p "out"))))
+         packages))
+
   (define build-exp
     (with-imported-modules imported-modules
       (with-extensions (list guile-zlib)
@@ -135,11 +145,12 @@  (define build-exp
                          (srfi srfi-1)
                          (srfi srfi-26))
 
-            (define module-dir
-              (string-append #$linux "/lib/modules"))
+            (define module-dirs
+              (map (cut string-append <> "/lib/modules")
+                   '#$package-inputs))
 
             (define modules
-              (let* ((lookup  (cut find-module-file module-dir <>))
+              (let* ((lookup  (cut find-module-file module-dirs <>))
                      (modules (map lookup '#$modules)))
                 (append modules
                         (recursive-module-dependencies
@@ -172,20 +183,23 @@  (define* (raw-initrd file-systems
                       #:key
                       (linux linux-libre)
                       (linux-modules '())
+                      (linux-extra-module-paths '())
                       (mapped-devices '())
                       (keyboard-layout #f)
                       (helper-packages '())
                       qemu-networking?
                       volatile-root?
                       (on-error 'debug))
-  "Return as a file-like object a raw initrd, with kernel
-modules taken from LINUX.  FILE-SYSTEMS is a list of file-systems to be
-mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via 'root'.  LINUX-MODULES is a list of kernel
-modules to be loaded at boot time. MAPPED-DEVICES is a list of device
-mappings to realize before FILE-SYSTEMS are mounted.
-HELPER-PACKAGES is a list of packages to be copied in the initrd. It may include
-e2fsck/static or other packages needed by the initrd to check root partition.
+  "Return as a file-like object a raw initrd, with kernel modules taken from
+LINUX.  FILE-SYSTEMS is a list of file-systems to be mounted by the initrd,
+possibly in addition to the root file system specified on the kernel command
+line via 'root'.  LINUX-MODULES is a list of kernel modules to be loaded at
+boot time. LINUX-EXTRA-MODULE-PATHS is a list of file-like objects which will
+be searched for modules in addition to the linux kernel. MAPPED-DEVICES is a
+list of device mappings to realize before FILE-SYSTEMS are mounted.
+HELPER-PACKAGES is a list of packages to be copied in the initrd. It may
+include e2fsck/static or other packages needed by the initrd to check root
+partition.
 
 When true, KEYBOARD-LAYOUT is a <keyboard-layout> record denoting the desired
 console keyboard layout.  This is done before MAPPED-DEVICES are set up and
@@ -221,7 +235,8 @@  (define file-system-scan-commands
           #~())))
 
   (define kodir
-    (flat-linux-module-directory linux linux-modules))
+    (flat-linux-module-directory (cons linux linux-extra-module-paths)
+                                 linux-modules))
 
   (expression->initrd
    (with-imported-modules (source-module-closure
@@ -366,6 +381,7 @@  (define* (base-initrd file-systems
                       #:key
                       (linux linux-libre)
                       (linux-modules '())
+                      (linux-extra-module-paths '())
                       (mapped-devices '())
                       (keyboard-layout #f)
                       qemu-networking?
@@ -386,9 +402,10 @@  (define* (base-initrd file-systems
 QEMU-NETWORKING? and VOLATILE-ROOT? behaves as in raw-initrd.
 
 The initrd is automatically populated with all the kernel modules necessary
-for FILE-SYSTEMS and for the given options.  Additional kernel
-modules can be listed in LINUX-MODULES.  They will be added to the initrd, and
-loaded at boot time in the order in which they appear."
+for FILE-SYSTEMS and for the given options.  Additional kernel modules can be
+listed in LINUX-MODULES.  Additional search paths for modules can be listed in
+LINUX-EXTRA-MODULE-PATHS.  They will be added to the initrd, and loaded at
+boot time in the order in which they appear."
   (define linux-modules*
     ;; Modules added to the initrd and loaded from the initrd.
     `(,@linux-modules
@@ -408,6 +425,7 @@  (define helper-packages
   (raw-initrd file-systems
               #:linux linux
               #:linux-modules linux-modules*
+              #:linux-extra-module-paths linux-extra-module-paths
               #:mapped-devices mapped-devices
               #:helper-packages helper-packages
               #:keyboard-layout keyboard-layout