Skip to content

Conversation

brianredbeard
Copy link

Add what is needed to build on riscv64.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

This is an update which supersedes #420 which brings it in alignment with the current upstream and fixes a minor style nit around the definition of __riscv.

Add what is needed to build on riscv64.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

This is an update to rhboot#420 which brings it in alignment with the current
upstream.

Signed-off-by: Brian 'redbeard' Harrington <redbeard@dead-city.org>
@brianredbeard
Copy link
Author

@vathpela can we get a review?

@brianredbeard
Copy link
Author

FYI, reverted the previous commit after a discussion with @xypron. Heinrich helpfully pointed out that the structure of pre-processor definitions in the form of __riscv is (tragically) required due to definitions in GCC.

@davidlt
Copy link

davidlt commented Mar 25, 2024

FYI gnu-efi ( https://github.com/rhboot/gnu-efi/tree/master ) has been rebased to 3.0.18 (released 3 days ago!). The builds are already available for F40 and F41 in Fedora too:
F40: https://koji.fedoraproject.org/koji/buildinfo?buildID=2426104
F41: https://koji.fedoraproject.org/koji/buildinfo?buildID=2426088

Make.defaults Outdated
ARCH_GNUEFI ?= riscv64
ARCH_SUFFIX ?= riscv64
ARCH_SUFFIX_UPPER ?= RISCV64
FORMAT := -O binary
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had some discussions with Peter Jones last year. There was a strong preference not to add more "-O binary" architectures. Two patches landed in binutils 2.42:
[PATCH] Add basic support for RISC-V 64-bit EFI objects
[PATCH] Handle "efi-app-riscv64" and similar targets in objcopy. // From Peter Jones

That means we have efi-app-riscv64 support. That probably means it should look more like aarch64. I haven't looked deeper or tested it.

Copy link
Contributor

@xypron xypron Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidlt Shim has an .sbat section with the contents of a CSV file for defining the security patch level. How would you create that section with binutils' efi-app-riscv64 target?

The content of the .sbat section is something like:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,4,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.ubuntu,1,Ubuntu,shim,15.8-0ubuntu1,https://www.ubuntu.com/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It really seems that the lines

+     FORMAT                  := -O binary
+     SUBSYSTEM               := 0xa
+     ARCH_LDFLAGS            += --defsym=EFI_SUBSYSTEM=$(SUBSYSTEM)
+     TIMESTAMP_LOCATION      := 72

are not needed. SUBSYSTEM defaults to 0x0a in branch main of https://github.com/rhboot/gnu-efi.git:

gnu-efi/gnuefi/crt0-efi-riscv64.S:20:
#define EFI_SUBSYSTEM 10

TIMESTAMP_LOCATION is unused.

FORMAT is a flag passed to riscv64-linux-gnu-objcopy which seems unneded.

@brianredbeard Could you please drop the lines from your merge request.

@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).
@davidlt
Copy link

davidlt commented Mar 27, 2024

The last commit is missing Signed-off-by: based on the review. How sure how important that is.


#if defined(__riscv) && __riscv_xlen == 64
#ifndef DEFAULT_LOADER
#define DEFAULT_LOADER L"\\grubriscv64.efi"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the file name be shorter (as in 8.3 format) given a FAT filesystem?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • FAT supports long file names since Windows 95.
  • The default name for EFI binaries on RISC-V is /EFI/BOOT/BOOTRISCV64.EFI. That is not 8.3 either.
  • Distros like Fedora and Ubuntu are already using grubriscv64.efi as file name (cf. https://fedoraproject.org/wiki/Architectures/RISC-V/GRUB2)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, cool. I was just thinking I've seen some vendors expect/model on it being strict FAT12 in the x86 space, and seeing the longer file name started to bug me. Also interesting Ubuntu beat Debian since I checked Debian packaging yesterday when the question came up in another project of "what would the filename be?!" so we could correctly account for it.

Copy link

@gmbr3 gmbr3 Apr 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UEFI requires support for FAT long filenames (LFN) in all FAT12, FAT16 and FAT32 so it's fine

@andreabolognani
Copy link

Hey, I'm wondering what's missing to get this one merged at this point? From a cursory look, it seems that all points raised during review have been addressed.

I'm really keen on seeing this make its way to Fedora. Along with the lack of an up-to-date GRUB2 (which is something that's supposedly being addressed as we speak), not having shim is, at least as far as I'm aware, the last remaining blocker to producing usable (albeit still unofficial) Fedora images for RISC-V.

