Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated support for SHIM on RISC-V #641

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Updated support for SHIM on RISC-V #641

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5031a99
Initial RISC-V support
xypron Apr 5, 2021
9f9813b
bug: Remove extraneous configuration from RISC-V
brianredbeard Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cryptlib/Include/OpenSslSupport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

#define CONFIG_HEADER_BN_H

#if defined(MDE_CPU_X64) || defined(MDE_CPU_AARCH64) || defined(MDE_CPU_IA64)
#if defined(MDE_CPU_X64) || defined(MDE_CPU_AARCH64) || \
defined(MDE_CPU_IA64) || defined(MDE_CPU_RISCV64)
//
// With GCC we would normally use SIXTY_FOUR_BIT_LONG, but MSVC needs
// SIXTY_FOUR_BIT, because 'long' is 32-bit and only 'long long' is
Expand Down
3 changes: 3 additions & 0 deletions Cryptlib/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ endif
ifeq ($(ARCH),arm)
DEFINES += -DMDE_CPU_ARM
endif
ifeq ($(ARCH),riscv64)
DEFINES += -DMDE_CPU_RISCV64
endif

LDFLAGS = -nostdlib -znocombreloc

Expand Down
3 changes: 3 additions & 0 deletions Cryptlib/OpenSSL/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ endif
ifeq ($(ARCH),arm)
DEFINES += -DMDE_CPU_ARM
endif
ifeq ($(ARCH),riscv64)
DEFINES += -DMDE_CPU_RISCV64
endif

LDFLAGS = -nostdlib -znocombreloc

Expand Down
10 changes: 10 additions & 0 deletions Make.defaults
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,16 @@ ifeq ($(ARCH),arm)
SUBSYSTEM := 0xa
ARCH_LDFLAGS += --defsym=EFI_SUBSYSTEM=$(SUBSYSTEM)
endif
ifeq ($(ARCH),riscv64)
ARCH_CFLAGS ?= -DMDE_CPU_RISCV64 -DPAGE_SIZE=4096
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 •
edited
Loading

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.

SUBSYSTEM := 0xa
ARCH_LDFLAGS += --defsym=EFI_SUBSYSTEM=$(SUBSYSTEM)
TIMESTAMP_LOCATION := 72
endif

DEFINES = -DDEFAULT_LOADER='L"$(DEFAULT_LOADER)"' \
-DDEFAULT_LOADER_CHAR='"$(DEFAULT_LOADER)"'
Expand Down
111 changes: 111 additions & 0 deletions elf_riscv64_efi.lds
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
OUTPUT_FORMAT("elf64-littleriscv", "elf64-littleriscv", "elf64-littleriscv")
OUTPUT_ARCH(riscv)
ENTRY(_start)
SECTIONS
{
.text 0x0 : {
_text = .;
*(.text.head)
*(.text)
*(.text.*)
*(.gnu.linkonce.t.*)
_evtext = .;
. = ALIGN(4096);
}
_etext = .;
_text_size = . - _text;
_text_vsize = _evtext - _text;

. = ALIGN(4096);
.data :
{
_data = .;
*(.sdata)
*(.data)
*(.data1)
*(.data.*)
*(.got.plt)
*(.got)

*(.dynamic)

/* the EFI loader doesn't seem to like a .bss section, so we stick
it all into .data: */
. = ALIGN(16);
_bss = .;
*(.sbss)
*(.scommon)
*(.dynbss)
*(.bss)
*(COMMON)
_evdata = .;
. = ALIGN(4096);
_bss_end = .;
}
_edata = .;
_data_vsize = _evdata - _data;
_data_size = . - _data;

/*
* Note that _sbat must be the beginning of the data, and _esbat must be the
* end and must be before any section padding. The sbat self-check uses
* _esbat to find the bounds of the data, and if the padding is included, the
* CSV parser (correctly) rejects the data as having NUL values in one of the
* required columns.
*/
. = ALIGN(4096);
.sbat :
{
_sbat = .;
*(.sbat)
*(.sbat.*)
_esbat = .;
. = ALIGN(4096);
_epsbat = .;
}
_sbat_size = _epsbat - _sbat;
_sbat_vsize = _esbat - _sbat;

. = ALIGN(4096);
.rodata :
{
_rodata = .;
*(.rodata*)
*(.srodata)
. = ALIGN(16);
*(.note.gnu.build-id)
. = ALIGN(4096);
*(.vendor_cert)
*(.data.ident)
. = ALIGN(4096);
}
. = ALIGN(4096);
.rela :
{
*(.rela.dyn)
*(.rela.plt)
*(.rela.got)
*(.rela.data)
*(.rela.data*)
}
. = ALIGN(4096);
.dyn :
{
*(.dynsym)
*(.dynstr)
_evrodata = .;
. = ALIGN(4096);
}
_erodata = .;
_rodata_size = . - _rodata;
_rodata_vsize = _evrodata - _rodata;
_alldata_size = . - _data;

/DISCARD/ :
{
*(.rel.reloc)
*(.eh_frame)
*(.note.GNU-stack)
}
.comment 0 : { *(.comment) }
}
2 changes: 2 additions & 0 deletions include/asm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ static inline uint64_t read_counter(void)
__asm__ __volatile__ ("mrs %0, pmccntr_el0" : "=r" (val));
#elif defined(__arm__)
__asm__ __volatile__ ("mrc p15, 0, %0, c9, c13, 0" : "=r" (val));
#elif defined(__riscv) && __riscv_xlen == 64
__asm__ __volatile__ ("csrr %0, 0xc01" : "=r" (val) : : "memory");
#else
#error unsupported arch
#endif
Expand Down
2 changes: 2 additions & 0 deletions include/peimage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
#define IMAGE_FILE_MACHINE_X64 0x8664
#define IMAGE_FILE_MACHINE_ARMTHUMB_MIXED 0x01c2
#define IMAGE_FILE_MACHINE_ARM64 0xaa64
#define IMAGE_FILE_MACHINE_RISCV32 0x5032
#define IMAGE_FILE_MACHINE_RISCV64 0x5064

