[Qemu-devel] [PATCH v2 0/5] Fixes for target/m68k

[Qemu-devel] [PATCH v2 0/5] Fixes for target/m68k

[Laurent Vivier]

This is a series of fixes for target/m68k found:
- with RISU (bit operation with immediate)
- while debugging package build under chroot
  (gen_flush_flags() and CAS address modes)
- while I was working on the softmmu mode
  (CAS alignment and SP address modes)

v2:
- Don't align stack access on coldfire.

Laurent Vivier (5):
  target-m68k: fix bit operation with immediate value
  target-m68k: fix gen_flush_flags()
  target-m68k: manage pre-dec et post-inc in CAS
  target-m68k: CAS doesn't need aligned access
  target-m68k: increment/decrement with SP

 target/m68k/translate.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v2 1/5] target-m68k: fix bit operation with immediate value

[Laurent Vivier]

M680x0 bit operations with an immediate value use 9 bits of the 16bit
value, while coldfire ones use only 8 bits.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target/m68k/translate.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 5f7357e..410f56a 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1801,9 +1801,16 @@ DISAS_INSN(bitop_im)
     op = (insn >> 6) & 3;
 
     bitnum = read_im16(env, s);
-    if (bitnum & 0xff00) {
-        disas_undef(env, s, insn);
-        return;
+    if (m68k_feature(s->env, M68K_FEATURE_M68000)) {
+        if (bitnum & 0xfe00) {
+            disas_undef(env, s, insn);
+            return;
+        }
+    } else {
+        if (bitnum & 0xff00) {
+            disas_undef(env, s, insn);
+            return;
+        }
     }
 
     SRC_EA(env, src1, opsize, 0, op ? &addr: NULL);
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 1/5] target-m68k: fix bit operation with immediate value

[Richard Henderson]

On 01/13/2017 04:51 AM, Laurent Vivier wrote:
> M680x0 bit operations with an immediate value use 9 bits of the 16bit
> value, while coldfire ones use only 8 bits.

I don't see that in the reference manual.  Where do you get this from?


r~

Re: [Qemu-devel] [PATCH v2 1/5] target-m68k: fix bit operation with immediate value

[Laurent Vivier]

Le 13/01/2017 à 18:55, Richard Henderson a écrit :
> On 01/13/2017 04:51 AM, Laurent Vivier wrote:
>> M680x0 bit operations with an immediate value use 9 bits of the 16bit
>> value, while coldfire ones use only 8 bits.
> 
> I don't see that in the reference manual.  Where do you get this from?

See "BSET Instruction Format:BIT NUMBER STATIC, SPECIFIED AS IMMEDIATE
DATA", p. 4.58 of "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL", "BIT
NUMBER" is bits 0 to 8 of extended word.

See "Test a Bit and Set", p. 4-22 of "ColdFire Family Programmer’s
Reference Manual, Rev. 3" (link given by Thomas), "Bit Number" is bits 0
to 7 of extended word.

I've found the problem with RISU.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 1/5] target-m68k: fix bit operation with immediate value

[Richard Henderson]

On 01/13/2017 10:23 AM, Laurent Vivier wrote:
> See "BSET Instruction Format:BIT NUMBER STATIC, SPECIFIED AS IMMEDIATE
> DATA", p. 4.58 of "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL", "BIT
> NUMBER" is bits 0 to 8 of extended word.

Oops, yep.  I misread that.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH v2 2/5] target-m68k: fix gen_flush_flags()

[Laurent Vivier]

gen_flush_flags() is setting unconditionally cc_op_synced to 1
and s->cc_op to CC_OP_FLAGS, whereas env->cc_op can be set
to something else by a previous tcg fragment.

We fix that by not setting cc_op_synced to 1
(except for gen_helper_flush_flags() that updates env->cc_op)

FIX: https://github.com/vivier/qemu-m68k/issues/19

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target/m68k/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 410f56a..0e97900 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -595,18 +595,19 @@ static void gen_flush_flags(DisasContext *s)
 
     case CC_OP_DYNAMIC:
         gen_helper_flush_flags(cpu_env, QREG_CC_OP);
+        s->cc_op_synced = 1;
         break;
 
     default:
         t0 = tcg_const_i32(s->cc_op);
         gen_helper_flush_flags(cpu_env, t0);
         tcg_temp_free(t0);
+        s->cc_op_synced = 1;
         break;
     }
 
     /* Note that flush_flags also assigned to env->cc_op.  */
     s->cc_op = CC_OP_FLAGS;
-    s->cc_op_synced = 1;
 }
 
 static inline TCGv gen_extend(TCGv val, int opsize, int sign)
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 2/5] target-m68k: fix gen_flush_flags()

[Richard Henderson]

On 01/13/2017 04:52 AM, Laurent Vivier wrote:
> gen_flush_flags() is setting unconditionally cc_op_synced to 1
> and s->cc_op to CC_OP_FLAGS, whereas env->cc_op can be set
> to something else by a previous tcg fragment.
> 
> We fix that by not setting cc_op_synced to 1
> (except for gen_helper_flush_flags() that updates env->cc_op)
> 
> FIX: https://github.com/vivier/qemu-m68k/issues/19
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  target/m68k/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH v2 3/5] target-m68k: manage pre-dec et post-inc in CAS

[Laurent Vivier]