@jmontleon
Copy link

It is not very clean, but I was able to build and boot shim on a VisionFive 2 and EDK2 on a qemu/libvirt VM. I forked shim and gnu-efi with some rough work

I had to pull a few things together to get builds working on riscv64.

You can basically clone https://github.com/jmontleon/shim/tree/riscv and
cd shim && make clean && make && sudo cp shimriscv64.efi /boot/efi/EFI/BOOT/BOOTRISCV64.EFI && sudo cp fbriscv64.efi /boot/efi/EFI/BOOT/ && sudo cp shimriscv64.efi /boot/efi/EFI/fedora/

With /boot/efi/EFI/fedora/BOOTRISCV64.csv containingshimriscv64.efi,Fedora,,This is the boot entry for Fedora the system boots and you get a boot entry created.

@julian-klode
Copy link
Collaborator

Hey, I'm wondering what's missing to get this one merged at this point? From a cursory look, it seems that all points raised during review have been addressed.

I'm really keen on seeing this make its way to Fedora. Along with the lack of an up-to-date GRUB2 (which is something that's supposedly being addressed as we speak), not having shim is, at least as far as I'm aware, the last remaining blocker to producing usable (albeit still unofficial) Fedora images for RISC-V.

I don't think any distro has intentions to sign anything for RISC-V yet and I don't know if Microsoft would even accept it, or what the value would be in handing Microsoft the control over the RISC-V CA too.

And unsigned you can just use grub directly and be happy, so I think there's no value merging changes like this and adding the maintenance overhead or have it potentially break each release because nobody tests it.

@xypron
Copy link
Contributor

xypron commented Jun 24, 2024 via email

@andreabolognani
Copy link

And unsigned you can just use grub directly and be happy, so I think there's no value merging changes like this and adding the maintenance overhead or have it potentially break each release because nobody tests it.

AFAIK shim doesn't just handle the Secure Boot part though, it also takes care of locating and running the GRUB binary when installed as the fallback entry (${ESP}/BOOT/BOOTRISCV64.EFI). At least that's how things seem to work in Fedora, though admittedly the Debian/Ubuntu boot path appears to be set up differently.

Anyway, that's the part I'm interested in, as it would allow us to ship disk images that can be booted right away, without the need on the user's part to create UEFI boot entries themselves,

For this to work there's no need for the shim binary to be signed, much less to determine who the signing entity would be.

@jmontleon
Copy link

ncroxon/gnu-efi#34 was merged.

Using that work and this PR I had to make a few changes to get things working properly.
main...jmontleon:shim:riscv

I also ended up having to reimplement the closed PR #410. I did see the note re: patching gnu-efi instead of shim, which would basically be undoing the CHAR8 commit (ncroxon/gnu-efi@ce1ec9d) if that approach is desired.

@gmbr3
Copy link

gmbr3 commented Jul 9, 2024

undoing the CHAR8 commit (ncroxon/gnu-efi@ce1ec9d) if that approach is desired.

I doubt it. SHIM and SHIM's gnu-efi is outdated and in a very inconsistent state. CHAR8 should be 'unsigned char' since that's how UEFI spec says it is.

@vathpela
Copy link
Member

