Sathvika Vasireddy <sathv...@linux.vnet.ibm.com> writes:
> This adds emulation support for the following instruction:
> * Set Boolean (setb)
>
> Signed-off-by: Sathvika Vasireddy <sathv...@linux.vnet.ibm.com>
> ---
> arch/powerpc/lib/sstep.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index c6aebc149d14..263c613d7490 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1964,6 +1964,18 @@ int analyse_instr(struct instruction_op *op, const
> struct pt_regs *regs,
> op->val = ~(regs->gpr[rd] | regs->gpr[rb]);
> goto logical_done;
>
> + case 128: /* setb */
> + if (!cpu_has_feature(CPU_FTR_ARCH_300))
> + goto unknown_opcode;
Ok, if I've understood correctly...
> + ra = ra & ~0x3;
This masks off the bits of RA that are not part of BTF:
ra is in [0, 31] which is [0b00000, 0b11111]
Then ~0x3 = ~0b00011
ra = ra & 0b11100
This gives us then,
ra = btf << 2; or
btf = ra >> 2;
Let's then check to see if your calculations read the right fields.
> + if ((regs->ccr) & (1 << (31 - ra)))
> + op->val = -1;
> + else if ((regs->ccr) & (1 << (30 - ra)))
> + op->val = 1;
> + else
> + op->val = 0;
CR field: 7 6 5 4 3 2 1 0
bit: 0123 0123 0123 0123 0123 0123 0123 0123
normal bit #: 0.....................................31
ibm bit #: 31.....................................0
If btf = 0, ra = 0, check normal bits 31 and 30, which are both in CR0.
CR field: 7 6 5 4 3 2 1 0
bit: 0123 0123 0123 0123 0123 0123 0123 0123
^^
If btf = 7, ra = 0b11100 = 28, so check normal bits 31-28 and 30-28,
which are 3 and 2.
CR field: 7 6 5 4 3 2 1 0
bit: 0123 0123 0123 0123 0123 0123 0123 0123
^^
If btf = 3, ra = 0b01100 = 12, for normal bits 19 and 18:
CR field: 7 6 5 4 3 2 1 0
bit: 0123 0123 0123 0123 0123 0123 0123 0123
^^
So yes, your calculations, while I struggle to follow _how_ they work,
do in fact seem to work.
Checkpatch does have one complaint:
CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'regs->ccr'
#30: FILE: arch/powerpc/lib/sstep.c:1971:
+ if ((regs->ccr) & (1 << (31 - ra)))
I don't really mind the parenteses: I think you are safe to ignore
checkpatch here unless someone else complains :)
If you do end up respinning the patch, I think it would be good to make
the maths a bit clearer. I think it works because a left shift of 2 is
the same as multiplying by 4, but it would be easier to follow if you
used a temporary variable for btf.
However, I do think this is still worth adding to the kernel either way,
so:
Reviewed-by: Daniel Axtens <d...@axtens.net>
Kind regards,
Daniel
> + goto compute_done;
> +
> case 154: /* prtyw */
> do_prty(regs, op, regs->gpr[rd], 32);
> goto logical_done_nocc;
> --
> 2.16.4
