Prepare to support UKIs - #523
Conversation
Provide the bootloader(efi) RPM capability so newer versions of twoliter and the core kit, which support UKI, can depend on it to select GRUB as the EFI bootloader. Conflict with the uki-image feature, since UKI images require systemd-boot instead of GRUB. Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>
Split shim into two subpackages: one that chain-loads GRUB (installed at the current location, for backwards compatibility) and a new one that chain-loads systemd-boot instead. Older versions of twoliter don't understand the 'uki-image' feature, so the GRUB subpackage's requirements are left unrestricted to remain installable there. To prevent it from being pulled in on UKI images with newer twoliter versions, it instead conflicts with the uki-image feature; the systemd-boot subpackage conflicts with no-uki-image accordingly. Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>
The patch prevented systemd-boot from measuring PCR9 Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>
The patch prevented systemd-boot from measuring PCR9 Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>
The patch prevented systemd-boot from measuring PCR9 Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>
e0e2d4f to
7a7e783
Compare
| # Avoid explicit image-feature(no-uki-image) requires for backwards compatibility | ||
| # The conflict is enough for now to prevent installing GRUB when UKIs are used |
There was a problem hiding this comment.
| # Avoid explicit image-feature(no-uki-image) requires for backwards compatibility | |
| # The conflict is enough for now to prevent installing GRUB when UKIs are used |
There was a problem hiding this comment.
I actually wrote this one comment. Usually, we do the strong Requires and Conflicts, but in this case I avoided the Requires for a reason - don't require downstreams to bump their twoliter version if they take this kernel kit.
| %global systemd_boot_efi_image systemd-boot%{_cross_efi_arch}.efi | ||
| %global shim_systemd_boot_efi_image shim-systemd-boot%{_cross_efi_arch}.efi |
There was a problem hiding this comment.
looks like you could interpolate these variables e.g.
%global shim_systemd_boot_efi_image shim-%{systemd_boot_efi_image}
| # Install grub-chaining shim at the default boot path, for backwards | ||
| # compatibility with image builds that expect to find it there. | ||
| find %{_builddir}/build-grub -name '%{shim_efi_image}' -exec \ | ||
| cp {} "%{buildroot}%{efidir}/%{boot_efi_image}" \; |
There was a problem hiding this comment.
any reason these all changed from mv to cp? And no longer rm -rf %{buildroot}%{_datadir} ?
There was a problem hiding this comment.
rm -f is no longer needed because we don't do the direct install anymore. re cp mv, nothing in particular, I can revert back if you feel strongly about this.
| Provides: %{name}(shim-efi) = 1: | ||
| Conflicts: %{_cross_os}image-feature(uki-image) | ||
|
|
||
| %description grub | ||
| %{summary}. | ||
|
|
||
| %package systemd-boot | ||
| Summary: Shim built to chain-load systemd-boot | ||
| Requires: %{name} | ||
| Requires: %{_cross_os}image-feature(uki-image) | ||
| Provides: %{name}(shim-efi) = 0: |
There was a problem hiding this comment.
[P1] Epoch-only Provides: %{name}(shim-efi) = N: is non-idiomatic and not load-bearing
Both subpackages declare a versioned virtual provide with only an Epoch and an empty version string:
shim-grub:Provides: %{name}(shim-efi) = 1:(line 36)shim-systemd-boot:Provides: %{name}(shim-efi) = 0:(line 46)
The base package has an unversioned Requires: %{name}(shim-efi) (line 26).
The normal RPM syntax is NAME = [EPOCH:]VERSION[-RELEASE]. = 1: with an empty version is parseable by rpm (Epoch=1, Version="") but:
- The unversioned
Requiresin the base does not consider the Epoch at all — either subpackage satisfies it. So the Epoch never participates in dependency resolution for this Requires. - Subpackage selection is actually driven by the
Conflicts/Requires: image-feature(...)rules, not by the Epoch. The epoch-only versioning implies a tie-break mechanism that isn't actually load-bearing. - If in the future both subpackages become eligible under some variant, the well-formedness of
= 1:vs= 0:becomes depsolver-dependent.
Suggested fix: either drop the versioned Provides entirely and rely on the image-feature Conflicts/Requires (which are what really determine selection today), or make it well-formed and document the intent, e.g.:
Provides: %{name}(shim-efi) = 1:%{version}-%{release}
...
Provides: %{name}(shim-efi) = 0:%{version}-%{release}with a comment explaining it is a tie-breaker for the shim-grub default.
Confidence: ~70%.
| find %{_builddir}/build-grub -name '%{mokm_efi_image}' -exec \ | ||
| cp {} "%{buildroot}%{efidir}/%{mokm_efi_image}" \; |
There was a problem hiding this comment.
[P2] MokManager is copied only from build-grub but shipped in the base package
%install copies mm*.efi exclusively out of the build-grub tree, and the resulting file is packaged in the base %files list (line 104), which is installed alongside both subpackages.
Concerns:
- Asymmetric build coupling. MokManager doesn't depend on
DEFAULT_LOADER, so both build trees produce it. Sourcing it only frombuild-grubcouples the base package to that specific build. If a follow-up PR conditionally disables the grub build for UKI-only variants (e.g., wraps lines 77, 80-81 in%if !%{uki_only}), the base%fileslist still references%{efidir}/%{mokm_efi_image}and the build breaks non-obviously. find … -exec cpdoesn't signal 'no match'.findreturns 0 whether or not it matched. If the grub build silently stops producingmm*.efiin the future, the failure surfaces later at%files("File not found") rather than at the copy step.
Suggested fix: either copy mm*.efi from build-systemd-boot (identical bits, decoupled from the grub build) or add a brief comment stating that MokManager is loader-independent and either build tree is acceptable, e.g.:
# MokManager is not affected by DEFAULT_LOADER; either build tree is fine.
find %{_builddir}/build-grub -name '%{mokm_efi_image}' -exec \
cp {} "%{buildroot}%{efidir}/%{mokm_efi_image}" \;Confidence: ~60%.
| %package grub | ||
| Summary: Shim built to chain-load GRUB | ||
| # Avoid explicit image-feature(no-uki-image) requires for backwards compatibility | ||
| # The conflict is enough for now to prevent installing GRUB when UKIs are used | ||
| Requires: %{name} | ||
| Provides: %{name}(shim-efi) = 1: | ||
| Conflicts: %{_cross_os}image-feature(uki-image) | ||
|
|
||
| %description grub | ||
| %{summary}. | ||
|
|
||
| %package systemd-boot | ||
| Summary: Shim built to chain-load systemd-boot | ||
| Requires: %{name} | ||
| Requires: %{_cross_os}image-feature(uki-image) | ||
| Provides: %{name}(shim-efi) = 0: | ||
| Conflicts: %{_cross_os}image-feature(no-uki-image) | ||
|
|
||
| %description systemd-boot | ||
| %{summary}. |
There was a problem hiding this comment.
[P2] shim-grub and shim-systemd-boot have no explicit mutual Conflicts
The two subpackages are designed to be mutually exclusive, but nothing in the spec directly prevents co-installation. Mutual exclusion is only enforced transitively through the image-feature(...) Requires/Conflicts:
shim-grubConflictsimage-feature(uki-image)(line 37)shim-systemd-bootRequiresimage-feature(uki-image)(line 45) and Conflictsimage-feature(no-uki-image)(line 47)
The two subpackages install into disjoint paths (bootXX.efi vs shim-systemd-bootXX.efi), so RPM sees no file conflict either.
Scenario: a variant recipe that lists both Requires: shim-grub and Requires: shim-systemd-boot (dual-boot image or copy/paste error) will resolve without any RPM-level signal. The resulting image ships both loaders and boot behavior is then only determined by whatever bootXX.efi the firmware/UKI configuration points at.
If the intent really is "exactly one", encoding it in the spec is stronger than delegating it to image-feature discipline in downstream recipes.
Suggested fix: add mutual Conflicts to make the invariant explicit:
%package grub
...
Conflicts: %{name}-systemd-boot
Conflicts: %{_cross_os}image-feature(uki-image)
%package systemd-boot
...
Conflicts: %{name}-grub
Conflicts: %{_cross_os}image-feature(no-uki-image)Alternatively, document in a comment that mutual exclusion is intentionally delegated to image-feature declarations upstream of this spec.
Confidence: ~55%.
Description of changes:
This series prepares the kernel kit to support Unified Kernel Images.
The first commit in the series prevents
grubfrom being used with a variant that opt-in to use the new UKI format, and makes it provide the%{_cross_os}bootloader(efi)capability.The second commit in the series prepares an extra copy of the
shimthat looks forsystemd-bootinstead of GRUB, and re-structures the spec to resolve either subpackage depending on the features enabled for the variant.The remaining commits in the series drop a patch to don't measure PCR9 in the kernel.
systemd-boottriggering PCR9 measurements in the kernel, but this patch prevented the kernel from actually updating the PCR. The side-effect is that in GRUB-based AMIs, PCR9 measurements will be as follows:Testing done:
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.