Patchwork [Qemu-ppc] Incorrect handling of more PPC64 insns (PATCH)

login
register
mail settings
Submitter Torbjorn Granlund
Date May 7, 2013, 7:30 p.m.
Message ID <86ppx2oaen.fsf@shell.gmplib.org>
Download mbox | patch
Permalink /patch/242434/
State New
Headers show

Comments

Torbjorn Granlund - May 7, 2013, 7:30 p.m.
I realised a possible problem with my suggested patch.

What about a 32-bit processor?  Then NARROW_MODE macro is identical 0.

The pre-patch behaviour was then to ignore the L bit and decode both
32-bit and 64-bit instruction in the same way.

Apparently that is correct behaviour.  (The manual is slightly vague,
but I let hardware decide.)

With my patch, the bit is not ignored, and invalid code will be
generated for 32-bit targets, if they'd set the L bit.

Here is an uglier but hopefully completely correct patch.
Alexander Graf - May 7, 2013, 10 p.m.
Am 07.05.2013 um 21:30 schrieb Torbjorn Granlund <tg@gmplib.org>:

> I realised a possible problem with my suggested patch.
> 
> What about a 32-bit processor?  Then NARROW_MODE macro is identical 0.
> 
> The pre-patch behaviour was then to ignore the L bit and decode both
> 32-bit and 64-bit instruction in the same way.
> 
> Apparently that is correct behaviour.  (The manual is slightly vague,
> but I let hardware decide.)
> 
> With my patch, the bit is not ignored, and invalid code will be
> generated for 32-bit targets, if they'd set the L bit.
> 
> Here is an uglier but hopefully completely correct patch.
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 1a84653..69d684c 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -675,49 +675,65 @@ 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 defined(TARGET_PPC64)
> +    if (!(ctx->opcode & 0x00200000)) {

The ppc64 target can also execute as ppc32 CPU if you pass in the correct -cpu value. So this one looks slightly bogus...

Alex

> +#endif
>         gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                      1, crfD(ctx->opcode));
> +#if defined(TARGET_PPC64)
>     } else {
>         gen_op_cmp(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                    1, crfD(ctx->opcode));
>     }
> +#endif
> }
> 
> /* cmpi */
> static void gen_cmpi(DisasContext *ctx)
> {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> +#if defined(TARGET_PPC64)
> +    if (!(ctx->opcode & 0x00200000)) {
> +#endif
>         gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
>                       1, crfD(ctx->opcode));
> +#if defined(TARGET_PPC64)
>     } else {
>         gen_op_cmpi(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
>                     1, crfD(ctx->opcode));
>     }
> +#endif
> }
> 
> /* cmpl */
> static void gen_cmpl(DisasContext *ctx)
> {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> +#if defined(TARGET_PPC64)
> +    if (!(ctx->opcode & 0x00200000)) {
> +#endif
>         gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                      0, crfD(ctx->opcode));
> +#if defined(TARGET_PPC64)
>     } else {
>         gen_op_cmp(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                    0, crfD(ctx->opcode));
>     }
> +#endif
> }
> 
> /* cmpli */
> static void gen_cmpli(DisasContext *ctx)
> {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> +#if defined(TARGET_PPC64)
> +    if (!(ctx->opcode & 0x00200000)) {
> +#endif
>         gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
>                       0, crfD(ctx->opcode));
> +#if defined(TARGET_PPC64)
>     } else {
>         gen_op_cmpi(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
>                     0, crfD(ctx->opcode));
>     }
> +#endif
> }
> 
> /* isel (PowerPC 2.03 specification) */
> 
> -- 
> Torbjörn

Patch

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 1a84653..69d684c 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -675,49 +675,65 @@  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 defined(TARGET_PPC64)
+    if (!(ctx->opcode & 0x00200000)) {
+#endif
         gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
                      1, crfD(ctx->opcode));
+#if defined(TARGET_PPC64)
     } else {
         gen_op_cmp(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
                    1, crfD(ctx->opcode));
     }
+#endif
 }
 
 /* cmpi */
 static void gen_cmpi(DisasContext *ctx)
 {
-    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
+#if defined(TARGET_PPC64)
+    if (!(ctx->opcode & 0x00200000)) {
+#endif
         gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
                       1, crfD(ctx->opcode));
+#if defined(TARGET_PPC64)
     } else {
         gen_op_cmpi(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
                     1, crfD(ctx->opcode));
     }
+#endif
 }
 
 /* cmpl */
 static void gen_cmpl(DisasContext *ctx)
 {
-    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
+#if defined(TARGET_PPC64)
+    if (!(ctx->opcode & 0x00200000)) {
+#endif
         gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
                      0, crfD(ctx->opcode));
+#if defined(TARGET_PPC64)
     } else {
         gen_op_cmp(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
                    0, crfD(ctx->opcode));
     }
+#endif
 }
 
 /* cmpli */
 static void gen_cmpli(DisasContext *ctx)
 {
-    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
+#if defined(TARGET_PPC64)
+    if (!(ctx->opcode & 0x00200000)) {
+#endif
         gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
                       0, crfD(ctx->opcode));
+#if defined(TARGET_PPC64)
     } else {
         gen_op_cmpi(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
                     0, crfD(ctx->opcode));
     }
+#endif
 }
 
 /* isel (PowerPC 2.03 specification) */