[PATCH] lib: sbi_illegal_insn: Emulate 'MZ'/c.li s4,-13 instruction

[Jessica Clarke]

On 9 Nov 2023, at 06:55, Xiang W <wxjstz at 126.com> wrote:
> 在 2023-11-09星期四的 06:24 +0000，Jessica Clarke写道：
>> On 9 Nov 2023, at 04:04, Xiang W <wxjstz at 126.com> wrote:
>>> 
>>> 在 2023-11-09星期四的 01:22 +0100，Björn Töpel写道：
>>>> On Thu, 9 Nov 2023 at 01:11, Jessica Clarke <jrtc27 at jrtc27.com> wrote:
>>>>> 
>>>>> On 9 Nov 2023, at 00:02, Björn Töpel <bjorn at kernel.org> wrote:
>>>>>> 
>>>>>> Hi Andreas!
>>>>>> 
>>>>>> On Wed, 8 Nov 2023 at 22:55, Andreas Schwab <schwab at linux-m68k.org> wrote:
>>>>>>> 
>>>>>>> On Nov 08 2023, Björn Töpel wrote:
>>>>>>> 
>>>>>>>> +static int compressed_insn(ulong insn, struct sbi_trap_regs *regs)
>>>>>>>> +{
>>>>>>>> +     /* Only handle 'MZ'/c.li s4,-13/0x5a3d */
>>>>>>>> +     if (!misa_extension('C') && (insn & 0xffff) == 0x5a4d) {
>>>>>>>> +             regs->s4 = -13;
>>>>>>>> +             regs->mepc += 4;
>>>>>>> 
>>>>>>> By skipping 4 bytes execution will resume in the middle of the next insn
>>>>>>> (the jump around the header).
>>>>>> 
>>>>>> This is in a non-C environment -- "!misa_extension('C')", so we're not
>>>>>> jumping into the middle of the next insn.
>>>>> 
>>>>> But that’s not c.li, is it. That’s bjorn.c.li, a 32-bit instruction
>>>>                                                  ^^^^ My very own! :-D
>>>> 
>>>>> whose first 16-bit parcel is the same as c.li and whose second 16-bit
>>>>> parcel is ignored. I really do not think this is a good idea.
>>>>> 
>>>>>> Note that the Linux kernel needs the change pointed out in this patch,
>>>>>> to build w/o C.
>>>>> 
>>>>> That sounds like a Linux problem. Other OSes cope just fine. Fixing in
>>>>> firmware is not the right approach; fix your software instead. Which
>>>>> may mean abandoning an image format that isn’t fit for purpose. But the
>>>>> solution isn’t to hack in whatever random crud Linux wants in firmware,
>>>>> and other non-firmware SBI implementations like hypervisors.
>>>> 
>>>> There are other kernels that people care about?
>>>> 
>>>>> So this is a resounding NAK from me.
>>>> 
>>>> Jokes aside; Fair enough! I'll go back to the drawing board.
>>> 
>>> The kernel cannot handle this problem because the kernel has not initialized
>>> stvec at this time. And the MZ cannot be changed because it is part of the PE
>>> header. The best way is to skip the MZ of the PE header through the upper
>>> bootloader. Therefore, when the kernel is used as the payload of opensbi,
>>> skipped operations also need to be added. sbi_hart_switch_mode needs to add
>>> some code to detect whether the current core supports C extension, whether
>>> there is 0x5a4d5a4d at next_addr, and modify next_addr.
>> 
>> That’s also extremely ugly; now you’re just skipping instructions in
>> the payload rather than trapping like the current de-facto spec would
>> mandate. And what would you skip? Again, you can’t just skip 2 bytes,
>> you would have to skip a whole 4 bytes, which is more than just the
>> c.li instruction. And what happens if a vendor, not implementing C,
>> wants to use that encoding space? For example, Qualcomm, who are
>> proposing to ditch C entirely. It may not conform to a standard
>> profile, but it is a thing that SBI must support.
>> 
>> Linux is abusing file formats and trying to create a polyglot where
>> it’s not possible. That’s Linux’s problem, and it needs to stop
>> assuming things that aren’t true. This grotesque approach is not
>> present in other OSes.
> How do other operating systems solve it? This is a problem with the uefi
> image file format. Can we remove dos header?

They don’t delude themselves that you can just jump to the start of a
UEFI PE/COFF executable and interpret it as machine code.

Jess