On the face, the code this patch adds looks mostly okay to me, but there are still several problems here regarding the result. Even after following jmontleon's suggestions above, there are issues here, ranging from a minor problem with the PR itself to things that make this difficult to consider merging:

  • missing DCO
  • This doesn't add RISC-V to CI, and AFAICT for the reasons below, and it wouldn't pass if it did
  • CHAR8 and char don't match in signedness. I don't have a dog in the fight of which signedness CHAR8 should be on this platform (or really any others), but I don't see how they can be fully independent, and they haven't been in the past, so there's a fair amount of code depending on it. If there's a compelling reason for them not to be the same, we still need patches to fix all the errors that causes, which right now seem to be a lot.
  • This tree doesn't build with the newer upstream gnu-efi because of the API/ABI issues with realloc() and others. That looks easy to fix with a #define, but again, this won't pass CI until that's fixed.
  • the need to add va_start() (etc) macros looks wrong to me, and the comment that gnu-efi needs them is quite disturbing. gnu-efi is providing these functions (or at least their decls; it should always wind up using compiler builtins). That makes me suspect there's still an issue in the upstream gnu-efi variadics, which will bite us on x86. I haven't tried getting rid of this and seeing if it's really necessary, because this tree doesn't build.
  • in general I'm pretty hesitant to switch to newer upstream gnu-efi because that repo is an absolute mess of unnecessary merge commits and nearly empty changelogs, and stuff like the variadic function addition here makes me think we're going to get stuck holding the bag with printf() doing memory corruption again, which isn't acceptable.

@jmontleon
Copy link

jmontleon commented Nov 23, 2024

@vathpela It has been awhile since I looked but I remain able to build off the latest upstream gnu-efi with the additional changes I laid out in https://github.com/jmontleon/shim
At the time I used gnu-efi 33727c2abe8a77dca612f04abd19e10cc5e487fd

But updating gnu-efi to 5ea320f0f01c8de8f9dd4e4e38a245608f0287dd still builds:

git clone git@github.com:jmontleon/shim
cd shim/
git submodule update --init
make

