On 18 September 2015 at 15:07, Sergey Fedorov <serge.f...@gmail.com> wrote:
> On 18.09.2015 16:50, Peter Maydell wrote:
>> On 14 September 2015 at 11:51, Sergey Fedorov <serge.f...@gmail.com> wrote:
>>> --- a/target-arm/translate-a64.c
>>> +++ b/target-arm/translate-a64.c
>>> @@ -11000,11 +11000,13 @@ 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);
>>> + break;
>>> + } else {
>>> + gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
>>> + goto done_generating;
>>> + }
>> You seem to have dropped the "advance the PC" code -- why is that ok?
>>
>
> I also dropped the immediately following goto statement. With these
> changes PC is advanced in the same way as it happens during normal
> translation. That is because we actually have to do the instruction
> translation process here to support the case when a breakpoint with
> matching PC is architecturally mismatched. As I understand, that
> "advance the PC" code was necessary to produce a TB with non-zero size
> so that it can be invalidated later when we clear the breakpoint.
OK, that makes sense for the BP_CPU case but you still have the
"goto done_generating;" in the else clause...
Also, should we maybe make this TB be only one insn long even for
the BP_CPU case? It seems like in the common case we will only
execute one insn.
thanks
-- PMM
