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.
thanks
-- PMM
