Subject: Re: [PATCH 1/2] powerpc: Fix emulation of mcrf in emulate_step()

From: Segher Boessenkool <[email-masked]>
Date: Wed, 14 Jun 2017 23:18:05 -0500

Hi Anton,

On Thu, Jun 15, 2017 at 09:46:38AM +1000, Anton Blanchard wrote:
> The mcrf emulation code was looking at the CR fields in the reverse
> order. It also relied on reserved fields being zero which is somewhat
> fragile, so fix that too.

It masked out the reserved bits.  I find the new code to be less
readable (but also more correct ;-) ).  Maybe there should be (inline)
helper function to insert/extract CR fields?

Segher

> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -683,8 +683,10 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
>  	case 19:
>  		switch ((instr >> 1) & 0x3ff) {
>  		case 0:		/* mcrf */
> -			rd = (instr >> 21) & 0x1c;
> -			ra = (instr >> 16) & 0x1c;
> +			rd = 7 - ((instr >> 23) & 0x7);
> +			ra = 7 - ((instr >> 18) & 0x7);
> +			rd *= 4;
> +			ra *= 4;
>  			val = (regs->ccr >> ra) & 0xf;
>  			regs->ccr = (regs->ccr & ~(0xfUL << rd)) | (val << rd);
>  			goto instr_done;