Efi arm64 loader fixes #209
Conversation
|
If it is deemed desirable to keep doing our own PE binary loading + jumping to the entry point then I suggest using the new upstream mechanism to hook the load- and start-image boot services calls, which is already (partly) backported on the fedora-44 branch, combined with this patch: This provides a much cleaner (and easier to rebase) to do our own PE binary loading + starting vs the current broken code. For now this can be used on just aarch64, but on the next rebase (once 2.14) is out I would suggest to also switch to this code for i386/x86_64 which would greatly reduce the delta vs upstream and if both Ubuntu and Fedora are using this it would seem reasonable to try and upstream this. Let me know if you want to go the route of using the mechanism to hook the load- and start-image boot services calls together with Ubuntu's peimage module patch which uses these hooks, then I'll update this pull-request to include these patches. |
|
I think you should go ahead and do this with the new upstream mechanism. |
|
Also, FYI, I think this branch is out of date compared to what's in Dist-Git: https://src.fedoraproject.org/rpms/grub2/blob/rawhide/f/grub.patches |
Signed-off-by: Leo Sandoval <lsandova@redhat.com>
Now this branch should be sync |
|
@jwrdegoede thanks for your patches. Let us revisit them ASAP. |
|
Thank you for fixing the fedora-44 branch being out of sync. @lsandov1 I'm not sure what you mean with "Let us revisit them ASAP"? Do you mean that you're going to take a look when you have some time ? Or do you want me to rebase, finish the backport of the load-image + start-image hooks and add the peimage patch from Ubuntu which adds a cleaner version of the code to manually load the PE binary and jump to its entry point (vs relying on the UEFI firmware to do this) ? Or maybe both? |
yes, I meant that.
At some point in the near future (once 2.14 is out), we plan to rebase ALL patches on top on it but this time, we want to use the upstream loaders instead of PE loading + jumping to the entry point. As I mentioned, I plan to do this work pretty soon so the delta between fedora and upstream reduces considerably on this area. Seems that the peimage approach is much better approach based on the plans I mentioned before. Let me discuss it with the team and see possible implications before going into that route.
|
Right, note that the reverts which are the main part of this PR basically switch back to using the upstream loaders (on arm64 only) and that is enough to fix the load a UKI with just the stub from GRUB (with grub providing the initrd + cmdline, so not chainloading) case working. The peimage patch from Ubuntu re-adds GRUB doing the PE loading + jumping instead of using the EFI firmware's load-image and start-image methods, but then in a cleaner way which is not hardcoded for the kernel EFI stub PE binary layout like the current Fedora code this PR reverts. I've tested with both fedora-44 + just the reverts as well as with upstream's main branch + Ubuntu's peimage patch. I've not tried the fedora-44 branch with Ubuntu's peimage patch yet, but if you decide that is preferred over just doing the reverts I expect it should not be too much work to get that backported. |
|
@jwrdegoede thanks for the full explanation Hans, I got the full context now. Let's go with this PR: we have the goal to revert/remove all fedora custom loader stuff and you already do it for arm64, so this basically aligns to what we have in mind and planned. Help me understand what you have done
Finally, create PR for grub2 dist-git so CI can test this change. |
Yes I've tested the default Fedora 43 kernel images with this change, so that is the default PE kernel image with only the kernel EFI-stub (vmlinuz.efi target). But this was on the same hw as where I've tested the UKI. I've not tested this on other arm64 platforms.
No, it actually gets us closer to doing secureboot in a safe way on arm64 since now the DTBs (which are security sensitive) will be part of a single EFI binary which will get signed with Fedora's signing key, rather then GRUB loading an unsigned DTB file and which DTB to load is based on pretty much immutable strings from the firmware. If someone can flash a custom firmware they could make us load another DTB, but if they can flash custom firmware they already have full access anyways. |
|
I've created a dist-git pull-req for CI as requested: https://src.fedoraproject.org/rpms/grub2/pull-request/200 |
This fully reverts commit ae0fba3. This will break RISC-V builds, but further reverts in this series will completely remove the troublesome parse_pe_header() function. Note the modifed code is not used on x86 (#ifdef !x86). This is part of a series of reverts to move non x86 vmlinuz loading back to using EFI boot services load_image + start_image calls. The current DYI approach breaks booting kernel EFI binaries which have the stubble stub for automatic DTB loading. Switching back to EFI boot services load_image + start_image fixes this and also brings Fedora's grub inline with how upstream grub is loading and booting kernels on arm64 platforms. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
…NX is required." This fully reverts commit b899582. Note this is actually a no-op since the modified linux_cmd() x86-only code is not used on x86 at all, instead x86 uses the linux_cmd() function from grub-core/loader/i386/efi/linux.c . This is part of a series of reverts to move non x86 vmlinuz loading back to using EFI boot services load_image + start_image calls. The current DYI approach breaks booting kernel EFI binaries which have the stubble stub for automatic DTB loading. Switching back to EFI boot services load_image + start_image fixes this and also brings Fedora's grub inline with how upstream grub is loading and booting kernels on arm64 platforms. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
This reverts a small part of commit be61be0. The only reverted bits are the changes to grub_cmd_linux() these changes call grub_efi_check_nx_image_support() and store the result into a local variable, which is then never used. So reverting these changes is a no-op as the removed code only reads a flag which is never used. Also note that this version of grub_cmd_linux() is never used on x86, which uses linux_cmd() from grub-core/loader/i386/efi/linux.c . This is part of a series of reverts to move non x86 vmlinuz loading back to using EFI boot services load_image + start_image calls. This partial revert is done to make further reverts in this series cleaner. The current DYI approach breaks booting kernel EFI binaries which have the stubble stub for automatic DTB loading. Switching back to EFI boot services load_image + start_image fixes this and also brings Fedora's grub inline with how upstream grub is loading and booting kernels on arm64 platforms. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
This fully reverts commit 9728e07. Note that the modifed version of grub_cmd_linux() is never used on x86, which uses linux_cmd() from grub-core/loader/i386/efi/linux.c . This is part of a series of reverts to move non x86 vmlinuz loading back to using EFI boot services load_image + start_image calls. The current DYI approach breaks booting kernel EFI binaries which have the stubble stub for automatic DTB loading. Switching back to EFI boot services load_image + start_image fixes this and also brings Fedora's grub inline with how upstream grub is loading and booting kernels on arm64 platforms. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
This fully reverts commit ba84a3e. Note the modifed code is not used on x86 (#ifdef !x86). This is part of a series of reverts to move non x86 vmlinuz loading back to using EFI boot services load_image + start_image calls. The current DYI approach breaks booting kernel EFI binaries which have the stubble stub for automatic DTB loading. Switching back to EFI boot services load_image + start_image fixes this and also brings Fedora's grub inline with how upstream grub is loading and booting kernels on arm64 platforms. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
This fully reverts commit 81dbbbc. Note the modifed code is not used on x86 (#ifdef !x86). This is part of a series of reverts to move non x86 vmlinuz loading back to using EFI boot services load_image + start_image calls. The current DYI approach breaks booting kernel EFI binaries which have the stubble stub for automatic DTB loading. Switching back to EFI boot services load_image + start_image fixes this and also brings Fedora's grub inline with how upstream grub is loading and booting kernels on arm64 platforms. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
This fully reverts commit 3cfd4a1. Note the modifed code is not used on x86 (#ifdef !x86). This is part of a series of reverts to move non x86 vmlinuz loading back to using EFI boot services load_image + start_image calls. The current DYI approach breaks booting kernel EFI binaries which have the stubble stub for automatic DTB loading. Switching back to EFI boot services load_image + start_image fixes this and also brings Fedora's grub inline with how upstream grub is loading and booting kernels on arm64 platforms. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
This fully reverts commit fa8f199. Note that the modifed version of grub_cmd_linux() is never used on x86, which uses linux_cmd() from grub-core/loader/i386/efi/linux.c . This concludes the series of reverts to move non x86 vmlinuz loading back to using EFI boot services load_image + start_image calls. The current DYI approach breaks booting kernel EFI binaries which have the stubble stub for automatic DTB loading. Switching back to EFI boot services load_image + start_image fixes this and also brings Fedora's grub inline with how upstream grub is loading and booting kernels on arm64 platforms. This also fixes 2 issues with the xen_hypervisor and xen_module commands: 1) With the now reverted move of setting loaded_image->load_options in finalize_params() the commandline would no longer be set for the xen case. 2) The handover_offset variable passed by grub_efi_linux_boot is a global variable set by the linux cmd and this would either be 0 or the value from the last run linux cmd, neither of which is likely correct for the xen case. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Fix grub_arch_efi_linux_boot_image () leaking the memory allocated for mempath by freeing it after use. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Fix grub_arch_efi_linux_boot_image () not unloading the loaded image when allocating memory for the commandline fails. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
When building grub for x86_64-efi or i386-efi then loader/i386/efi/linux.c already registers "linux" and "initrd" commands and those will be used instead of those defined in the generic loader/efi/linux.c code. Disable (#ifdef out) the unused code in loader/efi/linux.c when building for x86. This also avoids registering the linux and initrd commands twice. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
The latest gcc does not like the initializer used for the
grub_net_link_level_address_t hw_addr variable:
../../grub-core/net/drivers/ieee1275/ofnet.c: In function ‘grub_ieee1275_parse_bootpath’:
../../grub-core/net/drivers/ieee1275/ofnet.c:158:3: warning: braces around scalar initializer
158 | grub_net_link_level_address_t hw_addr = {0, {{0, 0, 0, 0, 0, 0}}};
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../grub-core/net/drivers/ieee1275/ofnet.c:158:3: note: (near initialization for ‘hw_addr.len’)
../../grub-core/net/drivers/ieee1275/ofnet.c:158:3: error: braces around scalar initializer
../../grub-core/net/drivers/ieee1275/ofnet.c:158:3: note: (near initialization for ‘hw_addr.len’)
../../grub-core/net/drivers/ieee1275/ofnet.c:158:52: warning: excess elements in scalar initializer
158 | grub_net_link_level_address_t hw_addr = {0, {{0, 0, 0, 0, 0, 0}}};
...
Since we're zeroing it anyways switch to a simpler initializer
to fix this.
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
jwrdegoede
force-pushed
the
efi-arm64-loader-fixes
branch
from
1dfef7b to
0e1e28d
Compare
|
Just pushed a new version, changes:
|
3 participants
The Fedora 44 change to make aarch64 live isos just work on Windows on Arm laptops:
https://fedoraproject.org/wiki/Changes/Automatic_DTB_selection_for_aarch64_EFI_systems
This uses the auto DTB selection build into the systemd-stub. The modified kernel image is in essence an incomplete UKI containing just a UKI boot-stub and the DTBs, but not an initrd nor a kernel commandline. The host generated initrd and host specific commandline will still be supplied by GRUB as usual, including allowing the user to make changes to the commandline through GRUB.
Currently the rhboot/fedora-44 branch from which Fedora 44's GRUB is build fails to boot the UKI-light kernel images used for this changed.
This is caused by a set of Fedora downstream GRUB changes which replace calling EFI boot services load_image + start_image with code to manually load the PE binary and jump to the entry point.
This pull-requests consists of 8 (partial) reverts, reverting to the grub mainline code for starting EFI PE binaries on aarch64. Note the code changes only influence aarch64, which is why some of the reverts are partial since some of the code is also used on i386/x86_64 builds.
I've put a notice in the commit message which commits are full reverts so that the origin commit + the revert can simply be dropped on the next rebase.
On top of this reverts there are 2 bugfixes and a commit disabling most of the generic grub-core/loader/efi/linux.c code on i386/x86_64 since that is unused on i386/x86_64.