On 09.10.2015 17:00, Peter Maydell wrote: > On 9 October 2015 at 14:53, Sergey Fedorov <serge.f...@gmail.com> wrote: >> On 08.10.2015 21:40, Peter Maydell wrote: >>> On 28 September 2015 at 11:07, Sergey Fedorov <serge.f...@gmail.com> wrote: >>>> 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> >>>> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c >>>> index ec0936c..426229f 100644 >>>> --- a/target-arm/translate-a64.c >>>> +++ b/target-arm/translate-a64.c >>>> @@ -11082,11 +11082,14 @@ void gen_intermediate_code_internal_a64(ARMCPU >>>> *cpu, >>>> if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) { >>>> 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); >>>> + } else { >>>> + gen_exception_internal_insn(dc, 0, EXCP_DEBUG); >>> We shouldn't just continue here, because now we'll try to generate the >>> code for the instruction even in the "we know this will always be a bp" >>> case. Also, you've dropped the "advance PC" part which we need so this >>> TB is not zero length. >> Actually, I was going to do the same way as some architectures (e.g. >> alpha) did: always translate one instruction so that TB size is >> determined by actual instruction decoding. At least, it makes sense for >> AArch32 where we can have 16/32-bit insns. If we advance PC incorrectly, >> we will get "Disassembler disagrees with translator over instruction >> decoding" warning messages when in_asm log enabled. I can rewrite it >> with PC advancement, but at least, I would like to change the >> advancement to 4 bytes for A64 translation. > Hmm, I see. I'm not sure it makes sense to do a bunch of extra > work at codegen just to avoid a debug message. It would be nicer > to suppress that warning some other way. >
I think we could try figuring out the actual instruction length in case of Thumb instruction, i.e. to do partial instruction decoding.