[PATCH] [POWERPC] Emulate isel (Integer Select) instruction

[Kumar Gala]

On Nov 21, 2007, at 7:09 AM, Geert Uytterhoeven wrote:

> On Tue, 20 Nov 2007, Kumar Gala wrote:
>> On Nov 20, 2007, at 11:54 AM, Scott Wood wrote:
>>> On Mon, Nov 19, 2007 at 09:36:57PM -0600, Kumar Gala wrote:
>>>> isel (Integer Select) is a new user space instruction in the
>>>> PowerISA 2.04 spec.  Not all processors implement it so lets  
>>>> emulate
>>>> to ensure code built with isel will run everywhere.
>>>
>>> Given that the instruction is meant to be a performance enhancement,
>>> we should probably warn the first few times it's emulated, so the  
>>> user
>>> knows they should change their toolchain setup if possible.
>>
>> The same is true of mcrxr, popcntb, and possibly string ld/st.
>>
>> Feel free to submit a patch that warns about their usage.
>
> Something like this?
>
> Probably we also want it for:
>
>  - arch/powerpc/kernel/align.c
>      o emulate_dcbz()
>      o emulate_multiple()
>      o emulate_fp_pair()
>      o emulate_spe()
>
>  - arch/powerpc/kernel/softemu8xx.c
>      o Soft_emulate_8xx()
>
>  - arch/powerpc/kernel/traps.c
>      o SoftwareEmulation()

You missed math_emu.

>   - arch/powerpc/kernel/vecemu.c
>      o emulate_altivec()

I'm not sure I would concern this one emulation, there isn't much you  
can do about the denorm fixup.

How about some per processor counters in sysfs under the processor.

> Question: do we want it for emulate_single_step(), too?

What do you mean, we should could the emulation, the emulate single  
step just is for handling if you are doing debug while hitting an  
emulated insn.

> So far my Debian userland didn't trigger any of them on the PS3, I  
> had to
> write an explicit test ;-)
> ---
> arch/powerpc/kernel/traps.c |   19 +++++++++++++++++--
> 1 files changed, 17 insertions(+), 2 deletions(-)
>
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -707,6 +707,14 @@ static int emulate_popcntb_inst(struct p
> 	return 0;
> }
>
> +#define WARN_EMULATE(type)						\
> +	do {								\
> +	    static unsigned int count;					\
> +	    if (count++ < 10)						\
> +		    pr_warning("%s used emulated %s instruction\n",	\
> +			       current->comm, type);			\
> +	} while (0)
> +
> static int emulate_instruction(struct pt_regs *regs)
> {
> 	u32 instword;
> @@ -721,31 +729,38 @@ static int emulate_instruction(struct pt
>
> 	/* Emulate the mfspr rD, PVR. */
> 	if ((instword & INST_MFSPR_PVR_MASK) == INST_MFSPR_PVR) {
> +		WARN_EMULATE("mfpvr");
> 		rd = (instword >> 21) & 0x1f;
> 		regs->gpr[rd] = mfspr(SPRN_PVR);
> 		return 0;
> 	}
>
> 	/* Emulating the dcba insn is just a no-op.  */
> -	if ((instword & INST_DCBA_MASK) == INST_DCBA)
> +	if ((instword & INST_DCBA_MASK) == INST_DCBA) {
> +		WARN_EMULATE("dcba");
> 		return 0;
> +	}
>
> 	/* Emulate the mcrxr insn.  */
> 	if ((instword & INST_MCRXR_MASK) == INST_MCRXR) {
> 		int shift = (instword >> 21) & 0x1c;
> 		unsigned long msk = 0xf0000000UL >> shift;
>
> +		WARN_EMULATE("mcrxr");
> 		regs->ccr = (regs->ccr & ~msk) | ((regs->xer >> shift) & msk);
> 		regs->xer &= ~0xf0000000UL;
> 		return 0;
> 	}
>
> 	/* Emulate load/store string insn. */
> -	if ((instword & INST_STRING_GEN_MASK) == INST_STRING)
> +	if ((instword & INST_STRING_GEN_MASK) == INST_STRING) {
> +		WARN_EMULATE("string");
> 		return emulate_string_inst(regs, instword);
> +	}
>
> 	/* Emulate the popcntb (Population Count Bytes) instruction. */
> 	if ((instword & INST_POPCNTB_MASK) == INST_POPCNTB) {
> +		WARN_EMULATE("popcntb");
> 		return emulate_popcntb_inst(regs, instword);
> 	}
>

This looks good as a start.

- k