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

[Richard Henderson <richard.henderson@linaro.org>]

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 <richard.henderson@linaro.org
> <mailto:richard.henderson@linaro.org>> 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;
>     > +        }