I did a scratch a riscv64 scratch build (http://fedora.riscv.rocks/koji/taskinfo?taskID=1837632) and tested in a virtual machine quick. It seems to still work ok.

I did see an issue with va_list on x86_64 and added a workaround patch to the srpm there so it should build on x86_64 now as well.

I'm definitely not insisting this or all the rest is 100% correct and ready to merge, but it should at least be possible to build and perhaps we can start working down the list of other issues you raised above?

@gmbr3
Copy link

gmbr3 commented Dec 4, 2024

  • CHAR8 and char don't match in signedness. I don't have a dog in the fight of which signedness CHAR8 should be on this platform (or really any others), but I don't see how they can be fully independent, and they haven't been in the past, so there's a fair amount of code depending on it. If there's a compelling reason for them not to be the same, we still need patches to fix all the errors that causes, which right now seem to be a lot.

UEFI spec says CHAR8 is Latin-1 (0-255) so it's unsigned

  • This tree doesn't build with the newer upstream gnu-efi because of the API/ABI issues with realloc() and others. That looks easy to fix with a #define, but again, this won't pass CI until that's fixed.

Can be done by defining GNU_EFI_3_0_COMPAT

  • the need to add va_start() (etc) macros looks wrong to me, and the comment that gnu-efi needs them is quite disturbing. gnu-efi is providing these functions (or at least their decls; it should always wind up using compiler builtins). That makes me suspect there's still an issue in the upstream gnu-efi variadics, which will bite us on x86. I haven't tried getting rid of this and seeing if it's really necessary, because this tree doesn't build.

Not defining GNU_EFI_USE_EXTERNAL_STDARG??

and stuff like the variadic function addition here makes me think we're going to get stuck holding the bag with printf() doing memory corruption again, which isn't acceptable.

Totally agree - this is a bit clutching at straws though - if there's a problem that isn't fixed just send a bug report

@vathpela vathpela mentioned this pull request Jan 8, 2025
@vathpela
Copy link
Member

vathpela commented Jan 8, 2025

@vathpela It has been awhile since I looked but I remain able to build off the latest upstream gnu-efi with the additional changes I laid out in https://github.com/jmontleon/shim At the time I used gnu-efi 33727c2abe8a77dca612f04abd19e10cc5e487fd

But updating gnu-efi to 5ea320f0f01c8de8f9dd4e4e38a245608f0287dd still builds:

git clone git@github.com:jmontleon/shim
cd shim/
git submodule update --init
make

I did a scratch a riscv64 scratch build (http://fedora.riscv.rocks/koji/taskinfo?taskID=1837632) and tested in a virtual machine quick. It seems to still work ok.

Okay, but if I try that in the fairly straightforward way with this PR's code, which seems like it should be similar, it doesn't work because of the char signedness issue on riscv64: https://github.com/rhboot/shim/actions/runs/12678246015/job/35335419817?pr=713
It also breaks every other build because of that issue and the va_list problem: https://github.com/rhboot/shim/actions/runs/12678246015/job/35335417707?pr=713

I don't doubt that you've got something working, but I'm not reviewing some series of changes somewhere else, because I can't pull that. I'm reviewing a pull request here, and it doesn't work for the reasons state above.

I did see an issue with va_list on x86_64 and added a workaround patch to the srpm there so it should build on x86_64 now as well.

That needs to be represented in the patches here.

I'm definitely not insisting this or all the rest is 100% correct and ready to merge, but it should at least be possible to build and perhaps we can start working down the list of other issues you raised above?

Sure, that makes sense - but that's what this PR is for? The changes we need should be in the PR.

Now obviously we need to have some more discussion about gnu-efi especially with regard to the char issue and the va_list weirdness; I understand that. But there's no proposed fix for those issues in this PR as far as I can see.

@vathpela
Copy link
Member

vathpela commented Jan 8, 2025

Okay, but if I try that in the fairly straightforward way with this PR's code, which seems like it should be similar, it doesn't work because of the char signedness issue on riscv64: https://github.com/rhboot/shim/actions/runs/12678246015/job/35335419817?pr=713 It also breaks every other build because of that issue and the va_list problem: https://github.com/rhboot/shim/actions/runs/12678246015/job/35335417707?pr=713

Minor correction to myself - it's failing in #713 because there's no patch here to fix the define to make the breaking change with realloc() work. If I define GNU_EFI_3_0_COMPAT, it then breaks on something regarding elf.h changes and include paths in gnu-efi. I haven't debugged this further.

@vathpela
Copy link
Member

vathpela commented Jan 8, 2025

  • CHAR8 and char don't match in signedness. I don't have a dog in the fight of which signedness CHAR8 should be on this platform (or really any others), but I don't see how they can be fully independent, and they haven't been in the past, so there's a fair amount of code depending on it. If there's a compelling reason for them not to be the same, we still need patches to fix all the errors that causes, which right now seem to be a lot.

UEFI spec says CHAR8 is Latin-1 (0-255) so it's unsigned

I'm not even sure if this is actually an issue right now, because again, this PR doesn't give me a tree that builds due to other issues. Happy to continue this part of the discussion once we've figured out the blocking issues.

  • This tree doesn't build with the newer upstream gnu-efi because of the API/ABI issues with realloc() and others. That looks easy to fix with a #define, but again, this won't pass CI until that's fixed.

Can be done by defining GNU_EFI_3_0_COMPAT

Looks likely, but even adding that here doesn't get me a tree that builds with any version of gnu-efi that I have.

  • the need to add va_start() (etc) macros looks wrong to me, and the comment that gnu-efi needs them is quite disturbing. gnu-efi is providing these functions (or at least their decls; it should always wind up using compiler builtins). That makes me suspect there's still an issue in the upstream gnu-efi variadics, which will bite us on x86. I haven't tried getting rid of this and seeing if it's really necessary, because this tree doesn't build.

Not defining GNU_EFI_USE_EXTERNAL_STDARG??

It may well be that the right answer is to define that for this platform and not add the code that's added to include/system/stdarg.h by 0fd170b . That at least seems plausible.

and stuff like the variadic function addition here makes me think we're going to get stuck holding the bag with printf() doing memory corruption again, which isn't acceptable.

Totally agree - this is a bit clutching at straws though - if there's a problem that isn't fixed just send a bug report

No, this is not clutching at straws - things that a) are sometimes hard to even notice, b) take up giant amounts of time to debug and c) can introduce exploitable flaws on other arches are an absolute showstopper, and I'm telling you that's a significant concern.

Even if I weren't concerned about that, just rebasing to the upstream gnu-efi tree doesn't look possible without significant work, as it's an absolute mess. I can't even fast forward the master branch on top of an older checkout of that master branch. Seriously, try it:

git branch old-master 3.0.18
git worktree add old-master old-master
cd old-master
git rebase origin/master

