Re: [Qemu-devel] [PATCH RFC v20 3/8] target/avr: Add mechanism to check for active debugger connection

[Michael Rolnik]

Got it.

Sent from my cell phone, please ignore typos

On Mon, Jun 3, 2019, 7:37 PM Richard Henderson <address@hidden>
wrote:

> On 6/3/19 11:29 AM, Michael Rolnik wrote:
> > 1. There's a break
> > instruction
> https://www.microchip.com/webdoc/avrassembler/avrassembler.wb_BREAK.html
> > 2. There's a set of tests that use break.
> >
> > So I assume I have to implement this instruction as described in the
> spec.
>
> The spec talks about fuses, not gdb.  A valid implementation of this
> instruction is a no-op -- it say so right there in the spec.
>
> What does it mean to "test" break?  AFAIK, you can't test this at all from
> within the cpu itself, since it does not generate a cpu-level exception.
>
> If gdb is setting a breakpoint via -S, it should be done via
> cpu_breakpoint_test.
>
>
>
> > On Mon, Jun 3, 2019, 6:44 PM Richard Henderson <
> address@hidden
> > <mailto:address@hidden>> wrote:
> >
> >     On 6/1/19 4:12 PM, Michael Rolnik wrote:
> >     > Hi Richard.
> >     >
> >     > If I implement it this way
> >     >
> >     > ```
> >     >  static bool trans_BREAK(DisasContext *ctx, arg_BREAK *a)
> >     >  {
> >     >      if (avr_feature(ctx->env, AVR_FEATURE_BREAK) == false) {
> >     >          gen_helper_unsupported(cpu_env);
> >     >      } else {
> >     >          tcg_gen_movi_tl(cpu_pc, ctx->inst[0].npc);
> >     >          gen_helper_debug(cpu_env);
> >     >      }
> >     >
> >     >      ctx->bstate = BS_EXCP;
> >     >
> >     >      return true;
> >     >  }
> >     > ```
> >     >
> >     > qemu (without -s -S flags) crashes when debugger is not connected
> >
> >     I was not suggesting using the internal qemu EXCP_DEBUG, but another
> AVR
> >     specific exception, much the same way as every other cpu has a
> cpu-specific
> >     debug exception.
> >
> >     Or perhaps always do nothing.  Why is gdb insertting BREAK in the
> first place?
> >      It should be using the "hardware breakpoint" support that qemu
> advertises as
> >     part of the gdbstub protocol, and that you support here:
> >
> >     > +        if (unlikely(cpu_breakpoint_test(cs, OFFSET_CODE + cpc *
> 2, BP_ANY))
> >     > +                 || cpu_breakpoint_test(cs, OFFSET_DATA + cpc * 2,
> >     BP_ANY)) {
> >     > +            tcg_gen_movi_i32(cpu_pc, cpc);
> >     > +            gen_helper_debug(cpu_env);
> >     > +            ctx.bstate = BS_EXCP;
> >     > +            goto done_generating;
> >     > +        }
>
>