| Message ID | 15e572f61bb5c74b01210b9245ce2750a31d6002.1535609090.git.sandipan@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc sstep: Extend instruction emulation support | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | warning | Test checkpatch on branch next |
| snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
| snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
| snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
On Mon, Sep 03, 2018 at 08:49:38PM +0530, Sandipan Das wrote: > +#ifdef __powerpc64__ > + case 265: /* modud */ > + if (!cpu_has_feature(CPU_FTR_ARCH_300)) > + return -1; > + op->val = regs->gpr[ra] % regs->gpr[rb]; > + goto compute_done; > +#endif The mod instruction has special cases that aren't handled by this C code, too (divide by 0, or signed division of the most negative number by -1). For the mod intruction the behaviour is undefined in those cases, but you probably should force some specific behaviour. You don't want the kernel to execute a trap instruction, etc. :-) Segher
Hi Segher, On Wednesday 05 September 2018 02:51 AM, Segher Boessenkool wrote: > On Mon, Sep 03, 2018 at 08:49:38PM +0530, Sandipan Das wrote: >> +#ifdef __powerpc64__ >> + case 265: /* modud */ >> + if (!cpu_has_feature(CPU_FTR_ARCH_300)) >> + return -1; >> + op->val = regs->gpr[ra] % regs->gpr[rb]; >> + goto compute_done; >> +#endif > > The mod instruction has special cases that aren't handled by this C code, > too (divide by 0, or signed division of the most negative number by -1). > For the mod intruction the behaviour is undefined in those cases, but you > probably should force some specific behaviour. You don't want the kernel > to execute a trap instruction, etc. :-) > Agreed. In that case, the same would apply to the divw, divwu, divd and divdu instructions as well, right? Cause I don't see these cases being handled for them currently. Also, if I execute a modulo or division instruction for any of these special cases in a userspace binary, I don't see any exceptions being generated. It's just that the result is undefined (usually same as one of the source operands, I don't remember if it was the dividend or the divisor). So, I'm wondering if this would be necessary. - Sandipan
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c index ddc2de28e00a..b3b508d175fb 100644 --- a/arch/powerpc/lib/sstep.c +++ b/arch/powerpc/lib/sstep.c @@ -1704,7 +1704,13 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs, (int) regs->gpr[rb]; goto arith_done; - +#ifdef __powerpc64__ + case 265: /* modud */ + if (!cpu_has_feature(CPU_FTR_ARCH_300)) + return -1; + op->val = regs->gpr[ra] % regs->gpr[rb]; + goto compute_done; +#endif case 266: /* add */ op->val = regs->gpr[ra] + regs->gpr[rb]; goto arith_done; @@ -1753,7 +1759,14 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs, } return -1; - +#ifdef __powerpc64__ + case 777: /* modsd */ + if (!cpu_has_feature(CPU_FTR_ARCH_300)) + return -1; + op->val = (long int) regs->gpr[ra] % + (long int) regs->gpr[rb]; + goto compute_done; +#endif case 779: /* modsw */ if (!cpu_has_feature(CPU_FTR_ARCH_300)) return -1;
This adds emulation support for the following integer instructions: * Modulo Signed Doubleword (modsd) * Modulo Unsigned Doubleword (modud) Signed-off-by: Sandipan Das <sandipan@linux.ibm.com> --- arch/powerpc/lib/sstep.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)