Re: [PATCH 1/2] powerpc/sstep: Add emulation support for‘setb’ instruction
From: Daniel Axtens <hidden>
Date: 2021-04-16 07:45:31
Sathvika Vasireddy [off-list ref] writes:
quoted hunk ↗ jump to hunk
This adds emulation support for the following instruction: * Set Boolean (setb) Signed-off-by: Sathvika Vasireddy <redacted> --- 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 <redacted>
Kind regards,
Daniel
+ goto compute_done; + case 154: /* prtyw */ do_prty(regs, op, regs->gpr[rd], 32); goto logical_done_nocc; -- 2.16.4