Rebasing our patches on top of it is also a mess, partly because of some refactoring patches that need to be rectified, partly because of the incredibly excessive use of merge commits, and partly because several patches from the rhboot gnu-efi tree appear to have been cherry-picked and sent upstream with different commit messages and attribution, which is itself concerning but in a different way.

@andreabolognani
Copy link

@vathpela thanks for looking further into this!

I have no wisdom to share when it comes to the specifics of the changes being discussed, so I'll leave that to the other people involved in the conversation.

It seems obvious to me though that the diff in this PR doesn't match what is currently being used in Fedora RISC-V, since we are definitely building the package successfully there. @jmontleon perhaps you can work with @brianredbeard to update this PR so that it matches reality?

Anyway, I see that you've enabled a riscv64 cross-compilation job as part of #713. That's very nice! And it looks like it didn't require a big effort to do so either.

In case you are interested in going further than mere compile-testing (once the PR is updated, of course!), we have documented the process of setting up an emulated riscv64 environment using QEMU. It should hopefully be fairly straightforward, but feel free to reach out to me if you have any questions.

Hope this helps :)

@gmbr3
Copy link

gmbr3 commented Jan 14, 2025

@vathpela

Currently attempting to build a working tree
gnu-efi needs changes - ncroxon/gnu-efi#62
my current shim tree - https://github.com/gmbr3/shim/tree/gnu-efi --> now passes CI for cross but fails testing on gnuefi_signed_strncmp?????)
I maybe found the va_list print issue ---> gmbr3/shim@b4f5cf4 and gmbr3/gnu-efi@b85fbaa

