On Fri, 14 Apr 2023 10:55:22 +0800
Pingfan Liu <piliu(a)redhat.com> wrote:
Hi Philipp,
It is an interesting topic. And please see my inline comments.
On Tue, Apr 11, 2023 at 11:27 PM Philipp Rudo <prudo(a)redhat.com> wrote:
>
> A Unified Kernel Image (UKI) is a single EFI PE executable combining an
> EFI stub, a kernel image, an initrd image, and the kernel command line.
> They are defined in the Boot Loader Specification [1] as type #2
> entries. UKIs have the advantage that all code as well as meta data that
> is required to boot the system, not only the kernel image, is combined
> in a single PE file and can be signed for EFI SecureBoot. This extends
> the coverage of SecureBoot extensively.
>
> For RHEL support for UKI were included into kernel-ark with 16c7e3ee836e
> ("redhat: Add sub-RPM with a EFI unified kernel image for virtual
> machines").
>
> There are two problems with UKIs from the kdump point of view at the
> moment. First, they cannot be directly loaded via kexec_file_load and
> second, the initrd included isn't suitable for kdump. In order to enable
> kdump on systems with UKIs build the kdump initrd as usual and extract
> the kernel image before loading the crash kernel.
>
> [1]
https://uapi-group.org/specifications/specs/boot_loader_specification/
>
> Signed-off-by: Philipp Rudo <prudo(a)redhat.com>
> ---
> kdump-lib.sh | 21 ++++++++++++++++++++-
> kdumpctl | 12 +++++++++++-
> 2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index 44ea9a4..ddbb8ed 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -11,6 +11,11 @@ fi
> FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
> FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered"
>
> +is_uki()
> +{
> + [[ -f /boot/efi/EFI/Linux/vmlinuz-$1-virt.efi ]]
Could it be achieved by checking the sections' name in the binary? It
will be more robust since the naming of the section is part of UKI
specification.
The BLS contains a naming convention for the UKIs. I thought that
relying on the convention would be sufficient. Especially as it's much
more light weight compared to calling objdump and parsing for the
'right' sections same. But I don't have a strong opinion on it. So we
can also add a check for the section names.
BTW, just noticed that I forgot to add the binutils as dependency in
the .spec file...
> +}
> +
> is_fadump_capable()
> {
> # Check if firmware-assisted dump is enabled
> @@ -631,6 +636,11 @@ prepare_kdump_kernel()
> local dir img boot_dirlist boot_imglist kdump_kernel machine_id
> read -r machine_id < /etc/machine-id
>
> + if is_uki $kdump_kernelver; then
> + echo
"/boot/efi/EFI/Linux/vmlinuz-$kdump_kernelver-virt.efi"
> + return
> + fi
> +
> boot_dirlist=${KDUMP_BOOTDIR:-"/boot /boot/efi /efi /"}
> boot_imglist="$KDUMP_IMG-$kdump_kernelver$KDUMP_IMG_EXT
$machine_id/$kdump_kernelver/$KDUMP_IMG"
>
> @@ -705,7 +715,11 @@ prepare_kdump_bootinfo()
> fi
>
> # Set KDUMP_BOOTDIR to where kernel image is stored
> - KDUMP_BOOTDIR=$(dirname "$KDUMP_KERNEL")
> + if is_uki $KDUMP_KERNELVER; then
> + KDUMP_BOOTDIR=/boot
> + else
> + KDUMP_BOOTDIR=$(dirname "$KDUMP_KERNEL")
> + fi
>
> # Default initrd should just stay aside of kernel image, try to find it in
KDUMP_BOOTDIR
> boot_initrdlist="initramfs-$KDUMP_KERNELVER.img initrd"
> @@ -858,6 +872,11 @@ prepare_cmdline()
> # may cause the hot-remove of some pci hotplug device.
> is_aws_aarch64 && out=$(echo "$out" | sed -e
"/\<irqpoll\>//")
>
> + # Always disable gpt-auto-generator as it hangs during boot of the
> + # crash kernel. Furthermore we know which disk will be used for dumping
> + # (if at all) and add it explicitly.
> + is_uki $KDUMP_KERNELVER && out+="rd.systemd.gpt_auto=no "
> +
> # Trim unnecessary whitespaces
> echo "$out" | sed -e "s/^ *//g" -e "s/ *$//g"
-e "s/ \+/ /g"
> }
> diff --git a/kdumpctl b/kdumpctl
> index 9d02275..61b2b08 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -643,7 +643,7 @@ function remove_kdump_kernel_key()
> # as the currently running kernel.
> load_kdump()
> {
> - local ret
> + local ret uki
>
> KEXEC_ARGS=$(prepare_kexec_args "${KEXEC_ARGS}")
> KDUMP_COMMANDLINE=$(prepare_cmdline "${KDUMP_COMMANDLINE}"
"${KDUMP_COMMANDLINE_REMOVE}" "${KDUMP_COMMANDLINE_APPEND}")
> @@ -656,6 +656,16 @@ load_kdump()
> load_kdump_kernel_key
> fi
>
> + if is_uki $KDUMP_KERNELVER; then
> + uki=$KDUMP_KERNEL
> + KDUMP_KERNEL=$KDUMP_TMPDIR/vmlinuz
> + objcopy -O binary --only-section .linux "$uki"
"$KDUMP_KERNEL"
Out of topic, is the .linux signed? If not, we need to demand it in
the kernel.spec.
AFAIK the .linux isn't required to be signed by the standard because
the whole UKI can be/is signed. For RHEL however the .linux is signed
as the UKI is created after a normal kernel build with the signed
bzImage. And that's good. Because otherwise we couldn't load it via
kexec_file_load on a locked down kernel.
Thanks
Philipp
Thanks,
Pingfan
> + sync -f "$KDUMP_KERNEL"
> + # Make sure the temp file has the correct SELinux label.
> + # Otherwise starting the kdump.service will fail.
> + chcon -t boot_t $KDUMP_KERNEL
> + fi
> +
> ddebug "$KEXEC $KEXEC_ARGS $standard_kexec_args
--command-line=$KDUMP_COMMANDLINE --initrd=$TARGET_INITRD $KDUMP_KERNEL"
>
> # The '12' represents an intermediate temporary file descriptor
> --
> 2.39.2
> _______________________________________________
> kexec mailing list -- kexec(a)lists.fedoraproject.org
> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org
> Do not reply to spam, report it:
https://pagure.io/fedora-infrastructure/new_issue