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

Björn Töpel bjorn at kernel.org
Wed Nov 8 16:22:17 PST 2023


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.

Thank you for the input!


Björn



More information about the opensbi mailing list