diff mbox series

[RFC,v20,3/8] target/avr: Add mechanism to check for active debugger connection

Message ID 20190530190738.22713-4-mrolnik@gmail.com (mailing list archive)
State New, archived
Headers show
Series QEMU AVR 8 bit cores | expand

Commit Message

Michael Rolnik May 30, 2019, 7:07 p.m. UTC
From: Sarah Harris <S.E.Harris@kent.ac.uk>

AVR CPUs have a BREAK instruction which behaves differently depending on whether debugging is enabled.
Since the hardware fuses that normally control this are difficult to emulate, and the BREAK instruction is useful for testing, the BREAK instruction is instead enabled/disabled depending on whether a GDB session is attached.

Signed-off-by: Sarah Harris <S.E.Harris@kent.ac.uk>
Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
---
 gdbstub.c              | 5 +++++
 include/exec/gdbstub.h | 4 ++++
 2 files changed, 9 insertions(+)

Comments

Richard Henderson May 31, 2019, 1:54 p.m. UTC | #1
On 5/30/19 2:07 PM, Michael Rolnik wrote:
> From: Sarah Harris <S.E.Harris@kent.ac.uk>
> 
> AVR CPUs have a BREAK instruction which behaves differently depending on whether debugging is enabled.
> Since the hardware fuses that normally control this are difficult to emulate, and the BREAK instruction is useful for testing, the BREAK instruction is instead enabled/disabled depending on whether a GDB session is attached.

I don't think that this is the right way to model this.

I think that BREAK should always raise an exception and return to the main
loop.  If there we find that the debugger is not attached, the main loop will
simply continue.  Which becomes a nop, as documented.


r~
Michael Rolnik June 1, 2019, 9:12 p.m. UTC | #2
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 @
gdb_set_stop_cpu, after having fixed it as follows it stops right after the
BREAK and does not advance, however when gdb is connected it works fine.
```
 void gdb_set_stop_cpu(CPUState *cpu)
 {






*     GDBProcess *p;     if (!gdbserver_state) {         return;     }
 p = gdb_get_cpu_process(gdbserver_state, cpu);*

     if (!p->attached) {
         /*
          * Having a stop CPU corresponding to a process that is not
attached
          * confuses GDB. So we ignore the request.
          */
         return;
     }

     gdbserver_state->c_cpu = cpu;
     gdbserver_state->g_cpu = cpu;
 }
```



On Fri, May 31, 2019 at 4:54 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 5/30/19 2:07 PM, Michael Rolnik wrote:
> > From: Sarah Harris <S.E.Harris@kent.ac.uk>
> >
> > AVR CPUs have a BREAK instruction which behaves differently depending on
> whether debugging is enabled.
> > Since the hardware fuses that normally control this are difficult to
> emulate, and the BREAK instruction is useful for testing, the BREAK
> instruction is instead enabled/disabled depending on whether a GDB session
> is attached.
>
> I don't think that this is the right way to model this.
>
> I think that BREAK should always raise an exception and return to the main
> loop.  If there we find that the debugger is not attached, the main loop
> will
> simply continue.  Which becomes a nop, as documented.
>
>
> r~
>
Richard Henderson June 3, 2019, 3:44 p.m. UTC | #3
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;
> +        }



r~
Michael Rolnik June 3, 2019, 4:29 p.m. UTC | #4
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.

Sent from my cell phone, please ignore typos

On Mon, Jun 3, 2019, 6:44 PM Richard Henderson <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;
> > +        }
>
>
>
> r~
>
Richard Henderson June 3, 2019, 4:36 p.m. UTC | #5
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;
>     > +        }
Michael Rolnik June 3, 2019, 5:04 p.m. UTC | #6
Got it.

Sent from my cell phone, please ignore typos

On Mon, Jun 3, 2019, 7:37 PM Richard Henderson <richard.henderson@linaro.org>
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 <
> 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;
> >     > +        }
>
>
Michael Rolnik June 5, 2019, 7:20 a.m. UTC | #7
Hi Richard.

I am still struggling with this one.

The spec says.
The BREAK instruction is used by the On-chip Debug system, and is normally
not used in the application software.
When the BREAK instruction is executed, the AVR CPU is set in the Stopped
Mode.
This gives the On-chip Debugger access to internal resources.
If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are
unprogrammed, the CPU will treat the BREAK instruction as a NOP and will
not enter the Stopped mode.

I read is as follows
- If user has an intention of using debugger, BREAK should be translated to
QEMU debug breakpoint
- If user has no intention of using debugger, BREAK should be translated
into NOP.