//
// EXE file formats
Expand Down
12 changes: 11 additions & 1 deletion include/system/stdarg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ typedef __builtin_va_list __builtin_sysv_va_list;
#endif

#if defined(__aarch64__) || defined(__arm__) || defined(__i386__) || \
defined(__i486__) || defined(__i686__) || defined(__COVERITY__)
defined(__i486__) || defined(__i686__) || defined(__COVERITY__) || defined(__riscv)

typedef __builtin_va_list ms_va_list;
typedef __builtin_va_list __builtin_ms_va_list;
Expand All @@ -38,6 +38,16 @@ typedef __builtin_va_list sysv_va_list;
#define sysv_va_start(marker, arg) __builtin_va_start(marker, arg)
#define sysv_va_arg(marker, type) __builtin_va_arg(marker, type)
#define sysv_va_end(marker) __builtin_va_end(marker)

/*
* gnu-efi needs this.
*/
typedef __builtin_va_list va_list;
# define va_start(v,l) __builtin_va_start(v,l)
# define va_end(v) __builtin_va_end(v)
# define va_arg(v,l) __builtin_va_arg(v,l)
# define va_copy(d,s) __builtin_va_copy(d,s)

/*
* OpenSSL's X509ConstructCertificateStack needs this.
*/
Expand Down
3 changes: 3 additions & 0 deletions lib/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ endif
ifeq ($(ARCH),arm)
DEFINES += -DMDE_CPU_ARM
endif
ifeq ($(ARCH),riscv64)
DEFINES += -DMDE_CPU_RISCV64
endif

LDFLAGS = -nostdlib -znocombreloc

Expand Down
6 changes: 6 additions & 0 deletions pe-relocate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ allow_64_bit(void)
if (in_protocol)
return 1;
return 0;
#elif defined (__riscv) && __riscv_xlen == 64
return 1;
#else /* assuming everything else is 32-bit... */
return 0;
#endif
Expand All @@ -300,6 +302,8 @@ allow_32_bit(void)
return 1;
#elif defined(__aarch64__)
return 0;
#elif defined (__riscv) && __riscv_xlen == 64
return 0;
#else /* assuming everything else is 32-bit... */
return 1;
#endif
Expand All @@ -326,6 +330,8 @@ static const UINT16 machine_type =
IMAGE_FILE_MACHINE_I386;
#elif defined(__ia64__)
IMAGE_FILE_MACHINE_IA64;
#elif defined(__riscv) && __riscv_xlen == 64
IMAGE_FILE_MACHINE_RISCV64;
#else
#error this architecture is not supported by shim
#endif
Expand Down
15 changes: 15 additions & 0 deletions shim.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,21 @@
#endif
#endif

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

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)

Copy link

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 •
edited
Loading

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

#endif
#ifndef DEFAULT_LOADER_CHAR
#define DEFAULT_LOADER_CHAR "\\grubriscv64.efi"
#endif
#ifndef EFI_ARCH
#define EFI_ARCH L"riscv64"
#endif
#ifndef DEBUGDIR
#define DEBUGDIR L"/usr/lib/debug/usr/share/shim/riscv64/"
#endif
#endif

#ifndef DEBUGSRC
#define DEBUGSRC L"/usr/src/debug/shim-" VERSIONSTR "." EFI_ARCH
#endif
Expand Down