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

[Xiang W]

在 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?

Regards,
Xiang W
> 
> Jess