(yes i'm aware commit messages look awful -- this was purely for testing)

@gmbr3
Copy link

gmbr3 commented Jan 14, 2025

Things still to do - after people have given more comments about my changes:

  • rework shim's lds/crt0 to be based off the current gnu-efi version
  • set GNU stack to NX (trivial, with only affect build not runtime until we set it in PE aswell)
  • rework commits themselves
  • Fix RWX LOAD perms (likely lds issue)
  • Fix (.init/.fini)_array section with zero size

@gmbr3
Copy link

gmbr3 commented Feb 11, 2025

@andreabolognani
Copy link

@gmbr3 I am in no position to give any feedback on the technical side of things, but I have to say that I'm absolutely ecstatic to see you making progress. As someone who's extremely keen on seeing shim running on riscv64, your continued efforts give me a lot of hope. Thank you and keep up the excellent work!

@andreabolognani
Copy link

@gmbr3 I took a look at your work — not the code itself, which is unfortunately way too complex and specialized for me to even begin to grasp, but rather the overall structure in terms of git branches and so on — and my understanding is as follows.

What you're trying to do is to get upstream gnu-efi to a point where shim could successfully build against it instead of having to rely on its own soft fork which is based on an aging gnu-efi version (3.0.12 from 2020). This work has been happening in gnu-efi#62.

Additionally, you have your own shim branch where you're working on integrating those gnu-efi changes back into shim.

Does this sound about right? Assuming I haven't gotten anything horribly wrong, the effort seems to me like it's going in the right direction. I can't speak for the shim maintainers though.

What I think would be really helpful is if you opened a separate PR for your changes. Maybe mark it as draft for now, since it can't really be considered for inclusion yet, but at least it'd provide a way for the maintainers to conveniently see all your work in one place and hopefully provide feedback. You could consider incorporating #712/#713 into it as well.

Thank you in advance for considering this, and once again for your ongoing work on riscv64 support!

@marta-lewandowska
Copy link

Getting closer

https://github.com/gmbr3/shim/commits/gnu-efi/
https://github.com/gmbr3/gnu-efi/commits/shim-test/

@gmbr3 I can't competently review your code either, but I did try building it on x86, aarch64, and risc-v fedora VMs. It builds and boots on the first two, but the build fails on risc-v:
[fedora@localhost shim]$ make EFIDIR=fedora DESTDIR=tmp/ install
sed -e "s,@@Version@@,15.8,"
-e "s,@@uname@@,Linux riscv64 unknown unknown GNU/Linux,"
-e "s,@@COMMIT@@,8f7f7f1acadd1c0cfb28ac58537a71ad6eaa5a33,"
< /home/fedora/shim/version.c.in > version.c
gcc -std=gnu11 -ggdb -gdwarf-4 -gstrict-dwarf -ffreestanding -fmacro-prefix-map=/home/fedora/shim/= -fno-stack-protector -fno-strict-aliasing -fpic -fshort-wchar -Os -Wall -Wextra -Wno-missing-field-initializers -Werror -nostdinc -I/home/fedora/shim/Cryptlib -I/home/fedora/shim/Cryptlib/Include -I/home/fedora/shim/gnu-efi/inc -I/home/fedora/shim/gnu-efi/inc/ -I/home/fedora/shim/gnu-efi/inc/protocol -I/home/fedora/shim/include -iquote /home/fedora/shim -iquote /home/fedora/shim -isystem /home/fedora/shim/include/system -isystem /usr/lib/gcc/riscv64-redhat-linux/14/include -DDEFAULT_LOADER='L"\\grub.efi"' -DDEFAULT_LOADER_CHAR='"\\grub.efi"' -DEFI_ARCH='L""' -DDEBUGDIR='L"/usr/lib/debug/usr/share/shim/-15.8/"' -DGNU_EFI_USE_REALLOCATEPOOL_ABI=0 -DGNU_EFI_USE_COMPAREGUID_ABI=0 -c -o shim.o shim.c
In file included from shim.h:47,
from shim.c:14:
/home/fedora/shim/include/system/stdarg.h:56:2: error: #error what arch is this
56 | #error what arch is this
| ^~~~~
In file included from shim.h:156:
include/asm.h: In function ‘read_counter’:
include/asm.h:23:2: error: #error unsupported arch
23 | #error unsupported arch
| ^~~~~
In file included from shim.h:194:
Cryptlib/Include/OpenSslSupport.h: At top level:
Cryptlib/Include/OpenSslSupport.h:71:2: error: #error Unknown target architecture
71 | #error Unknown target architecture
| ^~~~~
In file included from shim.c:20:
/home/fedora/shim/Cryptlib/Include/openssl/bn.h:314:5: error: unknown type name ‘BN_ULONG’
314 | BN_ULONG d; / Pointer to an array of 'BN_BITS2' bit
| ^~~~~~~~

and so on...

@U2FsdGVkX1
Copy link

Is there any progress?

@andreabolognani
Copy link

I was able to reproduce the build error that @marta-lewandowska reported above.

Looking at the changes that are included in the https://github.com/gmbr3/shim/commits/gnu-efi/ branch a bit more closely (though I have to once again put out there the disclaimer that I know next to nothing about shim from a developer's perspective) I get the impression that @gmbr3 has been working pretty much exclusively on making shim build against the latest version of gnu-efi rather than porting it to the riscv64 architecture.

That is an extremely valuable endeavor, as it would leave us much better positioned to move forward with the riscv64 port; however, assuming that I've understood the situation correctly, it's not surprising that the branch would fail to build on riscv64. The fact that it apparently builds and runs fine on x86_64 and aarch64 is certainly encouraging though!

I've gone one step further and rebased the branch from this PR, which contains the initial riscv64 port by @xypron and further updates by @brianredbeard, on top of the gnu-efi update branch by @gmbr3. The result manages to get a lot further into the build process, but unfortunately still fails in the end:

ld -o shimriscv64.so --hash-style=sysv -nostdlib -znocombreloc -T /home/abologna/src/upstream/shim/elf_riscv64_efi.lds -shared -Bsymbolic -L/home/abologna/src/upstream/shim/gnu-efi/riscv64/lib -L/home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi -LCryptlib -LCryptlib/OpenSSL /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/crt0-efi-riscv64.o --build-id=sha1  --no-undefined shim.o globals.o mok.o netboot.o cert.o replacements.o tpm.o version.o errlog.o sbat.o sbat_data.o sbat_var.o pe.o pe-relocate.o httpboot.o csv.o load-options.o Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a lib/lib.a /home/abologna/src/upstream/shim/gnu-efi/riscv64/lib/libefi.a /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/libgnuefi.a -lefi -lgnuefi --start-group Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a --end-group /usr/lib/gcc/riscv64-redhat-linux/14/libgcc.a lib/lib.a
ld -o mmriscv64.so --hash-style=sysv -nostdlib -znocombreloc -T /home/abologna/src/upstream/shim/elf_riscv64_efi.lds -shared -Bsymbolic -L/home/abologna/src/upstream/shim/gnu-efi/riscv64/lib -L/home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi -LCryptlib -LCryptlib/OpenSSL /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/crt0-efi-riscv64.o --build-id=sha1  --no-undefined MokManager.o PasswordCrypt.o crypt_blowfish.o errlog.o sbat_data.o globals.o Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a lib/lib.a /home/abologna/src/upstream/shim/gnu-efi/riscv64/lib/libefi.a /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/libgnuefi.a -lefi -lgnuefi --start-group Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a --end-group /usr/lib/gcc/riscv64-redhat-linux/14/libgcc.a lib/lib.a
ld -o fbriscv64.so --hash-style=sysv -nostdlib -znocombreloc -T /home/abologna/src/upstream/shim/elf_riscv64_efi.lds -shared -Bsymbolic -L/home/abologna/src/upstream/shim/gnu-efi/riscv64/lib -L/home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi -LCryptlib -LCryptlib/OpenSSL /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/crt0-efi-riscv64.o --build-id=sha1  --no-undefined fallback.o tpm.o errlog.o sbat_data.o globals.o Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a lib/lib.a /home/abologna/src/upstream/shim/gnu-efi/riscv64/lib/libefi.a /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/libgnuefi.a -lefi -lgnuefi --start-group Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a --end-group /usr/lib/gcc/riscv64-redhat-linux/14/libgcc.a lib/lib.a
ld: /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/crt0-efi-riscv64.o: in function `_start':
/home/abologna/src/upstream/shim/gnu-efi/gnuefi/crt0-efi-riscv64.S:27:(.text+0x8): undefined reference to `ImageBase'
ld: warning: .init_array section has zero size
ld: warning: .fini_array section has zero size
make: *** [Makefile:158: fbriscv64.so] Error 1
make: *** Waiting for unfinished jobs....
ld: /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/crt0-efi-riscv64.o: in function `_start':
/home/abologna/src/upstream/shim/gnu-efi/gnuefi/crt0-efi-riscv64.S:27:(.text+0x8): undefined reference to `ImageBase'
ld: /home/abologna/src/upstream/shim/gnu-efi/riscv64/gnuefi/crt0-efi-riscv64.o: in function `_start':
/home/abologna/src/upstream/shim/gnu-efi/gnuefi/crt0-efi-riscv64.S:27:(.text+0x8): undefined reference to `ImageBase'
ld: pe.o: in function `handle_image':
/home/abologna/src/upstream/shim/pe.c:612:(.text+0x1726): undefined reference to `__riscv_flush_icache'
ld: warning: .init_array section has zero size
ld: warning: .fini_array section has zero size
make: *** [Makefile:163: mmriscv64.so] Error 1
ld: warning: .init_array section has zero size
ld: warning: .fini_array section has zero size
make: *** [Makefile:153: shimriscv64.so] Error 1

@gmbr3 once again assuming that my understanding of the current state is correct, can you please go ahead and create a separate PR specifically for the effort of updating gnu-efi in shim? @vathpela has indicated that this needs to happen before the maintainers will look at the proposed changes.

Once that branch exists, perhaps @xypron can attempt to rebase his port of top of it? What exists in #420 and here appears to be tantalizingly close to just working, and I'm sure that if someone who actually knows what they're doing could take another stab at it we'd have a real chance at getting the port merged.

I'm obviously always happy to build random branches and run any tests you might want me to.

@andreabolognani
Copy link

Coming back to this after a few busy months.

For the recently released RHEL 10 on RISC-V Developer Preview, I ended up cobbling together a fairly rough package based on the Fedora one. It combines shim 15.8, upstream gnu-efi 4.0.0, the patches found in this PR, plus a few fixes from @jmontleon's riscv branch and some of my own.

The dist-git tree has been published in the CentOS ISA SIG GitLab area, c10s-rv branch. I have also pushed an upstream-ish equivalent to my 15.8-riscv64 branch.

That all works fine on riscv64, but it fails to build on x86_64:

In file included from shim.h:57,
                 from globals.c:7:
.../gnu-efi/inc/efilib.h:560:5: error: unknown type name ‘va_list’
  560 |     va_list           args
      |     ^~~~~~~
.../gnu-efi/inc/efilib.h:50:1: note: ‘va_list’ is defined in header ‘<stdarg.h>’; this is probably fixable by adding ‘#include <stdarg.h>’
   49 | #include "libsmbios.h"
  +++ |+#include <stdarg.h>
   50 | 
.../gnu-efi/inc/efilib.h:576:5: error: unknown type name ‘va_list’
  576 |     va_list           args
      |     ^~~~~~~
.../gnu-efi/inc/efilib.h:576:5: note: ‘va_list’ is defined in header ‘<stdarg.h>’; this is probably fixable by adding ‘#include <stdarg.h>’
.../gnu-efi/inc/efilib.h:582:5: error: unknown type name ‘va_list’
  582 |     va_list             args
      |     ^~~~~~~
.../gnu-efi/inc/efilib.h:582:5: note: ‘va_list’ is defined in header ‘<stdarg.h>’; this is probably fixable by adding ‘#include <stdarg.h>’
.../gnu-efi/inc/efilib.h:639:5: error: unknown type name ‘va_list’
  639 |     va_list           args
      |     ^~~~~~~
.../gnu-efi/inc/efilib.h:639:5: note: ‘va_list’ is defined in header ‘<stdarg.h>’; this is probably fixable by adding ‘#include <stdarg.h>’
make: *** [<builtin>: globals.o] Error 1

So that's obviously a big problem, in addition to being a complete hodgepodge of patches messily building on top of each other with no rhyme or reason and being based on a now outdated version of shim.

In the meantime, gnu-efi#62 has been merged and is included in the gnu-efi 4.0.1 release. That's great news!

I was able to take @gmbr3's gnu-efi branch, swap in the actual gnu-efi 4.0.1 release add the usual set of riscv64 patches. The result is available from my gmbr3-riscv64 branch and, unlike last time around, it builds successfully both on x86_64 and riscv64 thanks to picking up Jason's fixes, which I had missed during the previous attempt.

But shim itself has moved forward. gnu-efi#69 identifies a number of additional commits that need to be upstreamed.

As a hack, I have coarsely chopped off the shim features related to the bits missing from upstream gnu-efi to see whether I would be able to get closer to the current state of shim. The result is in my main-riscv64 and yeah, it's pretty awful :) but it builds on both x86_64 and riscv64, so at least there's that!

It seems to me that we're really quite close to being able to switch to upstream gnu-efi and adding riscv64 support, it's mostly a matter of everyone coming together, picking up all the work that's already out there, organizing it and polishing it up so that it's suitable for merging.

@vathpela what are your thoughts on the matter? Does the work I've collected in the various branches mentioned above look like it's going in the right direction, are you seeing obvious problems with it and so on? Thanks!

@marta-lewandowska
Copy link

@andreabolognani it looks like gnu-efi changes needed for shim 16 have been merged upstream, but there is no PR for updating gnu-efi in shim.
Also, when you mention adding the "usual set of risc-v patches"; are you referring to this PR? Or fedora patches (which you mentioned in January don't match this PR)? No new code has been pushed to this PR since you wrote that it needs updates.
I guess it's not what you want to hear, but as vathpela mentioned before, if you want review of this PR, then the code needs to be here. If other changes are needed, like to gnu-efi, then a PR needs to be opened so that a discussion can happen.

@andreabolognani
Copy link

@marta-lewandowska understood.

I was hoping that @gmbr3 and @xypron would be spurred into action by my updates, but clearly that hasn't been the case so far. I also didn't want to split the conversation since we've collected so much good information here.

I will attempt to put together a somewhat reasonable branch of my own, polish it up to some degree and open a PR based on it. I'm really not the best candidate to be doing that due to lack of expertise when it comes to developing both gnu-efi and shim, but if there's no other way forward I'm willing to at least take a stab.

@andreabolognani
Copy link

andreabolognani commented Sep 26, 2025

@vathpela @marta-lewandowska I have now created #777 (for switching to upstream gnu-efi) and #778 (for adding riscv64 support on top of that). Hopefully this will help move things forward :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.