On 16.10.2015 16:58, Peter Maydell wrote: > From: Sergey Fedorov <serge.f...@gmail.com> > > A QEMU breakpoint match is not definitely an architectural breakpoint > match. If an exception is generated unconditionally during translation, > it is hardly possible to ignore it in the debug exception handler. > > Generate a call to a helper to check CPU breakpoints and raise an > exception only if any breakpoint matches architecturally. > > Signed-off-by: Sergey Fedorov <serge.f...@gmail.com> > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > target-arm/helper.h | 2 ++ > target-arm/op_helper.c | 29 ++++++++++++++++++----------- > target-arm/translate-a64.c | 17 ++++++++++++----- > target-arm/translate.c | 19 ++++++++++++++----- > 4 files changed, 46 insertions(+), 21 deletions(-) > (snip) > diff --git a/target-arm/translate.c b/target-arm/translate.c > index 1273000..9f1d740 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -11342,11 +11342,20 @@ void gen_intermediate_code(CPUARMState *env, > TranslationBlock *tb) > CPUBreakpoint *bp; > QTAILQ_FOREACH(bp, &cs->breakpoints, entry) { > if (bp->pc == dc->pc) { > - gen_exception_internal_insn(dc, 0, EXCP_DEBUG); > - /* Advance PC so that clearing the breakpoint will > - invalidate this TB. */ > - dc->pc += 2; > - goto done_generating; > + if (bp->flags & BP_CPU) { > + gen_helper_check_breakpoints(cpu_env); > + /* End the TB early; it's likely not going to be > executed */ > + dc->is_jmp = DISAS_UPDATE; > + } else { > + gen_exception_internal_insn(dc, 0, EXCP_DEBUG); > + /* Advance PC so that clearing the breakpoint will > + invalidate this TB. */ > + /* TODO: Advance PC by correct instruction length to > + * avoid disassembler error messages */ > + dc->pc += 2; > + goto done_generating; > + } > + break; > } > } > }
It turns out that this change introduced an issue which can be illustrated by the following test: cat >test.s <<EOF .text .global _start _start: adr r0, bp mcr p14, 0, r0, c0, c0, 4 // DBGBVR0 mov r0, #1 orr r0, r0, #(0xf << 5) mcr p14, 0, r0, c0, c0, 5 // DBGBCR0 bp: nop wfi b . EOF arm-linux-gnueabi-as -o test.o test.s arm-linux-gnueabi-ld -Ttext=0x40000000 -o test.elf test.o ./qemu-system-arm -nographic -machine virt -cpu cortex-a15 -kernel \ test.elf -D qemu.log -d in_asm,exec -singlestep Actually, that is the same test as in https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02715.html but for AArch32. Running this test QEMU hangs executing code at the address where breakpoint is set: ---------------- IN: 0x40000000: e28f000c add r0, pc, #12 ; 0xc Trace 0x7f7c8bdc0028 [40000000] ---------------- IN: 0x40000004: ee000e90 mcr 14, 0, r0, cr0, cr0, {4} Trace 0x7f7c8bdc0070 [40000004] ---------------- IN: 0x40000008: e3a00001 mov r0, #1 ; 0x1 Trace 0x7f7c8bdc00b0 [40000008] ---------------- IN: 0x4000000c: e3800e1e orr r0, r0, #480 ; 0x1e0 Trace 0x7f7c8bdc00f0 [4000000c] ---------------- IN: 0x40000010: ee000eb0 mcr 14, 0, r0, cr0, cr0, {5} Trace 0x7f7c8bdc0140 [40000010] ---------------- IN: 0x40000014: e1a00000 nop (mov r0,r0) Trace 0x7f7c8bdc0180 [40000014] Trace 0x7f7c8bdc0180 [40000014] Trace 0x7f7c8bdc0180 [40000014] Trace 0x7f7c8bdc0180 [40000014] ... I can conclude that it is due to 'dc->is_jmp = DISAS_UPDATE'. With the following patch everything is okay: diff --git a/target-arm/translate.c b/target-arm/translate.c index 9f1d740..b55c5c2 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -11345,7 +11345,6 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) if (bp->flags & BP_CPU) { gen_helper_check_breakpoints(cpu_env); /* End the TB early; it's likely not going to be executed */ - dc->is_jmp = DISAS_UPDATE; } else { gen_exception_internal_insn(dc, 0, EXCP_DEBUG); /* Advance PC so that clearing the breakpoint will As far as I understand, we can't do this in target-arm/translate.c before dc->pc is advanced properly because CPU state's PC doesn't get updated as in target-arm/translate-a64.c. Compare: target-arm/translate.c: case DISAS_JUMP: case DISAS_UPDATE: /* indicate that the hash table must be used to find the next TB */ tcg_gen_exit_tb(0); break; target-arm/translate-a64.c: case DISAS_UPDATE: gen_a64_set_pc_im(dc->pc); /* fall through */ case DISAS_JUMP: /* indicate that the hash table must be used to find the next TB */ tcg_gen_exit_tb(0); break; I think we could fix this problem by cleaning up DISAS_UPDATE usage in target-arm/translate.c and implementing PC update as in target-arm/translate-a64.c. I could prepare a patch for that. Another problem, I think, is that we should somehow restore the CPU state before raising an exception from check_breakpoints() helper. But so far I have no idea how to fix this... Any suggestions are highly appreciated :) Best regards, Sergey