svn commit: r340653 - in head/sys/powerpc: fpu include powerpc

[Justin Hibbits]

Well, this is to allow a retry in case the cache wasn't properly flushed
already.  This came about because the blrl instruction in the GOT was seen
as an illegal instruction, and it didn't seem to matter if I synced in
pmap_enter(), it would still SIGILL probabilistically.  Since this block
was already present for a reason it made sense to make it actually
functional, since it does solve that problem.  I don't know if there's yet
another hardware errata on the e500, regarding caching, because I would
expect a page load like this to get synced no matter the load address.

- Justin

On Mon, Nov 19, 2018, 18:14 Nathan Whitehorn <nwhitehorn at freebsd.org wrote:

> Is this reasonable? What if the junk in the cache happened to be a
> *valid* instruction? Won't this approach result in silent corruption and
> later failure?
> -Nathan
>
> On 11/19/18 3:54 PM, Justin Hibbits wrote:
> > Author: jhibbits
> > Date: Mon Nov 19 23:54:49 2018
> > New Revision: 340653
> > URL: https://svnweb.freebsd.org/changeset/base/340653
> >
> > Log:
> >   powerpc: Sync icache on SIGILL, in case of cache issues
> >
> >   The update of jemalloc to 5.1.0 exposed a cache syncing issue on a
> Freescale
> >   e500 base system.  There was already code in the FPU emulator to
> address
> >   this, but it was limited to a single static variable, and did not
> attempt to
> >   sync the cache.  This pulls that out to the higher level program
> exception
> >   handler, and syncs the cache.
> >
> >   If a SIGILL is hit a second time at the same address, it will be
> treated as
> >   a real illegal instruction, and handled accordingly.
> >
> > Modified:
> >   head/sys/powerpc/fpu/fpu_emu.c
> >   head/sys/powerpc/include/pcb.h
> >   head/sys/powerpc/powerpc/exec_machdep.c
> >
> > Modified: head/sys/powerpc/fpu/fpu_emu.c
> >
> ==============================================================================
> > --- head/sys/powerpc/fpu/fpu_emu.c    Mon Nov 19 22:18:18 2018
> (r340652)
> > +++ head/sys/powerpc/fpu/fpu_emu.c    Mon Nov 19 23:54:49 2018
> (r340653)
> > @@ -189,7 +189,6 @@ fpu_emulate(struct trapframe *frame, struct fpu *fpf)
> >  {
> >       union instr insn;
> >       struct fpemu fe;
> > -     static int lastill = 0;
> >       int sig;
> >
> >       /* initialize insn.is_datasize to tell it is *not* initialized */
> > @@ -243,17 +242,11 @@ fpu_emulate(struct trapframe *frame, struct fpu
> *fpf)
> >                       opc_disasm(frame->srr0, insn.i_int);
> >               }
> >  #endif
> > -             /*
> > -             * XXXX retry an illegal insn once due to cache issues.
> > -             */
> > -             if (lastill == frame->srr0) {
> > -                     sig = SIGILL;
> > +             sig = SIGILL;
> >  #ifdef DEBUG
> > -                     if (fpe_debug & FPE_EX)
> > -                             kdb_enter(KDB_WHY_UNSET, "illegal
> instruction");
> > +             if (fpe_debug & FPE_EX)
> > +                     kdb_enter(KDB_WHY_UNSET, "illegal instruction");
> >  #endif
> > -             }
> > -             lastill = frame->srr0;
> >               break;
> >       }
> >
> >
> > Modified: head/sys/powerpc/include/pcb.h
> >
> ==============================================================================
> > --- head/sys/powerpc/include/pcb.h    Mon Nov 19 22:18:18 2018
> (r340652)
> > +++ head/sys/powerpc/include/pcb.h    Mon Nov 19 23:54:49 2018
> (r340653)
> > @@ -89,6 +89,7 @@ struct pcb {
> >                       register_t      dbcr0;
> >               } booke;
> >       } pcb_cpu;
> > +     vm_offset_t pcb_lastill;        /* Last illegal instruction */
> >  };
> >  #endif
> >
> >
> > Modified: head/sys/powerpc/powerpc/exec_machdep.c
> >
> ==============================================================================
> > --- head/sys/powerpc/powerpc/exec_machdep.c   Mon Nov 19 22:18:18 2018
>       (r340652)
> > +++ head/sys/powerpc/powerpc/exec_machdep.c   Mon Nov 19 23:54:49 2018
>       (r340653)
> > @@ -94,6 +94,8 @@ __FBSDID("$FreeBSD$");
> >  #include <machine/trap.h>
> >  #include <machine/vmparam.h>
> >
> > +#include <vm/pmap.h>
> > +
> >  #ifdef FPU_EMU
> >  #include <powerpc/fpu/fpu_extern.h>
> >  #endif
> > @@ -1099,6 +1101,14 @@ ppc_instr_emulate(struct trapframe *frame, struct
> pcb
> >       }
> >       sig = fpu_emulate(frame, &pcb->pcb_fpu);
> >  #endif
> > +     if (sig == SIGILL) {
> > +             if (pcb->pcb_lastill != frame->srr0) {
> > +                     /* Allow a second chance, in case of cache sync
> issues. */
> > +                     sig = 0;
> > +                     pmap_sync_icache(PCPU_GET(curpmap), frame->srr0,
> 4);
> > +                     pcb->pcb_lastill = frame->srr0;
> > +             }
> > +     }
> >
> >       return (sig);
> >  }
> >
>
>