Re: [PATCH] riscv: CONFIG_EFI should not depend on CONFIG_RISCV_ISA_C

["Björn Töpel" <bjorn@kernel.org>]

Palmer Dabbelt <palmer@dabbelt.com> writes:

> On Tue, 24 Oct 2023 12:26:48 PDT (-0700), bjorn@kernel.org wrote:
>> From: Björn Töpel <bjorn@rivosinc.com>
>>
>> UEFI/PE mandates that the kernel Image starts with "MZ" ASCII
>> (0x5A4D). Convenient enough, "MZ" is a valid compressed RISC-V
>> instruction. This means that a non-UEFI loader can simply jump to
>> "code0" in the Image header [1] and start executing.
>>
>> The Image specification [1] says the following about "code0":
>>   |   This header is also reused to support EFI stub for RISC-V. EFI
>>   |   specification needs PE/COFF image header in the beginning of the
>>   |   kernel image in order to load it as an EFI application. In order
>>   |   to support EFI stub, code0 is replaced with "MZ" magic string
>>   |   and res3(at offset 0x3c) points to the rest of the PE/COFF
>>   |   header.
>>
>> "MZ" is not a valid instruction for implementations without the C
>> extension.
>>
>> A non-UEFI loader, loading a non-C UEFI image have the following
>> options:
>>   1. Trap and emulate "code0"
>>   2. Avoid "code0" if it is "MZ", and have the kernel entry at
>>      "code1".
>>
>> Replace the compressed instruction with a hex code variant, that works
>> for CONFIG_RISCV_ISA_C=n builds. Further, this change also make sure
>> that the "code0" instruction is 32b aligned.
>
> IMO this breaks the Image format on non-C systems: they'll jump straight 
> to the start (as there's no symbols left or anything) and then just fail 
> to execute the C instructions.

That's right! My idea, which was probably really poor articulated, was
to add trap/emulate in OpenSBI/non-C for the MZ instructions.

> We could amend the Image format to require bootloaders to handle this 
> (ie, look at the first instructions and emulate them if there's no C), 
> that would require some bootloader updates but given this doesn't work 
> maybe that's fine.
>
> We could also just stop producing Image+PE binaries on non-C systems 
> (ie, just produce PE).

Yes, or make the Image loader in Qemu et al more robust, by e.g.
checking code0/code1. The Image spec does say that code0 is 'MZ' for
PE+Image builds, and for non-C capable systems one could argue that it
should be checked/skipped by the loader.

Does the arguments above make sense for you? I.e.:
  1.   Merge this patch
  2a.  Add the trap/emu to OpenSBI
  (2b. Improve the image loader in Qemu?)


Björn