From 91767ab0824680a9c60d06aeb26ea7cc377e133d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 10 Oct 2023 16:32:16 +0200 Subject: [PATCH 1/5] kernel-install/90-loaderentry: do not read dtbs from /boot /boot is not trusted, so we shouldn't use load files from there. Also, space in /boot is limited, so it doesn't make sense to install the files under one location there and then copy them to a different location. We should only copy the files from /usr somewhere and then install it in the appropriate place under /boot. Also use "/usr/lib" instead of the "/lib" prefix. We don't support unmerged-user anymore. Addresses some of the feedback in https://github.com/systemd/systemd/pull/28582#discussion_r1285820556. --- src/kernel-install/90-loaderentry.install.in | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/kernel-install/90-loaderentry.install.in b/src/kernel-install/90-loaderentry.install.in index dc7c371e5a1..02497211b39 100755 --- a/src/kernel-install/90-loaderentry.install.in +++ b/src/kernel-install/90-loaderentry.install.in @@ -126,11 +126,9 @@ elif [ -f /usr/lib/kernel/devicetree ]; then fi if [ -n "$DEVICETREE" ]; then for prefix in \ - "/boot/dtb-$KERNEL_VERSION" \ - "/boot/dtbs/$KERNEL_VERSION" \ - "/lib/firmware/$KERNEL_VERSION/device-tree" \ - "/lib/linux-image-$KERNEL_VERSION" \ - "/lib/modules/$KERNEL_VERSION/dtb" + "/usr/lib/firmware/$KERNEL_VERSION/device-tree" \ + "/usr/lib/linux-image-$KERNEL_VERSION" \ + "/usr/lib/modules/$KERNEL_VERSION/dtb" do [ -f "$prefix/$DEVICETREE" ] || continue DEVICETREE_SRC="$prefix/$DEVICETREE" From 7a9d86bceb4fa99c2eb86b2d06e71346f1001953 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 11 Oct 2023 17:34:43 +0200 Subject: [PATCH 2/5] man/kernel-install: fix formatting and document /etc/kernel/devicetree Each filename should be a separate , so that they separated in the formatted text. Also, we list files in documentation in priority order, but here they were in reverse order. Also, rework the description of $KERNEL_INSTALL_CONF_ROOT to say that it makes kernel-install not look at the other files. This requires some more words, so make this a separate paragraph and refer from individual items to it. Also, drop some sentences with "Read by ...", they were already outdated. Partial fix for #28771. Co-authored-by: Emil Renner Berthing --- man/kernel-install.xml | 194 ++++++++++++++++++++++------------------- 1 file changed, 105 insertions(+), 89 deletions(-) diff --git a/man/kernel-install.xml b/man/kernel-install.xml index ddcc2184ac4..889520ff6e0 100644 --- a/man/kernel-install.xml +++ b/man/kernel-install.xml @@ -471,64 +471,81 @@ Files - - /usr/lib/kernel/install.d/*.install - /etc/kernel/install.d/*.install - - - Drop-in files which are executed by kernel-install. + /etc/kernel/install.d/*.install + /usr/lib/kernel/install.d/*.install + + Drop-in files which are executed by kernel-install. - + + - - /usr/lib/kernel/cmdline - /etc/kernel/cmdline - /proc/cmdline - - - Read by 90-loaderentry.install. The content of the file - /etc/kernel/cmdline specifies the kernel command line to use. If that file - does not exist, /usr/lib/kernel/cmdline is used. If that also does not - exist, /proc/cmdline is used. $KERNEL_INSTALL_CONF_ROOT - may be used to override the path. + /etc/kernel/cmdline + /usr/lib/kernel/cmdline + /proc/cmdline + + Specifies the kernel command line to use. The first of the files that is found will be used. + $KERNEL_INSTALL_CONF_ROOT may be used to override the search path; see below for + details. - + + - - /etc/kernel/tries - - - Read by 90-loaderentry.install and - 90-uki-copy.install. If this file exists a numeric value is read from it - and the naming of the generated entry file or UKI is slightly altered to include it as - $BOOT/loader/entries/ENTRY-TOKEN-KERNEL-VERSION+TRIES.conf - or - $BOOT/EFI/Linux/ENTRY-TOKEN-KERNEL-VERSION+TRIES.efi, respectively. This - is useful for boot loaders such as - systemd-boot7 - which implement boot attempt counting with a counter embedded in the entry file name. - $KERNEL_INSTALL_CONF_ROOT may be used to override the path. + /etc/kernel/devicetree + /usr/lib/kernel/devicetree + + Specifies the partial path to the file containing the device tree blob to install with the + kernel and use at boot. The first of the files that is found will be used. + $KERNEL_INSTALL_CONF_ROOT may be used to override the search path; see below for + details. + + The devicetree file contains a path, and this path specifies a location + relative to the kernel install tree. A set of locations is checked, including in particular + /usr/lib/modules/KERNEL_VERSION/dtb/, which is the + recommended location to place the dtb files under. For example, with + broadcom/bcm2711-rpi-4-b.dtb in the devicetree file, the + device tree blob for the Raspberry Pi 4 Model B would be installed, and the actual file would be + /usr/lib/modules/KERNEL_VERSION/dtb/broadcom/bcm2711-rpi-4-b.dtb. + + + + + + + + /etc/kernel/tries + + Read by 90-loaderentry.install and + 90-uki-copy.install. If this file exists, a numeric value is read from it and + the naming of the generated entry file or UKI is altered to include it as + $BOOT/loader/entries/ENTRY-TOKEN-KERNEL-VERSION+TRIES.conf + or + $BOOT/EFI/Linux/ENTRY-TOKEN-KERNEL-VERSION+TRIES.efi, + respectively. This is useful for boot loaders such as + systemd-boot7 + which implement boot attempt counting with a counter embedded in the entry file name. + $KERNEL_INSTALL_CONF_ROOT may be used to override the search path; see below for + details. - + - - - /etc/kernel/entry-token - - - If this file exists it is read and used as "entry token" for this system, i.e. is used for - naming Boot Loader Specification entries, see $KERNEL_INSTALL_ENTRY_TOKEN - above for details. $KERNEL_INSTALL_CONF_ROOT may be used to override the - path. - - + + /etc/kernel/entry-token + + If this file exists it is read and used as "entry token" for this system, i.e. is used for + naming Boot Loader Specification entries. See $KERNEL_INSTALL_ENTRY_TOKEN above + for details. $KERNEL_INSTALL_CONF_ROOT may be used to override the search path; see + below for details. + + + + /etc/machine-id @@ -540,63 +557,62 @@ + - - /etc/os-release - /usr/lib/os-release - + /etc/os-release + /usr/lib/os-release - Read by 90-loaderentry.install. - If available, PRETTY_NAME= is read from these files and used as the title of the boot menu entry. - Otherwise, Linux KERNEL-VERSION will be used. + Read by 90-loaderentry.install. If available, + PRETTY_NAME= is read from these files and used as the title of the boot menu + entry. Otherwise, Linux KERNEL-VERSION will be + used. - + - - /usr/lib/kernel/install.conf - /etc/kernel/install.conf - - - Configuration options for kernel-install, as a series of - KEY=VALUE assignments, compatible with shell - syntax, following the same rules as described in - os-release5. - /etc/kernel/install.conf will be read if present, and - /usr/lib/kernel/install.conf otherwise. This file is optional. - $KERNEL_INSTALL_CONF_ROOT may be used to override the path. - + /etc/kernel/install.conf + /usr/lib/kernel/install.conf + + Configuration file with options for kernel-install, as a series of + KEY=VALUE assignments, compatible with shell syntax, + following the same rules as described in + os-release5. The + first of the files that is found will be used. $KERNEL_INSTALL_CONF_ROOT may be + used to override the search path; see below for details. - Currently, the following keys are supported: - MACHINE_ID=, - BOOT_ROOT=, - layout=, - initrd_generator=, - uki_generator=. - See the Environment variables section above for details. + Currently, the following keys are supported: + MACHINE_ID=, + BOOT_ROOT=, + layout=, + initrd_generator=, + uki_generator=. + See the Environment variables section above for details. - - + + - - /etc/kernel/uki.conf - - - Ini-style configuration file for - ukify1 which - is only effective when $KERNEL_INSTALL_LAYOUT or layout= in - install.conf is set to and - $KERNEL_INSTALL_UKI_GENERATOR or uki_generator= in - install.conf is set to . - $KERNEL_INSTALL_CONF_ROOT may be used to override the path. - + /etc/kernel/uki.conf + + Ini-style configuration file for + ukify1 which is + only effective when $KERNEL_INSTALL_LAYOUT or layout= in + install.conf is set to and + $KERNEL_INSTALL_UKI_GENERATOR or uki_generator= in + install.conf is set to . + $KERNEL_INSTALL_CONF_ROOT may be used to override the search path; see below for + details. - - + + + + For various cases listed above, if the $KERNEL_INSTALL_CONF_ROOT environment + variable is set, it will override the search path. The files will be loaded only + from the directory specified by the environment variable. When the variable is not set, the listed paths + are tried in turn, and the first file that exists is used. From 40c0c9d4bcb1ff47af9935dea538e38447c09118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 10 Oct 2023 18:53:55 +0200 Subject: [PATCH 3/5] kernel-install/60-ukify: add helper function for locating input files Also rename config_file_location() to uki_conf_location() to make it obvious which config file was meant. --- src/kernel-install/60-ukify.install.in | 40 +++++++++++++++----------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/kernel-install/60-ukify.install.in b/src/kernel-install/60-ukify.install.in index 01146467d1a..44a7750da50 100755 --- a/src/kernel-install/60-ukify.install.in +++ b/src/kernel-install/60-ukify.install.in @@ -124,27 +124,35 @@ def we_are_wanted() -> bool: return True -def config_file_location() -> Optional[Path]: +def input_file_location( + filename: str, + *search_directories: str) -> Optional[Path]: + if root := os.getenv('KERNEL_INSTALL_CONF_ROOT'): - p = Path(root) / 'uki.conf' - else: - p = Path('/etc/kernel/uki.conf') - if p.exists(): - return p + search_directories = (root,) + elif not search_directories: + # This is the default search path. + search_directories = ('/etc/kernel', + '/usr/lib/kernel') + + for dir in search_directories: + p = Path(dir) / filename + if p.exists(): + return p return None +def uki_conf_location() -> Optional[Path]: + return input_file_location('uki.conf', + '/etc/kernel') + + def kernel_cmdline_base() -> list[str]: - if root := os.getenv('KERNEL_INSTALL_CONF_ROOT'): - return Path(root).joinpath('cmdline').read_text().split() - - for cmdline in ('/etc/kernel/cmdline', - '/usr/lib/kernel/cmdline'): - try: - return Path(cmdline).read_text().split() - except FileNotFoundError: - continue + path = input_file_location('cmdline') + if path: + return path.read_text().split() + # If we read /proc/cmdline, we need to do some additional filtering. options = Path('/proc/cmdline').read_text().split() return [opt for opt in options if not opt.startswith(('BOOT_IMAGE=', 'initrd='))] @@ -193,7 +201,7 @@ def call_ukify(opts): # argument set to prepopulate the namespace with the defaults. opts2 = ukify['create_parser']().parse_args(['build']) - opts2.config = config_file_location() + opts2.config = uki_conf_location() opts2.uname = opts.kernel_version opts2.linux = opts.kernel_image opts2.initrd = initrd_list(opts) From e3696e6f631f340f8e32c01b30d0a4a02b3f1ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 10 Oct 2023 19:39:59 +0200 Subject: [PATCH 4/5] ukify: fix .dtb section name in 'inspect' Fixup for df4a46733a609f1673de0bebb38e89fffd70c16c. --- src/ukify/ukify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 99a0d72650b..04dd4f958f9 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -250,7 +250,7 @@ DEFAULT_SECTIONS_TO_SHOW = { '.linux' : 'binary', '.initrd' : 'binary', '.splash' : 'binary', - '.dt' : 'binary', + '.dtb' : 'binary', '.cmdline' : 'text', '.osrel' : 'text', '.uname' : 'text', From 02d8d787f38cb4a02b16f2f85d8120394574b054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 10 Oct 2023 19:41:35 +0200 Subject: [PATCH 5/5] kernel-install/60-ukify: also support the convention with 'devicetree' file Requested in https://github.com/systemd/systemd/pull/28582#issuecomment-1673300596. The is the last requested changed, so fixes #28771. 90-loaderentry.install is modified to also check $KERNEL_INSTALL_CONF_ROOT when looking for the devicetree file. For normal use this is probably not needed, but it's nice to be consistent and it also makes it much easier to write the tests. In tests, also do 'ukify inspect' now that we have it. --- src/kernel-install/60-ukify.install.in | 28 ++++++++++++++++++++ src/kernel-install/90-loaderentry.install.in | 2 ++ src/kernel-install/test-kernel-install.sh | 17 ++++++++++++ 3 files changed, 47 insertions(+) diff --git a/src/kernel-install/60-ukify.install.in b/src/kernel-install/60-ukify.install.in index 44a7750da50..21ef694ba1f 100755 --- a/src/kernel-install/60-ukify.install.in +++ b/src/kernel-install/60-ukify.install.in @@ -147,6 +147,31 @@ def uki_conf_location() -> Optional[Path]: '/etc/kernel') +def devicetree_config_location() -> Optional[Path]: + return input_file_location('devicetree') + + +def devicetree_file_location(opts) -> Optional[Path]: + # This mirrors the logic in 90-loaderentry.install. Keep in sync. + configfile = devicetree_config_location() + if configfile is None: + return None + + devicetree = configfile.read_text().strip() + if not devicetree: + raise ValueError(f'{configfile!r} is empty') + + path = input_file_location( + devicetree, + f'/usr/lib/firmware/{opts.kernel_version}/device-tree', + f'/usr/lib/linux-image-{opts.kernel_version}', + f'/usr/lib/modules/{opts.kernel_version}/dtb', + ) + if path is None: + raise FileNotFoundError(f'DeviceTree file {devicetree} not found') + return path + + def kernel_cmdline_base() -> list[str]: path = input_file_location('cmdline') if path: @@ -208,6 +233,9 @@ def call_ukify(opts): # Note that 'uki.efi' is the name required by 90-uki-copy.install. opts2.output = opts.staging_area / 'uki.efi' + if devicetree := devicetree_file_location(opts): + opts2.devicetree = devicetree + opts2.cmdline = kernel_cmdline(opts) if BOOT_STUB: opts2.stub = BOOT_STUB diff --git a/src/kernel-install/90-loaderentry.install.in b/src/kernel-install/90-loaderentry.install.in index 02497211b39..a52dd812e43 100755 --- a/src/kernel-install/90-loaderentry.install.in +++ b/src/kernel-install/90-loaderentry.install.in @@ -126,10 +126,12 @@ elif [ -f /usr/lib/kernel/devicetree ]; then fi if [ -n "$DEVICETREE" ]; then for prefix in \ + "$KERNEL_INSTALL_CONF_ROOT" \ "/usr/lib/firmware/$KERNEL_VERSION/device-tree" \ "/usr/lib/linux-image-$KERNEL_VERSION" \ "/usr/lib/modules/$KERNEL_VERSION/dtb" do + [ -n "$prefix" ] || continue [ -f "$prefix/$DEVICETREE" ] || continue DEVICETREE_SRC="$prefix/$DEVICETREE" break diff --git a/src/kernel-install/test-kernel-install.sh b/src/kernel-install/test-kernel-install.sh index ad9462c7fb4..30bee06b533 100755 --- a/src/kernel-install/test-kernel-install.sh +++ b/src/kernel-install/test-kernel-install.sh @@ -39,6 +39,11 @@ BOOT_ROOT="$D/badboot" MACHINE_ID=badbadbadbadbadbad6abadbadbadbad EOF +# Create a 'devicetree' config file that points to a fake dtb file +echo 'subdir/whatever.dtb' >"$D/sources/devicetree" +mkdir "$D/sources/subdir" +echo 'DTBDTBDTBDTB' >"$D/sources/subdir/whatever.dtb" + export KERNEL_INSTALL_CONF_ROOT="$D/sources" # We "install" multiple plugins, but control which ones will be active via install.conf. export KERNEL_INSTALL_PLUGINS="${ukify_install} ${loaderentry_install} ${uki_copy_install}" @@ -60,9 +65,11 @@ grep -qE '^version +1.1.1' "$entry" grep -qE '^options +opt1 opt2' "$entry" grep -qE '^linux .*/the-token/1.1.1/linux' "$entry" grep -qE '^initrd .*/the-token/1.1.1/initrd' "$entry" +grep -qE '^devicetree .*/the-token/1.1.1/whatever.dtb' "$entry" grep -qE 'image' "$BOOT_ROOT/the-token/1.1.1/linux" grep -qE 'initrd' "$BOOT_ROOT/the-token/1.1.1/initrd" +grep -qE 'DTBDTB' "$BOOT_ROOT/the-token/1.1.1/whatever.dtb" "$kernel_install" inspect "$kernel_install" inspect "$D/sources/linux" @@ -95,9 +102,11 @@ grep -qE '^version +1.1.2' "$entry" grep -qE '^options +opt1 opt2' "$entry" grep -qE '^linux .*/the-token/1.1.2/linux' "$entry" ( ! grep -qE '^initrd' "$entry" ) +grep -qE '^devicetree .*/the-token/1.1.2/whatever.dtb' "$entry" grep -qE 'image' "$BOOT_ROOT/the-token/1.1.2/linux" test ! -e "$BOOT_ROOT/the-token/1.1.2/initrd" +grep -qE 'DTBDTB' "$BOOT_ROOT/the-token/1.1.2/whatever.dtb" # Check installation with boot counting echo '56' >"$D/sources/tries" @@ -120,6 +129,7 @@ if [ -f "$ukify" ]; then layout=uki uki_generator=ukify EOF + "$kernel_install" -v add 1.1.3 "$D/sources/linux" "$D/sources/initrd" uki="${BOOT_ROOT}/EFI/Linux/the-token-1.1.3+56.efi" test -f "$uki" @@ -129,6 +139,13 @@ EOF "$bootctl" kernel-inspect "$uki" | grep -qE 'Version: +1\.1\.3$' "$bootctl" kernel-inspect "$uki" | grep -qE 'Cmdline: +opt1 opt2$' fi + + "$ukify" inspect "$uki" | grep -qE '^.sbat' + "$ukify" inspect "$uki" | grep -qE '^.cmdline' + "$ukify" inspect "$uki" | grep -qE '^.uname' + "$ukify" inspect "$uki" | grep -qE '^.initrd' + "$ukify" inspect "$uki" | grep -qE '^.linux' + "$ukify" inspect "$uki" | grep -qE '^.dtb' fi # Test bootctl