however it seems that rising EXCP_DEBUG crashes QEMU when no debugger is
present or *-s* was not supplied.

This lead me to the following.
- checking for active GDB connection is not good, because it might change
but translated BREAK will not
- if *-s* is supplied BREAK should always raise EXCP_DEBUG exception
- if *-s* is not supplied BREAK should be translated into NOP

What do you think? How to check existence of *-s* option?

Regards,
Michael



On Mon, Jun 3, 2019 at 8:04 PM Michael Rolnik <mrolnik@gmail.com> wrote:

> Got it.
>
> Sent from my cell phone, please ignore typos
>
> On Mon, Jun 3, 2019, 7:37 PM Richard Henderson <
> richard.henderson@linaro.org> 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 <
>> 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;
>> >     > +        }
>>
>>
Richard Henderson June 5, 2019, 2:36 p.m. UTC | #8
On 6/5/19 2:20 AM, Michael Rolnik wrote:
> Hi Richard.
> 
> I am still struggling with this one.
> 
> The spec says.
> The BREAK instruction is used by the On-chip Debug system, and is normally not
> used in the application software. 
> When the BREAK instruction is executed, the AVR CPU is set in the Stopped Mode. 
> This gives the On-chip Debugger access to internal resources. 
> If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are unprogrammed,
> the CPU will treat the BREAK instruction as a NOP and will not enter the
> Stopped mode.

Yep.

> I read is as follows
> - If user has an intention of using debugger, BREAK should be translated to
> QEMU debug breakpoint
> - If user has no intention of using debugger, BREAK should be translated into NOP.

I do not read it that way.  The spec is talking about a specific implementation
of debugging -- fuses, jtag and all.  We do not need to implement breakpoints
using any of those mechanisms, because we can insert breakpoints via
on-the-side data structures and re-translation.


r~
Michael Rolnik June 5, 2019, 3:19 p.m. UTC | #9
Richard.

We use break instruction for testing. (Here
https://github.com/seharris/qemu-avr-tests/tree/master/instruction-tests).
Each test is a big list of small tests with a break between them. We run
the tests on HW and on QEMU then compare register after each break. If we
don't implement break the way Sarah suggested we have no way of testing.
What do you suggest?

Sent from my cell phone, please ignore typos

On Wed, Jun 5, 2019, 5:37 PM Richard Henderson <richard.henderson@linaro.org>
wrote:

> On 6/5/19 2:20 AM, Michael Rolnik wrote:
> > Hi Richard.
> >
> > I am still struggling with this one.
> >
> > The spec says.
> > The BREAK instruction is used by the On-chip Debug system, and is
> normally not
> > used in the application software.
> > When the BREAK instruction is executed, the AVR CPU is set in the
> Stopped Mode.
> > This gives the On-chip Debugger access to internal resources.
> > If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are
> unprogrammed,
> > the CPU will treat the BREAK instruction as a NOP and will not enter the
> > Stopped mode.
>
> Yep.
>
> > I read is as follows
> > - If user has an intention of using debugger, BREAK should be translated
> to
> > QEMU debug breakpoint
> > - If user has no intention of using debugger, BREAK should be translated
> into NOP.
>
> I do not read it that way.  The spec is talking about a specific
> implementation
> of debugging -- fuses, jtag and all.  We do not need to implement
> breakpoints
> using any of those mechanisms, because we can insert breakpoints via
> on-the-side data structures and re-translation.
>
>
> r~
>

On Wed, Jun 5, 2019, 5:37 PM Richard Henderson <richard.henderson@linaro.org>
wrote:

> On 6/5/19 2:20 AM, Michael Rolnik wrote:
> > Hi Richard.
> >
> > I am still struggling with this one.
> >
> > The spec says.
> > The BREAK instruction is used by the On-chip Debug system, and is
> normally not
> > used in the application software.
> > When the BREAK instruction is executed, the AVR CPU is set in the
> Stopped Mode.
> > This gives the On-chip Debugger access to internal resources.
> > If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are
> unprogrammed,
> > the CPU will treat the BREAK instruction as a NOP and will not enter the
> > Stopped mode.
>
> Yep.
>
> > I read is as follows
> > - If user has an intention of using debugger, BREAK should be translated
> to
> > QEMU debug breakpoint
> > - If user has no intention of using debugger, BREAK should be translated
> into NOP.
>
> I do not read it that way.  The spec is talking about a specific
> implementation
> of debugging -- fuses, jtag and all.  We do not need to implement
> breakpoints
> using any of those mechanisms, because we can insert breakpoints via
> on-the-side data structures and re-translation.
>
>
> r~
>
Richard Henderson June 5, 2019, 4:06 p.m. UTC | #10
On 6/5/19 10:19 AM, Michael Rolnik wrote:
> Richard.
> 
> We use break instruction for testing.
> (Here https://github.com/seharris/qemu-avr-tests/tree/master/instruction-tests).
> Each test is a big list of small tests with a break between them. We run the
> tests on HW and on QEMU then compare register after each break. If we don't
> implement break the way Sarah suggested we have no way of testing.
> What do you suggest?

Hmm.

Ordinarily I would say to use some sort of syscall/exception/interrupt
instruction, but I see that avr doesn't have any of those.

You could use "call breakpoint" instead of using raw BREAK instructions with
the implementation just being

breakpoint:
	ret

and have gdb issue "break breakpoint" before beginning.

But perhaps I'm being over-pedantic about this, and someone else should weigh
in.  Alex, I nominate you since you most recently touched gdbserver.  ;-)


r~
Alex Bennée June 5, 2019, 4:10 p.m. UTC | #11
Michael Rolnik <mrolnik@gmail.com> writes:

> Richard.
>
> We use break instruction for testing. (Here
> https://github.com/seharris/qemu-avr-tests/tree/master/instruction-tests).
> Each test is a big list of small tests with a break between them. We run
> the tests on HW and on QEMU then compare register after each break.

This is exactly the same process RISU uses for testing. But it works
with a) linux-user and b) some architecturally defined illegal
instruction that will cause a SIGILL.

