[Qemu-devel] [PATCH v4] m68k: implement movep instruction

Pavel Dovgalyuk posted 1 patch 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180206124431.31433.91946.stgit@pasha-VirtualBox
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppc passed
Test s390x passed
target/m68k/cpu.c       |    2 ++
target/m68k/cpu.h       |    1 +
target/m68k/translate.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+)
[Qemu-devel] [PATCH v4] m68k: implement movep instruction
Posted by Pavel Dovgalyuk 6 years, 1 month ago
This patch implements movep instruction. It moves data between a data register
and alternate bytes within the address space starting at the location
specified and incrementing by two.

It was designed for the original 68000 and used in firmwares for
interfacing the 8-bit peripherals through the 16-bit data bus.
Without this patch opcode for this instruction is recognized as some bitop.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Signed-off-by: Mihail Abakumov <mikhail.abakumov@ispras.ru>

--

v4: - fixed offset calculation misprint

v3: - simplified movep function code
    - joined movep masks

v2: - disabled movep for Coldfire
    - fixed gen_store/load usage
---
 target/m68k/cpu.c       |    2 ++
 target/m68k/cpu.h       |    1 +
 target/m68k/translate.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 98919b3..461db85 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -113,6 +113,7 @@ static void m68000_cpu_initfn(Object *obj)
     m68k_set_feature(env, M68K_FEATURE_M68000);
     m68k_set_feature(env, M68K_FEATURE_USP);
     m68k_set_feature(env, M68K_FEATURE_WORD_INDEX);
+    m68k_set_feature(env, M68K_FEATURE_MOVEP);
 }
 
 static void m68020_cpu_initfn(Object *obj)
@@ -135,6 +136,7 @@ static void m68020_cpu_initfn(Object *obj)
     m68k_set_feature(env, M68K_FEATURE_BKPT);
     m68k_set_feature(env, M68K_FEATURE_RTD);
     m68k_set_feature(env, M68K_FEATURE_CHK2);
+    m68k_set_feature(env, M68K_FEATURE_MOVEP);
 }
 #define m68030_cpu_initfn m68020_cpu_initfn
 
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 627fb78..1d79885 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -492,6 +492,7 @@ enum m68k_features {
     M68K_FEATURE_RTD,
     M68K_FEATURE_CHK2,
     M68K_FEATURE_M68040, /* instructions specific to MC68040 */
+    M68K_FEATURE_MOVEP,
 };
 
 static inline int m68k_feature(CPUM68KState *env, int feature)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 34db97b..70c7583 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2078,6 +2078,51 @@ DISAS_INSN(movem)
     tcg_temp_free(addr);
 }
 
