question about RISCV EFI stub support

[Palmer Dabbelt]

On Tue, 02 Mar 2021 00:20:12 PST (-0800), atishp at atishpatra.org wrote:
> On Mon, Mar 1, 2021 at 10:16 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>>
>> On Mon, 1 Mar 2021 at 15:43, Kefeng Wang <wangkefeng.wang at huawei.com> wrote:
>> >
>> > + Palmer, sorry forget add to cc list...
>> >
>> > On 2021/2/25 10:00, Atish Patra wrote:
>> > > On Wed, Feb 24, 2021 at 5:29 PM Kefeng Wang <wangkefeng.wang at huawei.com> wrote:
>> > >>
>> > >> On 2021/2/25 7:35, Atish Patra wrote:
>> > >>> On Wed, Feb 24, 2021 at 3:14 AM Kefeng Wang <wangkefeng.wang at huawei.com> wrote:
>> > >>>> Hi Atish and Ard,
>> > >>>>
>> > >>>> The commit "cb7d2dd5612a RISC-V: Add PE/COFF header for EFI stub" use
>> > >>>> c.li to
>> > >>>>
>> > >>>> add Magic "MZ" signature for PE/COFF header, which select RISCV_ISA_C,
>> > >>>> but "C"
>> > >>>>
>> > >>>> to the ISA is optional,  and if "C" is not support ,  EFI boot is not
>> > >>>> supported.
>> > >>>>
>> > >>>> Could we use ".word MZ_MAGIC" like x86 code or some other way to support
>> > >>>> "MZ" signature?
>> > >>>>
>> > >>> .word won't work but .short will. With .word, you end up 0000 in the 2
>> > >>> bytes which can be interpreted as illegal instruction.
>> > >> Yes, should use ".short" in RV.
>> > >>> I guess your larger objective is to make the C extension optional and
>> > >>> not enabled by default for the Linux kernel.
>> > >> "C" extension is optional, and the upstream kernel has the RISCV_ISA_C which
>> > >>
>> > >> could be configurable.  and I think the EFI_STUB should not depend or
>> > >> select it,
>> > >>
>> > >> and maybe we only want to support RV64G in some cases,  what's your option?
>> > >>
>> > > I think RISCV_ISA_C was enabled by default just to generate a smaller
>> > > binary size.
>> > > @palmer : was there any other reason ?
>> >
>> > Any comment about the too issue,
>> >
>> > 1)  directly use .shout MZ_MAGIC to decouple EFI_STUB and RISCV_ISA_C.
>> >
>> > 2)  don't set RISCV_ISA_C  to default y
>> >
>>
>> But doesn't that break non-EFI boot with the same image? The opcode
>> still needs to be executable, and have close-to-NOP semantics.
>
> Correct. It will break non-EFI boot for the hardware if the platform
> doesn't support C extension.
> Theoretically, we can decouple EFI_STUB & RISCV_ISA_C. But that's
> useless as the hardware has to support C anyways.
> Apologies for not seeing this through.
>
> If a platform doesn't have a C extension, they can not use the EFI
> booting method.

So the dependency here is actually a bit odd: platforms without C _can_ use 
EFI, the EFI images just can't be booted on non-EFI systems (because the flat 
binaries need to execute the PE/COFF header).  The issue is really that we 
can't support all three of non-C, EFI, and non-EFI in the Image format.

The reason we coupled C and EFI is really to avoid the existence of EFI-only 
kernels, which are a headache these days because EFI isn't widely supported or 
used on RISC-V.  The general assumption is that, since we recommend rv64gc for 
portable systems and EFI is mostly of interest to portable systems, we won't 
end up with non-C EFI systems and therefor don't need to worry about having 
incompatible kernels.  That said, if there's one thing I've learned from the 
whole RISC-V thing it's that whenever I say "we don't think hardware that does 
X will show up" it does.  Assuming that's the case and we end up with non-C 
systems that people want to run EFI on, we'll figure out a way to do that -- if 
someone has real hardware I'm inclined to support it, otherwise we just end up 
with a mess of vendor-specific forks.

One option would be to just let people build EFI-only kernels.  This would be a 
trivial matter, as suggested somewhere in this thread -- I'd probably add some 
Kconfig juggling to try and get portable binaries where possible, but that's 
pretty straight-forward stuff.  The issue there is that we'll end up with 
kernels that can only be booted on EFI systems.  That may be a reasonable thing 
to do at some point in the future if EFI ends up widely implemented, but it'd 
be a headache today with EFI not being all that common.

Another option would be to just formalize the first four bytes of the Image 
format as not actually being instructions, but being a header that describes 
where to begin execution from.  That would be an ABI break and therefor would 
require changing the Image format, but we'd do it in a way that's 
ABI-compatible for C systems and we'd maintain the old Image format for the old 
systems.  That's not really much code, but it is a bit of a headache on the 
specification side.  Probably too much to be worth it, as if people really want 
EFI on these systems then EFI will probably be common enough that we'll all be 
running it.

I don't really want to do any of that, so I'm going to just hope we get lucky 
and don't end up with non-C hardware that anyone wants to run EFI on.