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

[Qemu-devel] [PATCH 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)

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 | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 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

[Qemu-devel] [PATCH 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

[Qemu-devel] [PATCH 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

[Qemu-devel] [PATCH 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

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

[Laurent Vivier]

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 | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index cf5d8dd..c83d902 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -725,7 +725,10 @@ 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));
+        tcg_gen_subi_i32(tmp, reg,
+                         reg0 == 7 && opsize == OS_BYTE
+                         ? 2
+                         : opsize_bytes(opsize));
         return tmp;
     case 5: /* Indirect displacement.  */
         reg = get_areg(s, reg0);
@@ -801,7 +804,10 @@ 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));
+            tcg_gen_addi_i32(tmp, reg,
+                             reg0 == 7 && opsize == OS_BYTE
+                             ? 2
+                             : opsize_bytes(opsize));
             delay_set_areg(s, reg0, tmp, true);
         }
         return result;
-- 
2.7.4

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

[Thomas Huth]

On 12.01.2017 21:18, Laurent Vivier wrote:
> 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.

I think this is only valid for the full 680x0 CPUs. According to
http://www.nxp.com/assets/documents/data/en/reference-manuals/CFPRM.pdf
the stack pointer behaves differently on ColdFire:

"2.2.5 Address Register Indirect with Predecrement Mode [...]
Note that the stack pointer (A7) is treated just like the other address
registers."

 Thomas

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

[Laurent Vivier]

Le 12/01/2017 à 22:14, Thomas Huth a écrit :
> On 12.01.2017 21:18, Laurent Vivier wrote:
>> 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.
> 
> I think this is only valid for the full 680x0 CPUs. According to
> http://www.nxp.com/assets/documents/data/en/reference-manuals/CFPRM.pdf
> the stack pointer behaves differently on ColdFire:
> 
> "2.2.5 Address Register Indirect with Predecrement Mode [...]
> Note that the stack pointer (A7) is treated just like the other address
> registers."

Yes, you're right. This is true only for 680x0:

"MOTOROLA M68000 FAMILY Programmer’s Reference Manual"

"2.2.5 Address Register Indirect with Predecrement Mode
...
If the address register is thestack pointer and the operand size is
byte, the address is decremented by two to keep the stack pointer
aligned to a word boundary."

Thank you, I will update this patch accordingly.

Laurent