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

[Jessica Clarke]

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
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.

So this is a resounding NAK from me.

Jess

> Björn
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi