| Message ID | 1368007612-1873-3-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On Wed, May 08, 2013 at 12:06:52PM +0200, Alexander Graf wrote: > When running a 32bit target CPU with qemu-(system-)-ppc, NARROW_MODE > is not set, so we never get to leverage the "32bit only" code path in > the compare op handlers. > > Introduce new handlers based on the 32bit only flag. That way we can > have 2 separate functions for 32bit mode and 64bit mode, which can > handle NARROW_MODE. > > Reported-by: Torbjorn Granlund <tg@gmplib.org> > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++-------- > 1 files changed, 40 insertions(+), 8 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index a018616..002f9ae 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -675,7 +675,7 @@ static inline void gen_set_Rc0(DisasContext *ctx, TCGv reg) > /* cmp */ > static void gen_cmp(DisasContext *ctx) > { > - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { > + if (!(ctx->opcode & 0x00200000)) { > gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], > 1, crfD(ctx->opcode)); > } else { > @@ -684,10 +684,17 @@ static void gen_cmp(DisasContext *ctx) > } > } > > +/* cmp 32bit only */ > +static void gen_cmp32(DisasContext *ctx) > +{ > + gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], > + 1, crfD(ctx->opcode)); > +} > + > /* cmpi */ > static void gen_cmpi(DisasContext *ctx) > { > - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { > + if (!(ctx->opcode & 0x00200000)) { > gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), > 1, crfD(ctx->opcode)); > } else { > @@ -696,10 +703,17 @@ static void gen_cmpi(DisasContext *ctx) > } > } > > +/* cmpi 32bit only */ > +static void gen_cmpi32(DisasContext *ctx) > +{ > + gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), > + 1, crfD(ctx->opcode)); > +} > + > /* cmpl */ > static void gen_cmpl(DisasContext *ctx) > { > - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { > + if (!(ctx->opcode & 0x00200000)) { > gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], > 0, crfD(ctx->opcode)); > } else { > @@ -708,10 +722,17 @@ static void gen_cmpl(DisasContext *ctx) > } > } > > +/* cmpl 32bit only */ > +static void gen_cmpl32(DisasContext *ctx) > +{ > + gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], > + 0, crfD(ctx->opcode)); > +} > + > /* cmpli */ > static void gen_cmpli(DisasContext *ctx) > { > - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { > + if (!(ctx->opcode & 0x00200000)) { > gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode), > 0, crfD(ctx->opcode)); > } else { > @@ -720,6 +741,13 @@ static void gen_cmpli(DisasContext *ctx) > } > } > > +/* cmpli 32bit only */ > +static void gen_cmpli32(DisasContext *ctx) > +{ > + gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode), > + 0, crfD(ctx->opcode)); > +} > + > /* isel (PowerPC 2.03 specification) */ > static void gen_isel(DisasContext *ctx) > { > @@ -8638,10 +8666,14 @@ GEN_SPE(efdtsteq, speundef, 0x1F, 0x0B, 0x00600000, 0xFFFFFFFF, PPC_SPE_DOUBLE > > static opcode_t opcodes[] = { > GEN_HANDLER(invalid, 0x00, 0x00, 0x00, 0xFFFFFFFF, PPC_NONE), > -GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_INTEGER), > -GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_INTEGER), > -GEN_HANDLER(cmpl, 0x1F, 0x00, 0x01, 0x00400000, PPC_INTEGER), > -GEN_HANDLER(cmpli, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_INTEGER), > +GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_64B), > +GEN_HANDLER_E(cmp32, 0x1F, 0x00, 0x00, 0x00400000, PPC_NONE, PPC2_32B), You have to declare the L bit as invalid, so that trying to execute a 64-bit cmp* instruction on a 32-bit CPU causes an invalid instruction exception. > +GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_64B), > +GEN_HANDLER_E(cmpi32, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_NONE, PPC2_32B), > +GEN_HANDLER(cmpl, 0x1F, 0x00, 0x01, 0x00400000, PPC_64B), > +GEN_HANDLER_E(cmpl32, 0x1F, 0x00, 0x01, 0x00400000, PPC_NONE, PPC2_32B), > +GEN_HANDLER(cmpli, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_64B), > +GEN_HANDLER_E(cmpli32, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_NONE, PPC2_32B), > GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205), > GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL), > GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_INTEGER), > -- > 1.6.0.2 > > >
On 08.05.2013, at 12:29, Aurelien Jarno wrote: > On Wed, May 08, 2013 at 12:06:52PM +0200, Alexander Graf wrote: >> When running a 32bit target CPU with qemu-(system-)-ppc, NARROW_MODE >> is not set, so we never get to leverage the "32bit only" code path in >> the compare op handlers. >> >> Introduce new handlers based on the 32bit only flag. That way we can >> have 2 separate functions for 32bit mode and 64bit mode, which can >> handle NARROW_MODE. >> >> Reported-by: Torbjorn Granlund <tg@gmplib.org> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++-------- >> 1 files changed, 40 insertions(+), 8 deletions(-) >> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >> index a018616..002f9ae 100644 >> --- a/target-ppc/translate.c >> +++ b/target-ppc/translate.c >> @@ -675,7 +675,7 @@ static inline void gen_set_Rc0(DisasContext *ctx, TCGv reg) >> /* cmp */ >> static void gen_cmp(DisasContext *ctx) >> { >> - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { >> + if (!(ctx->opcode & 0x00200000)) { >> gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], >> 1, crfD(ctx->opcode)); >> } else { >> @@ -684,10 +684,17 @@ static void gen_cmp(DisasContext *ctx) >> } >> } >> >> +/* cmp 32bit only */ >> +static void gen_cmp32(DisasContext *ctx) >> +{ >> + gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], >> + 1, crfD(ctx->opcode)); >> +} >> + >> /* cmpi */ >> static void gen_cmpi(DisasContext *ctx) >> { >> - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { >> + if (!(ctx->opcode & 0x00200000)) { >> gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), >> 1, crfD(ctx->opcode)); >> } else { >> @@ -696,10 +703,17 @@ static void gen_cmpi(DisasContext *ctx) >> } >> } >> >> +/* cmpi 32bit only */ >> +static void gen_cmpi32(DisasContext *ctx) >> +{ >> + gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), >> + 1, crfD(ctx->opcode)); >> +} >> + >> /* cmpl */ >> static void gen_cmpl(DisasContext *ctx) >> { >> - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { >> + if (!(ctx->opcode & 0x00200000)) { >> gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], >> 0, crfD(ctx->opcode)); >> } else { >> @@ -708,10 +722,17 @@ static void gen_cmpl(DisasContext *ctx) >> } >> } >> >> +/* cmpl 32bit only */ >> +static void gen_cmpl32(DisasContext *ctx) >> +{ >> + gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], >> + 0, crfD(ctx->opcode)); >> +} >> + >> /* cmpli */ >> static void gen_cmpli(DisasContext *ctx) >> { >> - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { >> + if (!(ctx->opcode & 0x00200000)) { >> gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode), >> 0, crfD(ctx->opcode)); >> } else { >> @@ -720,6 +741,13 @@ static void gen_cmpli(DisasContext *ctx) >> } >> } >> >> +/* cmpli 32bit only */ >> +static void gen_cmpli32(DisasContext *ctx) >> +{ >> + gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode), >> + 0, crfD(ctx->opcode)); >> +} >> + >> /* isel (PowerPC 2.03 specification) */ >> static void gen_isel(DisasContext *ctx) >> { >> @@ -8638,10 +8666,14 @@ GEN_SPE(efdtsteq, speundef, 0x1F, 0x0B, 0x00600000, 0xFFFFFFFF, PPC_SPE_DOUBLE >> >> static opcode_t opcodes[] = { >> GEN_HANDLER(invalid, 0x00, 0x00, 0x00, 0xFFFFFFFF, PPC_NONE), >> -GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_INTEGER), >> -GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_INTEGER), >> -GEN_HANDLER(cmpl, 0x1F, 0x00, 0x01, 0x00400000, PPC_INTEGER), >> -GEN_HANDLER(cmpli, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_INTEGER), >> +GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_64B), >> +GEN_HANDLER_E(cmp32, 0x1F, 0x00, 0x00, 0x00400000, PPC_NONE, PPC2_32B), > > You have to declare the L bit as invalid, so that trying to execute a > 64-bit cmp* instruction on a 32-bit CPU causes an invalid instruction > exception. You're right. I wanted to verify it against a real 32bit system first. It does indeed treat the L bit as reserved. Then we can simply remove the 32bit only variant handlers and only use the opcode table for the reserved bits. Alex
Aurelien Jarno <aurelien@aurel32.net> writes: You have to declare the L bit as invalid, so that trying to execute a 64-bit cmp* instruction on a 32-bit CPU causes an invalid instruction exception. Don't people read what I write? I give up. Goodbye.
On 08.05.2013, at 12:34, Torbjorn Granlund wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > You have to declare the L bit as invalid, so that trying to execute a > 64-bit cmp* instruction on a 32-bit CPU causes an invalid instruction > exception. > > Don't people read what I write? At least on my e500mc machine I happen to have handy, the following instruction gives me an illegal instruction exception: 10000320: 2f a0 00 00 cmpi cr7,1,r0,0 So for 32bit CPU types we have to mark the L bit invalid which means we always get into the branches that take us to cmp32. For 64bit capable CPUs we obviously want to honor the L bit even when running in 32bit mode (MSR.SF=0), as that's what real CPUs do. Alex
On Wed, May 08, 2013 at 12:34:26PM +0200, Torbjorn Granlund wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > You have to declare the L bit as invalid, so that trying to execute a > 64-bit cmp* instruction on a 32-bit CPU causes an invalid instruction > exception. > > Don't people read what I write? > Quoting the "IBM PowerPC Microprocessor Family: The Programming Environments Manual for 32 and 64-bit Microprocessors": | Note: In 32-bit implementations, if L = 1 the instruction form is invalid.
diff --git a/target-ppc/translate.c b/target-ppc/translate.c index a018616..002f9ae 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -675,7 +675,7 @@ static inline void gen_set_Rc0(DisasContext *ctx, TCGv reg) /* cmp */ static void gen_cmp(DisasContext *ctx) { - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { + if (!(ctx->opcode & 0x00200000)) { gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], 1, crfD(ctx->opcode)); } else { @@ -684,10 +684,17 @@ static void gen_cmp(DisasContext *ctx) } } +/* cmp 32bit only */ +static void gen_cmp32(DisasContext *ctx) +{ + gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], + 1, crfD(ctx->opcode)); +} + /* cmpi */ static void gen_cmpi(DisasContext *ctx) { - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { + if (!(ctx->opcode & 0x00200000)) { gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), 1, crfD(ctx->opcode)); } else { @@ -696,10 +703,17 @@ static void gen_cmpi(DisasContext *ctx) } } +/* cmpi 32bit only */ +static void gen_cmpi32(DisasContext *ctx) +{ + gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), + 1, crfD(ctx->opcode)); +} + /* cmpl */ static void gen_cmpl(DisasContext *ctx) { - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { + if (!(ctx->opcode & 0x00200000)) { gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], 0, crfD(ctx->opcode)); } else { @@ -708,10 +722,17 @@ static void gen_cmpl(DisasContext *ctx) } } +/* cmpl 32bit only */ +static void gen_cmpl32(DisasContext *ctx) +{ + gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], + 0, crfD(ctx->opcode)); +} + /* cmpli */ static void gen_cmpli(DisasContext *ctx) { - if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) { + if (!(ctx->opcode & 0x00200000)) { gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode), 0, crfD(ctx->opcode)); } else { @@ -720,6 +741,13 @@ static void gen_cmpli(DisasContext *ctx) } } +/* cmpli 32bit only */ +static void gen_cmpli32(DisasContext *ctx) +{ + gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode), + 0, crfD(ctx->opcode)); +} + /* isel (PowerPC 2.03 specification) */ static void gen_isel(DisasContext *ctx) { @@ -8638,10 +8666,14 @@ GEN_SPE(efdtsteq, speundef, 0x1F, 0x0B, 0x00600000, 0xFFFFFFFF, PPC_SPE_DOUBLE static opcode_t opcodes[] = { GEN_HANDLER(invalid, 0x00, 0x00, 0x00, 0xFFFFFFFF, PPC_NONE), -GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_INTEGER), -GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_INTEGER), -GEN_HANDLER(cmpl, 0x1F, 0x00, 0x01, 0x00400000, PPC_INTEGER), -GEN_HANDLER(cmpli, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_INTEGER), +GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_64B), +GEN_HANDLER_E(cmp32, 0x1F, 0x00, 0x00, 0x00400000, PPC_NONE, PPC2_32B), +GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_64B), +GEN_HANDLER_E(cmpi32, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_NONE, PPC2_32B), +GEN_HANDLER(cmpl, 0x1F, 0x00, 0x01, 0x00400000, PPC_64B), +GEN_HANDLER_E(cmpl32, 0x1F, 0x00, 0x01, 0x00400000, PPC_NONE, PPC2_32B), +GEN_HANDLER(cmpli, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_64B), +GEN_HANDLER_E(cmpli32, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_NONE, PPC2_32B), GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205), GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL), GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
When running a 32bit target CPU with qemu-(system-)-ppc, NARROW_MODE is not set, so we never get to leverage the "32bit only" code path in the compare op handlers. Introduce new handlers based on the 32bit only flag. That way we can have 2 separate functions for 32bit mode and 64bit mode, which can handle NARROW_MODE. Reported-by: Torbjorn Granlund <tg@gmplib.org> Signed-off-by: Alexander Graf <agraf@suse.de> --- target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 40 insertions(+), 8 deletions(-)