> If we
> don't implement break the way Sarah suggested we have no way of
> testing.

So this is the behaviour of AVR simulator when gdb is attached?

> What do you suggest?
>
> Sent from my cell phone, please ignore typos
>
> On Wed, Jun 5, 2019, 5:37 PM Richard Henderson <richard.henderson@linaro.org>
> wrote:
>
>> On 6/5/19 2:20 AM, Michael Rolnik wrote:
>> > Hi Richard.
>> >
>> > I am still struggling with this one.
>> >
>> > The spec says.
>> > The BREAK instruction is used by the On-chip Debug system, and is
>> normally not
>> > used in the application software.
>> > When the BREAK instruction is executed, the AVR CPU is set in the
>> Stopped Mode.
>> > This gives the On-chip Debugger access to internal resources.
>> > If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are
>> unprogrammed,
>> > the CPU will treat the BREAK instruction as a NOP and will not enter the
>> > Stopped mode.
>>
>> Yep.
>>
>> > I read is as follows
>> > - If user has an intention of using debugger, BREAK should be translated
>> to
>> > QEMU debug breakpoint
>> > - If user has no intention of using debugger, BREAK should be translated
>> into NOP.
>>
>> I do not read it that way.  The spec is talking about a specific
>> implementation
>> of debugging -- fuses, jtag and all.  We do not need to implement
>> breakpoints
>> using any of those mechanisms, because we can insert breakpoints via
>> on-the-side data structures and re-translation.
>>
>>
>> r~
>>
>
> On Wed, Jun 5, 2019, 5:37 PM Richard Henderson <richard.henderson@linaro.org>
> wrote:
>
>> On 6/5/19 2:20 AM, Michael Rolnik wrote:
>> > Hi Richard.
>> >
>> > I am still struggling with this one.
>> >
>> > The spec says.
>> > The BREAK instruction is used by the On-chip Debug system, and is
>> normally not
>> > used in the application software.
>> > When the BREAK instruction is executed, the AVR CPU is set in the
>> Stopped Mode.
>> > This gives the On-chip Debugger access to internal resources.
>> > If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are
>> unprogrammed,
>> > the CPU will treat the BREAK instruction as a NOP and will not enter the
>> > Stopped mode.
>>
>> Yep.
>>
>> > I read is as follows
>> > - If user has an intention of using debugger, BREAK should be translated
>> to
>> > QEMU debug breakpoint
>> > - If user has no intention of using debugger, BREAK should be translated
>> into NOP.
>>
>> I do not read it that way.  The spec is talking about a specific
>> implementation
>> of debugging -- fuses, jtag and all.  We do not need to implement
>> breakpoints
>> using any of those mechanisms, because we can insert breakpoints via
>> on-the-side data structures and re-translation.
>>
>>
>> r~
>>


--
Alex Bennée
Michael Rolnik June 5, 2019, 5:57 p.m. UTC | #12
Just ran the test with simavr, once it hit BREAK, gdb stopped

On Wed, Jun 5, 2019 at 7:12 PM Alex Bennée <alex.bennee@linaro.org> wrote:

>
> Michael Rolnik <mrolnik@gmail.com> writes:
>
> > Richard.
> >
> > We use break instruction for testing. (Here
> > https://github.com/seharris/qemu-avr-tests/tree/master/instruction-tests
> ).
> > Each test is a big list of small tests with a break between them. We run
> > the tests on HW and on QEMU then compare register after each break.
>
> This is exactly the same process RISU uses for testing. But it works
> with a) linux-user and b) some architecturally defined illegal
> instruction that will cause a SIGILL.
>
> > If we
> > don't implement break the way Sarah suggested we have no way of
> > testing.
>
> So this is the behaviour of AVR simulator when gdb is attached?
>
> > What do you suggest?
> >
> > Sent from my cell phone, please ignore typos
> >
> > On Wed, Jun 5, 2019, 5:37 PM Richard Henderson <
> richard.henderson@linaro.org>
> > wrote:
> >
> >> On 6/5/19 2:20 AM, Michael Rolnik wrote:
> >> > Hi Richard.
> >> >
> >> > I am still struggling with this one.
> >> >
> >> > The spec says.
> >> > The BREAK instruction is used by the On-chip Debug system, and is
> >> normally not
> >> > used in the application software.
> >> > When the BREAK instruction is executed, the AVR CPU is set in the
> >> Stopped Mode.
> >> > This gives the On-chip Debugger access to internal resources.
> >> > If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are
> >> unprogrammed,
> >> > the CPU will treat the BREAK instruction as a NOP and will not enter
> the
> >> > Stopped mode.
> >>
> >> Yep.
> >>
> >> > I read is as follows
> >> > - If user has an intention of using debugger, BREAK should be
> translated
> >> to
> >> > QEMU debug breakpoint
> >> > - If user has no intention of using debugger, BREAK should be
> translated
> >> into NOP.
> >>
> >> I do not read it that way.  The spec is talking about a specific
> >> implementation
> >> of debugging -- fuses, jtag and all.  We do not need to implement
> >> breakpoints
> >> using any of those mechanisms, because we can insert breakpoints via
> >> on-the-side data structures and re-translation.
> >>
> >>
> >> r~
> >>
> >
> > On Wed, Jun 5, 2019, 5:37 PM Richard Henderson <
> richard.henderson@linaro.org>
> > wrote:
> >
> >> On 6/5/19 2:20 AM, Michael Rolnik wrote:
> >> > Hi Richard.
> >> >
> >> > I am still struggling with this one.
> >> >
> >> > The spec says.
> >> > The BREAK instruction is used by the On-chip Debug system, and is
> >> normally not
> >> > used in the application software.
> >> > When the BREAK instruction is executed, the AVR CPU is set in the
> >> Stopped Mode.
> >> > This gives the On-chip Debugger access to internal resources.
> >> > If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are
> >> unprogrammed,
> >> > the CPU will treat the BREAK instruction as a NOP and will not enter
> the
> >> > Stopped mode.
> >>
> >> Yep.
> >>
> >> > I read is as follows
> >> > - If user has an intention of using debugger, BREAK should be
> translated
> >> to
> >> > QEMU debug breakpoint
> >> > - If user has no intention of using debugger, BREAK should be
> translated
> >> into NOP.
> >>
> >> I do not read it that way.  The spec is talking about a specific
> >> implementation
> >> of debugging -- fuses, jtag and all.  We do not need to implement
> >> breakpoints
> >> using any of those mechanisms, because we can insert breakpoints via
> >> on-the-side data structures and re-translation.
> >>
> >>
> >> r~
> >>
>
>
> --
> Alex Bennée
>
>
diff mbox series

Patch

diff --git a/gdbstub.c b/gdbstub.c
index 462f89edfe..94ee45798b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1793,6 +1793,11 @@  static int gdb_handle_packet(GDBState *s, const char *line_buf)
     return RS_IDLE;
 }
 
+bool gdb_is_active(void)
+{
+    return gdbserver_state != NULL;
+}
+
 void gdb_set_stop_cpu(CPUState *cpu)
 {
     GDBProcess *p = gdb_get_cpu_process(gdbserver_state, cpu);
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 08363969c1..d059bf5339 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -45,6 +45,10 @@  void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
  */
 void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
 int use_gdb_syscalls(void);
+/**
+ * gdb_is_active: return true if debugging in progress
+ */
+bool gdb_is_active(void);
 void gdb_set_stop_cpu(CPUState *cpu);
 void gdb_exit(CPUArchState *, int);
 #ifdef CONFIG_USER_ONLY