RISCV64 driver does not have the proper EFI subsystem set

[pbatard]

Despite having an explicit --defsym=EFI_SUBSYSTEM=0xb in the linker command:

riscv64-linux-gnu-ld -nostdlib  --warn-common --no-undefined --fatal-warnings --build-id=sha1 -z nocombreloc -z norelro -L/home/runner/work/gnu-efi/gnu-efi//apps/../riscv64/lib -L/home/runner/work/gnu-efi/gnu-efi//apps/../riscv64/gnuefi /home/runner/work/gnu-efi/gnu-efi//apps/../riscv64/gnuefi/crt0-efi-riscv64.o -shared -Bsymbolic -L/home/runner/work/gnu-efi/gnu-efi//apps/../riscv64/lib --defsym=EFI_SUBSYSTEM=0xb drv0.o -o drv0.so -T /home/runner/work/gnu-efi/gnu-efi//apps/../gnuefi/elf_riscv64_efi.lds -lefi -lgnuefi /usr/lib/gcc-cross/riscv64-linux-gnu/11/libgcc.a

It seems that the EFI subsystem is not properly applied to the RISCV64 driver, as the riscv64 artifacts from the build that produced https://github.com/ncroxon/gnu-efi/actions/runs/9159433032/job/25179796521 show that the drv0.efi is still set as a UEFI Application (EFI_SUBSYSTEM=0xa):

This is also confirmed by trying to load the driver in with an UEFI firmware in QEMU RISCV64, which fails because of the wrong subsystem type.

Considering that we have an explicit:

#ifndef EFI_SUBSYSTEM
#define EFI_SUBSYSTEM 10
#endif

in gnuefi/crt0-efi-riscv64.S that we don't have in other archs, and that removing it produces the error:

gnuefi/crt0-efi-riscv64.S:70: Error: cannot represent BFD_RELOC_16 relocation in object file

I would think that the issue is that we need a delayed symbol resolution in crt0-efi-riscv64.o (since it goes into libgnuefi.a which we then link with the app, and which is the library that contains the PE header with the EFI_SUBSYSTEM field), that either we haven't figured how to do, or that the current RISCV64 toolchain doesn't support yet...

[gmbr3]

Yep, no 16bit REL
(https://github.com/bminor/binutils-gdb/blob/92f59183ab085f5762650c3242a33119d161e668/bfd/elfxx-riscv.c#L973)
RISCV64 will have to use generator mode (like fwupd-efi) and/or objcopy
It can't support delayed symbol mode

[pbatard]

Any chance we could just combine the 16-bit Subsystem and subsequent 16-bit DllCharacteristics into a 32-bit, since we have 32bit REL? DllCharacteristics is always zero for UEFI and we are dealing with little endian values, so, on paper, this looks like a simpler workaround.

However, while my testing doing just that compiles without throwing any error (and without the need for the #ifndef EFI_SUBSYSTEM), the resulting Subsystem and DllCharacteristics fields in the PE are always set to 0. Yet, if I change the name of the EFI_SUBSYSTEM variable in the .S to something else, the linker will throw an error about undefined reference for that other name, which tends to indicate that the linker does see the 32-bit EFI_SUBSYSTEM as a delayed symbol, but it somehow still fail to populate it with the value we pass in --defsym=EFI_SUBSYSTEM...

Am I missing something obvious? Or is just another RISCV64 toolchain limitation/bug?

[gmbr3]

Yet, if I change the name of the EFI_SUBSYSTEM variable in the .S to something else, the linker will throw an error about undefined reference for that other name, which tends to indicate that the linker does see the 32-bit EFI_SUBSYSTEM as a delayed symbol, but it somehow still fail to populate it with the value we pass in --defsym=EFI_SUBSYSTEM...

That also seems weird 😕 , it should work like that.

Also DllCharacteristics is now populated by NX bit data for UEFI

[pbatard]

Hmm, further testing shows that the value we pass in --defsym=EFI_SUBSYSTEM ends up in the .rodata section rather than in the header...

[pbatard]

All things considered, I think our best course of action for now is to just add an extra step to our build that sets the SubSystem property after the objcopy step, and I have just proposed a patchset that does this.