+DISAS_INSN(movep)
+{
+    uint8_t i;
+    int16_t displ;
+    TCGv reg;
+    TCGv addr;
+    TCGv abuf;
+    TCGv dbuf;
+
+    displ = read_im16(env, s);
+
+    addr = AREG(insn, 0);
+    reg = DREG(insn, 9);
+
+    abuf = tcg_temp_new();
+    tcg_gen_addi_i32(abuf, addr, displ);
+    dbuf = tcg_temp_new();
+
+    if (insn & 0x40) {
+        i = 4;
+    } else {
+        i = 2;
+    }
+
+    if (insn & 0x80) {
+        for ( ; i > 0 ; i--) {
+            tcg_gen_shri_i32(dbuf, reg, (i - 1) * 8);
+            tcg_gen_qemu_st8(dbuf, abuf, IS_USER(s));
+            if (i > 1) {
+                tcg_gen_addi_i32(abuf, abuf, 2);
+            }
+        }
+    } else {
+        for ( ; i > 0 ; i--) {
+            tcg_gen_qemu_ld8u(dbuf, abuf, IS_USER(s));
+            tcg_gen_deposit_i32(reg, reg, dbuf, (i - 1) * 8, 8);
+            if (i > 1) {
+                tcg_gen_addi_i32(abuf, abuf, 2);
+            }
+        }
+    }
+    tcg_temp_free(abuf);
+    tcg_temp_free(dbuf);
+}
+
 DISAS_INSN(bitop_im)
 {
     int opsize;
@@ -5678,6 +5723,7 @@ void register_m68k_insns (CPUM68KState *env)
     BASE(bitop_reg, 0140, f1c0);
     BASE(bitop_reg, 0180, f1c0);
     BASE(bitop_reg, 01c0, f1c0);
+    INSN(movep,     0108, f138, MOVEP);
     INSN(arith_im,  0280, fff8, CF_ISA_A);
     INSN(arith_im,  0200, ff00, M68000);
     INSN(undef,     02c0, ffc0, M68000);


Re: [Qemu-devel] [PATCH v4] m68k: implement movep instruction
Posted by Laurent Vivier 6 years, 1 month ago
Le 06/02/2018 à 13:44, Pavel Dovgalyuk a écrit :
> This patch implements movep instruction. It moves data between a data register
> and alternate bytes within the address space starting at the location
> specified and incrementing by two.
> 
> It was designed for the original 68000 and used in firmwares for
> interfacing the 8-bit peripherals through the 16-bit data bus.

Did you test this change with some kind of firmware?

> Without this patch opcode for this instruction is recognized as some bitop.
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> Signed-off-by: Mihail Abakumov <mikhail.abakumov@ispras.ru>
> 
> --
> 
> v4: - fixed offset calculation misprint
> 
> v3: - simplified movep function code
>     - joined movep masks
> 
> v2: - disabled movep for Coldfire
>     - fixed gen_store/load usage
> ---
>  target/m68k/cpu.c       |    2 ++
>  target/m68k/cpu.h       |    1 +
>  target/m68k/translate.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 49 insertions(+)

Tested-by: Laurent Vivier <laurent@vivier.eu>

I've checked we have the same result on a real 68040

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v4] m68k: implement movep instruction
Posted by Pavel Dovgalyuk 6 years, 1 month ago
> From: Laurent Vivier [mailto:laurent@vivier.eu]
> Le 06/02/2018 à 13:44, Pavel Dovgalyuk a écrit :
> > This patch implements movep instruction. It moves data between a data register
> > and alternate bytes within the address space starting at the location
> > specified and incrementing by two.
> >
> > It was designed for the original 68000 and used in firmwares for
> > interfacing the 8-bit peripherals through the 16-bit data bus.
> 
> Did you test this change with some kind of firmware?

Yes, we implemented this instruction when tried to emulate Macintosh-128k on Qemu.

> > Without this patch opcode for this instruction is recognized as some bitop.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > Signed-off-by: Mihail Abakumov <mikhail.abakumov@ispras.ru>
> >
> > --
> >
> > v4: - fixed offset calculation misprint
> >
> > v3: - simplified movep function code
> >     - joined movep masks
> >
> > v2: - disabled movep for Coldfire
> >     - fixed gen_store/load usage
> > ---
> >  target/m68k/cpu.c       |    2 ++
> >  target/m68k/cpu.h       |    1 +
> >  target/m68k/translate.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 49 insertions(+)
> 
> Tested-by: Laurent Vivier <laurent@vivier.eu>
> 
> I've checked we have the same result on a real 68040
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks!

By the way, we also handled reset interrupt, but it is not compatible with other m68k platforms:

@@ -66,8 +66,9 @@ static void m68k_cpu_reset(CPUState *s)
     cpu_m68k_set_fpcr(env, 0);
     env->fpsr = 0;
 
-    /* TODO: We should set PC from the interrupt vector.  */
-    env->pc = 0;
+    env->vbr = 0;
+    /* PC and SP (for m68k) will be initialized by the reset handler */
+    s->exception_index = EXCP_RESET;
 }

@@ -378,6 +380,8 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         cpu_m68k_set_sr(env, sr &= ~SR_M);
         sp = env->aregs[7] & ~1;
         do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
+    } else if (cs->exception_index == EXCP_RESET) {
+        sp = cpu_ldl_kernel(env, env->vbr + vector - 4);
     } else {
         do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
     }

Pavel Dovgalyuk


