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

[Nathan Whitehorn]

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);
>  }
>