Updated support for SHIM on RISC-V #641
Conversation
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
force-pushed
the
riscv-shim
branch
from
144f786
to
debe983
Compare
|
@vathpela can we get a review? |
brianredbeard
force-pushed
the
riscv-shim
branch
from
debe983
to
5031a99
Compare
|
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 |
|
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: |
Make.defaults
Outdated
| ARCH_GNUEFI ?= riscv64 | ||
| ARCH_SUFFIX ?= riscv64 | ||
| ARCH_SUFFIX_UPPER ?= RISCV64 | ||
| FORMAT := -O binary |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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/
There was a problem hiding this comment.
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.
|
The last commit is missing |
| @@ -128,6 +128,21 @@ | |||
| #endif | |||
| #endif | |||
|
|
|||
| #if defined(__riscv) && __riscv_xlen == 64 | |||
| #ifndef DEFAULT_LOADER | |||
| #define DEFAULT_LOADER L"\\grubriscv64.efi" | |||
There was a problem hiding this comment.
Shouldn't the file name be shorter (as in 8.3 format) given a FAT filesystem?
There was a problem hiding this comment.
- 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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
UEFI requires support for FAT long filenames (LFN) in all FAT12, FAT16 and FAT32 so it's fine
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.