Re: [Qemu-devel] [PATCH v4] m68k: implement movep instruction
Posted by Laurent Vivier 6 years, 1 month ago
Le 06/02/2018 à 14:30, Pavel Dovgalyuk a écrit :
>> From: Laurent Vivier [mailto:laurent@vivier.eu]
>> Le 06/02/2018 à 13:44, Pavel Dovgalyuk a écrit :
>>> This patch implements movep instruction. It moves data between a data register
>>> and alternate bytes within the address space starting at the location
>>> specified and incrementing by two.
>>>
>>> It was designed for the original 68000 and used in firmwares for
>>> interfacing the 8-bit peripherals through the 16-bit data bus.
>>
>> Did you test this change with some kind of firmware?
> 
> Yes, we implemented this instruction when tried to emulate Macintosh-128k on Qemu.
> 
>>> Without this patch opcode for this instruction is recognized as some bitop.
>>>
>>> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>>> Signed-off-by: Mihail Abakumov <mikhail.abakumov@ispras.ru>
>>>
>>> --
>>>
>>> v4: - fixed offset calculation misprint
>>>
>>> v3: - simplified movep function code
>>>     - joined movep masks
>>>
>>> v2: - disabled movep for Coldfire
>>>     - fixed gen_store/load usage
>>> ---
>>>  target/m68k/cpu.c       |    2 ++
>>>  target/m68k/cpu.h       |    1 +
>>>  target/m68k/translate.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 49 insertions(+)
>>
>> Tested-by: Laurent Vivier <laurent@vivier.eu>
>>
>> I've checked we have the same result on a real 68040
>>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> Thanks!
> 
> By the way, we also handled reset interrupt, but it is not compatible with other m68k platforms:
> 
> @@ -66,8 +66,9 @@ static void m68k_cpu_reset(CPUState *s)
>      cpu_m68k_set_fpcr(env, 0);
>      env->fpsr = 0;
>  
> -    /* TODO: We should set PC from the interrupt vector.  */
> -    env->pc = 0;
> +    env->vbr = 0;
> +    /* PC and SP (for m68k) will be initialized by the reset handler */
> +    s->exception_index = EXCP_RESET;
>  }
> 
> @@ -378,6 +380,8 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>          cpu_m68k_set_sr(env, sr &= ~SR_M);
>          sp = env->aregs[7] & ~1;
>          do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> +    } else if (cs->exception_index == EXCP_RESET) {
> +        sp = cpu_ldl_kernel(env, env->vbr + vector - 4);
>      } else {
>          do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
>      }

It looks better of what I have already coded :)

Do you work using code in
https://github.com/vivier/qemu-m68k , branch q800-dev ?

I'm already emulating a Quadra 800, it can help for Macintosh-128k

Thanks,
Laurent


Re: [Qemu-devel] [PATCH v4] m68k: implement movep instruction
Posted by Pavel Dovgalyuk 6 years, 1 month ago
> From: Laurent Vivier [mailto:laurent@vivier.eu]
> Le 06/02/2018 à 14:30, Pavel Dovgalyuk a écrit :
> >> From: Laurent Vivier [mailto:laurent@vivier.eu]
> > Thanks!
> >
> > By the way, we also handled reset interrupt, but it is not compatible with other m68k
> platforms:
> >
> > @@ -66,8 +66,9 @@ static void m68k_cpu_reset(CPUState *s)
> >      cpu_m68k_set_fpcr(env, 0);
> >      env->fpsr = 0;
> >
> > -    /* TODO: We should set PC from the interrupt vector.  */
> > -    env->pc = 0;
> > +    env->vbr = 0;
> > +    /* PC and SP (for m68k) will be initialized by the reset handler */
> > +    s->exception_index = EXCP_RESET;
> >  }
> >
> > @@ -378,6 +380,8 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
> >          cpu_m68k_set_sr(env, sr &= ~SR_M);
> >          sp = env->aregs[7] & ~1;
> >          do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> > +    } else if (cs->exception_index == EXCP_RESET) {
> > +        sp = cpu_ldl_kernel(env, env->vbr + vector - 4);
> >      } else {
> >          do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> >      }
> 
> It looks better of what I have already coded :)
> 
> Do you work using code in
> https://github.com/vivier/qemu-m68k , branch q800-dev ?

No, it was a project for our students couple of years ago.
We used Qemu 2.3 with not-yet-included patches for 68000.
I believe that someday we'll port our peripherals onto the new version.

There were some fixes for processing the interrupts. As I can see, all of them are
not needed for the mainline Qemu.

We didn't find a solution for 24-bit address bus of 68000. Macintosh stores 32-bit values 
in address registers and uses them to access the memory. We just duplicated the memory layout,
but I believe that there is a better solution.

> I'm already emulating a Quadra 800, it can help for Macintosh-128k

Here is the repository with Mac-128: https://github.com/Dovgalyuk/qemu
We didn't finally fix all the bugs, but it can boot the OS, using some hacks.
One of the hack is related to IWM. We couldn't emulate all timings for that.
CPU controls disk rotation speed through controlling the strobe signal.
It was hard to synchronize this, because icount wasn't fully working and we used
semihosting - we intercepted the file operation system calls and didn't execute
ROM code, emulating them in Qemu instead.

Pavel Dovgalyuk