In these cases we must update the address register after
the operation.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target/m68k/translate.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 0e97900..23e2b06 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1963,6 +1963,15 @@ DISAS_INSN(cas)
     gen_partset_reg(opsize, DREG(ext, 0), load);
 
     tcg_temp_free(load);
+
+    switch (extract32(insn, 3, 3)) {
+    case 3: /* Indirect postincrement.  */
+        tcg_gen_addi_i32(AREG(insn, 0), addr, opsize_bytes(opsize));
+        break;
+    case 4: /* Indirect predecrememnt.  */
+        tcg_gen_mov_i32(AREG(insn, 0), addr);
+        break;
+    }
 }
 
 DISAS_INSN(cas2w)
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 3/5] target-m68k: manage pre-dec et post-inc in CAS

[Richard Henderson]

On 01/13/2017 04:52 AM, Laurent Vivier wrote:
> In these cases we must update the address register after
> the operation.
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  target/m68k/translate.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH v2 4/5] target-m68k: CAS doesn't need aligned access

[Laurent Vivier]

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target/m68k/translate.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 23e2b06..cf5d8dd 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1934,7 +1934,6 @@ DISAS_INSN(cas)
     default:
         g_assert_not_reached();
     }
-    opc |= MO_ALIGN;
 
     ext = read_im16(env, s);
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 4/5] target-m68k: CAS doesn't need aligned access

[Richard Henderson]

On 01/13/2017 04:52 AM, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  target/m68k/translate.c | 1 -
>  1 file changed, 1 deletion(-)

Huh.  Wow.  Ok.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH v2 5/5] target-m68k: increment/decrement with SP

[Laurent Vivier]

On 680x0 family only.

Address Register indirect With postincrement:

When using the stack pointer (A7) with byte size data, the register
is incremented by two.

Address Register indirect With predecrement:

When using the stack pointer (A7) with byte size data, the register
is decremented by two.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target/m68k/translate.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index cf5d8dd..727c189 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -725,7 +725,12 @@ static TCGv gen_lea_mode(CPUM68KState *env, DisasContext *s,
         }
         reg = get_areg(s, reg0);
         tmp = tcg_temp_new();
-        tcg_gen_subi_i32(tmp, reg, opsize_bytes(opsize));
+        if (reg0 == 7 && opsize == OS_BYTE &&
+            m68k_feature(s->env, M68K_FEATURE_M68000)) {
+            tcg_gen_subi_i32(tmp, reg, 2);
+        } else {
+            tcg_gen_subi_i32(tmp, reg, opsize_bytes(opsize));
+        }
         return tmp;
     case 5: /* Indirect displacement.  */
         reg = get_areg(s, reg0);
@@ -801,7 +806,12 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0,
         result = gen_ldst(s, opsize, reg, val, what);
         if (what == EA_STORE || !addrp) {
             TCGv tmp = tcg_temp_new();
-            tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
+            if (reg0 == 7 && opsize == OS_BYTE &&
+                m68k_feature(s->env, M68K_FEATURE_M68000)) {
+                tcg_gen_subi_i32(tmp, reg, 2);
+            } else {
+                tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
+            }
             delay_set_areg(s, reg0, tmp, true);
         }
         return result;
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 5/5] target-m68k: increment/decrement with SP

[Thomas Huth]

On 13.01.2017 13:52, Laurent Vivier wrote:
> On 680x0 family only.
> 
> Address Register indirect With postincrement:
> 
> When using the stack pointer (A7) with byte size data, the register
> is incremented by two.
> 
> Address Register indirect With predecrement:
> 
> When using the stack pointer (A7) with byte size data, the register
> is decremented by two.
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  target/m68k/translate.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index cf5d8dd..727c189 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -725,7 +725,12 @@ static TCGv gen_lea_mode(CPUM68KState *env, DisasContext *s,
>          }
>          reg = get_areg(s, reg0);
>          tmp = tcg_temp_new();
> -        tcg_gen_subi_i32(tmp, reg, opsize_bytes(opsize));
> +        if (reg0 == 7 && opsize == OS_BYTE &&
> +            m68k_feature(s->env, M68K_FEATURE_M68000)) {
> +            tcg_gen_subi_i32(tmp, reg, 2);
> +        } else {
> +            tcg_gen_subi_i32(tmp, reg, opsize_bytes(opsize));
> +        }
>          return tmp;
>      case 5: /* Indirect displacement.  */
>          reg = get_areg(s, reg0);
> @@ -801,7 +806,12 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0,
>          result = gen_ldst(s, opsize, reg, val, what);
>          if (what == EA_STORE || !addrp) {
>              TCGv tmp = tcg_temp_new();
> -            tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
> +            if (reg0 == 7 && opsize == OS_BYTE &&
> +                m68k_feature(s->env, M68K_FEATURE_M68000)) {
> +                tcg_gen_subi_i32(tmp, reg, 2);
> +            } else {
> +                tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
> +            }
>              delay_set_areg(s, reg0, tmp, true);
>          }
>          return result;
> 

Reviewed-by: Thomas Huth <huth@tuxfamily.org>

Re: [Qemu-devel] [PATCH v2 5/5] target-m68k: increment/decrement with SP

[Richard Henderson]

On 01/13/2017 04:52 AM, Laurent Vivier wrote:
> +            if (reg0 == 7 && opsize == OS_BYTE &&
> +                m68k_feature(s->env, M68K_FEATURE_M68000)) {
> +                tcg_gen_subi_i32(tmp, reg, 2);
> +            } else {
> +                tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
> +            }

post-increment add.


r~