Conversation
|
Tagging @xypron, @brianredbeard and @jmontleon since some of the changes come from them. |
andreabolognani
force-pushed
the
riscv64
branch
from
eae94d7 to
3482501
Compare
|
Rebased on top of #777. Additionally, I have dropped all definitions related to @xypron perhaps you could revisit your patch, integrating the follow-up fixes and improvements from me, @brianredbeard and @jmontleon? And of course any feedback from @vathpela would be much appreciated too :) Thanks in advance! |
andreabolognani
force-pushed
the
riscv64
branch
from
3482501 to
54f16b5
Compare
|
Rebased to match the current contents of the |
andreabolognani
force-pushed
the
riscv64
branch
from
de1f86c to
224883c
Compare
|
Rebased once again on top of the updated #777. Squashed some patches (with agreement from the authors) and fixed VPATH builds. |
Replace the shim-specific fork with the upstream version, specifically the most recent release. Some adjustment to shim's code are necessary to adapt to this change. Signed-off-by: Andrea Bolognani <abologna@redhat.com>
shim is a standalone EFI application so it shouldn't be necessary to look at the glibc headers when building it, and in fact attempting to do so results in a build failure. Signed-off-by: Andrea Bolognani <abologna@redhat.com>
We could theoretically set GNU_EFI_USE_REALLOCATEPOOL_ABI=0 to keep using the legacy ABI, but since gnu-efi uses the modern ABI internally and we call into its build systemd directly, doing that messes things up. Switching to the new ABI is just a matter of changing the order of arguments. Signed-off-by: Andrea Bolognani <abologna@redhat.com>
We could theoretically set GNU_EFI_USE_COMPARE_ABI=0 to keep using the legacy ABI, but since gnu-efi uses the modern ABI internally and we call into its build systemd directly, doing that messes things up. In a very small handful of cases we actually rely on the behavior of the old ABI because we don't just need to know whether or not the two GUIDs are identical, but also their relative sorting order. CompareGuidForSorting(), which retains the old behavior, is introduced to deal with those scenarios. Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Jason Montleon <jmontleo@redhat.com>
Signed-off-by: Callum Farmer <gmbr3@opensuse.org>
Signed-off-by: Callum Farmer <gmbr3@opensuse.org>
Signed-off-by: Callum Farmer <gmbr3@opensuse.org>
Signed-off-by: Callum Farmer <gmbr3@opensuse.org>
Signed-off-by: Callum Farmer <gmbr3@opensuse.org>
andreabolognani
force-pushed
the
riscv64
branch
from
224883c to
ec264d3
Compare
|
Rebased on top of #777 and added CI jobs for riscv64 cross-builds. |
* De-duplicate uses of .note.gnu.build-id/.eh_frame * Push .reloc after .data (see ncroxon/gnu-efi@03bfe2f) * ARM updates: * use new .text placement (0x1000) fixes allocation issue (ncroxon/gnu-efi@24a4cd0) * add needed symbols from gnu-efi * Add missed reloc section (ncroxon/gnu-efi@eadee98) Signed-off-by: Callum Farmer <gmbr3@opensuse.org>
Add what is needed to build on riscv64. Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
@davidlt and @xypron pointed out prior changed to binutils 2.42 which added support for RISC-V EFI objects. This reflects the upstream preference to avoid adding additional architectures which are emitting flat binary files via `objcopy` (i.e. `-O binary` architectures). Signed-off-by: Brian 'redbeard' Harrington <redbeard@dead-city.org>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
They don't seem to be necessary when building against the latest upstream version of gnu-efi. Signed-off-by: Andrea Bolognani <abologna@redhat.com>
|
Rebased on top of #777 once again. |
|
@xypron can you please consider giving this some sprucing up? The CI now passes and #777 looks close to being ready. You can feel free to squash any changes of my own into your commit, and I'm pretty confident neither @brianredbeard nor @jmontleon would mind if you did the same with theirs. Thanks! |
|
I've got one thing for you to cherry-pick: brianredbeard@29593a0 The TL;DR is that Not having the cert verification or revocation would probably defeat the point of what we're working on 😆 Like my previous attempt, I'm happy to play janitor and do the clean-up step as we go. |
|
I've worked through #777 and this one (#778) I've done a review, found and fixed several issues, and rebased the combined work into a cleaner commit series. Issues fixed in the riscv64 linker script (elf_riscv64_efi.lds):
Commit structure (4 commits, down from 19):
All original authors are preserved via --author, Co-Authored-By, and Signed-off-by. The final tree is identical to the current HEAD of #778 except for the linker script fixes and a minor whitespace fix in peimage.h. The same whitespace nit effects the ARM entry (a tab instead of spaces as per all other entries). I can take care of fixing that too if folks please. Branch is at https://github.com/brianredbeard/redhat-efi-boot-shim/tree/riscv-squash — happy to open a PR superseding both #777 and #778 if that works for everyone. One note, the current tag for the submodule is master and not 4.0.4 (or the proper tag/branch). I just want to confirm that's correct. |
The SBAT-related definitions are missing from upstream gnu-efi, so those are retained. The rest of the file is completely identical to the upstream counterpart. Signed-off-by: Jason Montleon <jmontleo@redhat.com>
The riscv64 linker script was derived from gnu-efi 4.0.4 and carried
over several layout decisions that conflict with how shim's PE/COFF
post-processing and runtime expect sections to be arranged. This
brings it in line with the x86_64, ia32, and aarch64 linker scripts.
Add missing output sections that shim relies on:
- .vendor_cert: holds vendor certificates emitted by cert.S. Without
this section the linker treats it as an orphan and places it
unpredictably, breaking vendor certificate verification at runtime.
- .sbatlevel: holds the SBAT variable payload (sbat_var_payload_header)
emitted by sbat_var.S. Without it the symbol lands at an arbitrary
address and SBAT automatic/latest variable parsing reads garbage.
- .data.ident: holds shim version/build identification emitted by
version.c. All other 64-bit architectures place this in its own
section so it can be inspected in the PE binary.
Merge .rodata into the .data output section. On x86_64, ia32, and
aarch64, .rodata* is placed inside .data. Having .rodata as a
standalone output section creates a separate PE/COFF section that
post-process-pe may not handle correctly.
Move .note.gnu.build-id before .data (matching x86_64/ia32/aarch64)
instead of after .dynstr where it was appended without alignment.
Place .reloc, .vendor_cert, and .dynamic between .data and .rela,
matching the section ordering used by every other architecture.
Fix _edata alignment from ALIGN(512) to implicit section alignment.
512 is the PE file alignment, not the section alignment, and every
other architecture relies on the preceding section's page alignment
rather than inserting a sub-page alignment here.
Remove manual '_DYNAMIC = .' definition before the .dynamic section.
No other architecture defines this symbol manually; the linker
provides it automatically from the .dynamic section.
Remove KEEP() around .reloc contents. No other architecture uses
KEEP for this section, and shim does not link with --gc-sections,
making it a no-op.
Fix inconsistent indentation on ALIGN(16) directives within .data.
Signed-off-by: Brian Redbeard <redbeard@dead-city.org>
Signed-off-by: Jason Montleon <jmontleo@redhat.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
|
@brianredbeard thanks a lot for taking a look! I've checked out your squash branch and I have to say that I'm personally not a big fan of lumping so many changes together into a small number of large commits: I much prefer having a larger number of smaller commits. The exception to the above is when you have a commit that introduces a change, only for the next commit to (partially) undo it. That's very much the case here, mostly on account of @xypron providing the original implementation and several other people adding fixes and improvements on top. In this specific scenario, I would welcome squashing some of the commits together to avoid unnecessary churn. But then, the original implementation could itself potentially use some splitting into smaller incremental steps... Anyway, I've cherry-picked your additional changes for now. Ultimately it's up to the maintainers whether they would rather have many small commits or few large ones, and I'd be happy to go in either direction once they will have made their preference known. |
|
#777 is already organized pretty much optimally as far as I'm concerned, and there are no functional changes that you're proposing for that part, so I just left it alone. The whitespace changes you mentioned and any other cleanups should IMO be made as a follow-up, to avoid potentially holding up the functional parts due to disagreements on non-functional concerns. |
5 participants
This is part of an attempt to bring riscv64 support into shim.
See #420 and #641 for previous discussion, as well as #777 which is a prerequisite of this PR.
Using this branch I was able to successfully build shim on both x86_64 and riscv64, and I was able to use the resulting binaries to boot Fedora 42 cloud images for both architectures, replacing the stock ones.