| Message ID | 1462396869-22424-12-git-send-email-laurent@vivier.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/04/2016 11:21 AM, Laurent Vivier wrote: > + reg = AREG(insn, 0); > + src = gen_load(s, opsize, reg, 1); > + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); > + > + reg = AREG(insn, 9); > + dest = gen_load(s, opsize, reg, 1); > + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); Delay the writeback to the first areg until after the second load. r~
Le 07/05/2016 à 00:00, Richard Henderson a écrit : > On 05/04/2016 11:21 AM, Laurent Vivier wrote: >> + reg = AREG(insn, 0); >> + src = gen_load(s, opsize, reg, 1); >> + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); >> + >> + reg = AREG(insn, 9); >> + dest = gen_load(s, opsize, reg, 1); >> + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); > > Delay the writeback to the first areg until after the second load. We can't delay because we can have "cmpm (%a0)+,(%a0)+" that is used to compare two consecutive memory contents. Laurent
On 7 May 2016 at 20:01, Laurent Vivier <laurent@vivier.eu> wrote: > > > Le 07/05/2016 à 00:00, Richard Henderson a écrit : >> On 05/04/2016 11:21 AM, Laurent Vivier wrote: >>> + reg = AREG(insn, 0); >>> + src = gen_load(s, opsize, reg, 1); >>> + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); >>> + >>> + reg = AREG(insn, 9); >>> + dest = gen_load(s, opsize, reg, 1); >>> + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); >> >> Delay the writeback to the first areg until after the second load. > > We can't delay because we can have "cmpm (%a0)+,(%a0)+" that is used to > compare two consecutive memory contents. If you write back to the first areg before the second load, don't you get the wrong value as seen by the exception handler if the second load faults? Usually you want to use the updated value for the purposes of calculating the address to use in the second load, but you don't want to actually update the guest CPU register until after the load has happened, in case it faults. (Disclaimer: I'm just assuming that on a fault no registers are updated, but if that wasn't the case the OS wouldn't be able to cleanly restart the instruction after fixing up a page fault, so it seems like a good guess.) thanks -- PMM
Le 07/05/2016 à 23:50, Peter Maydell a écrit : > On 7 May 2016 at 20:01, Laurent Vivier <laurent@vivier.eu> wrote: >> >> >> Le 07/05/2016 à 00:00, Richard Henderson a écrit : >>> On 05/04/2016 11:21 AM, Laurent Vivier wrote: >>>> + reg = AREG(insn, 0); >>>> + src = gen_load(s, opsize, reg, 1); >>>> + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); >>>> + >>>> + reg = AREG(insn, 9); >>>> + dest = gen_load(s, opsize, reg, 1); >>>> + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); >>> >>> Delay the writeback to the first areg until after the second load. >> >> We can't delay because we can have "cmpm (%a0)+,(%a0)+" that is used to >> compare two consecutive memory contents. > > If you write back to the first areg before the second > load, don't you get the wrong value as seen by the > exception handler if the second load faults? > > Usually you want to use the updated value for the > purposes of calculating the address to use in the > second load, but you don't want to actually update > the guest CPU register until after the load has > happened, in case it faults. > > (Disclaimer: I'm just assuming that on a fault no > registers are updated, but if that wasn't the case the OS > wouldn't be able to cleanly restart the instruction after > fixing up a page fault, so it seems like a good guess.) You're right. But I didn't really care here of the page faults because m68k only supports non privileged instructions: coldfire is used in semi-hosting mode (no MMU) and 680x0-like machine has no hardware (I use it in linux-user mode). I'm working on a Quadra 800 [1] emulation where I have added all the needed hardware and privileged instructions, and of course, I have to re-work all the memory access because of what you have described above... But I'm going to try to update this series taking care of the memory access fault as much as possible. Laurent [1] https://github.com/vivier/qemu-m68k/tree/q800-v2.4.0
On 8 May 2016 at 10:07, Laurent Vivier <laurent@vivier.eu> wrote: > But I didn't really care here of the page faults because m68k only > supports non privileged instructions: coldfire is used in semi-hosting > mode (no MMU) and 680x0-like machine has no hardware (I use it in > linux-user mode). It does still affect linux-user if your guest program has a SIGSEGV handler and uses it to fix up and restart faulting instructions, but I think only a few obscure JIT-type apps do this. thanks -- PMM
diff --git a/target-m68k/translate.c b/target-m68k/translate.c index 029c166..2d92bdd 100644 --- a/target-m68k/translate.c +++ b/target-m68k/translate.c @@ -2758,6 +2758,26 @@ DISAS_INSN(cmpa) gen_update_cc_cmp(s, reg, src, opsize); } +DISAS_INSN(cmpm) +{ + TCGv src; + TCGv dest; + TCGv reg; + int opsize; + + opsize = insn_opsize(insn); + + reg = AREG(insn, 0); + src = gen_load(s, opsize, reg, 1); + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); + + reg = AREG(insn, 9); + dest = gen_load(s, opsize, reg, 1); + tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); + + gen_update_cc_cmp(s, dest, src, opsize); +} + DISAS_INSN(eor) { TCGv src; @@ -4876,6 +4896,7 @@ void register_m68k_insns (CPUM68KState *env) INSN(cmpa, b1c0, f1c0, CF_ISA_A); INSN(cmp, b000, f100, M68000); INSN(eor, b100, f100, M68000); + INSN(cmpm, b108, f138, M68000); INSN(cmpa, b0c0, f0c0, M68000); INSN(eor, b180, f1c0, CF_ISA_A); BASE(and, c000, f000);
Signed-off-by: Laurent Vivier <laurent@vivier.eu> --- target-m68k/translate.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)