Re: [Qemu-devel] [PATCH v3 35/45] windbg: debug exception subscribing

[Mihail Abakumov]

Ladi Prosek писал 2017-11-29 10:13:
> On Tue, Nov 21, 2017 at 3:10 PM, Mihail Abakumov
> <address@hidden> wrote:
> > Added handler registration of gdb debug exception. Its exception also 
> > can be used for windbg.
> >
> > Signed-off-by: Mihail Abakumov <address@hidden>
> > Signed-off-by: Pavel Dovgalyuk <address@hidden>
> > Signed-off-by: Dmitriy Koltunov <address@hidden>
> > ---
> >  cpus.c                  |   18 +++++++++++++++++-
> >  gdbstub.c               |    4 ++++
> >  include/sysemu/sysemu.h |    2 ++
> >  windbgstub.c            |   16 ++++++++++++----
> >  4 files changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/cpus.c b/cpus.c
> > index 9bed61eefc..212553b7e3 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -77,6 +77,8 @@ int64_t max_advance;
> >  static QEMUTimer *throttle_timer;
> >  static unsigned int throttle_percentage;
> >
> > +static void (*excp_debug_handler)(CPUState *cpu);
> > +
> >  #define CPU_THROTTLE_PCT_MIN 1
> >  #define CPU_THROTTLE_PCT_MAX 99
> >  #define CPU_THROTTLE_TIMESLICE_NS 10000000
> > @@ -960,9 +962,23 @@ static bool cpu_can_run(CPUState *cpu)
> >      return true;
> >  }
> >
> > +bool register_excp_debug_handler(void (*handler)(CPUState *cpu))
> > +{
> > +    if (excp_debug_handler == NULL) {
> > +        excp_debug_handler = handler;
> > +        return true;
> > +    } else {
> > +        error_report("ERROR: Something debugger already using");
>
> So I take it that -windbg and -gdb cannot be used at the same time.
> Should it be handled in a more explicit way with a more user-friendly
> error?
>
> Right now I get this error, which is more like an implementation
> detail (could the debug handler be refactored into a multicast event
> in the future?) and does not even make sense as a sentence.

Yes. I have added a more user-friendly error. "Something debugger is
already in use. '-gdb' and '-windbg' cannot be used at the same time".
Or what do you mean under the explicit way? How to do it better?

> > +        return false;
> > +    }
> > +}
> > +
> >  static void cpu_handle_guest_debug(CPUState *cpu)
> >  {
> > -    gdb_set_stop_cpu(cpu);
> > +    if (excp_debug_handler != NULL) {
> > +        excp_debug_handler(cpu);
> > +    }
> > +
> >      qemu_system_debug_request();
> >      cpu->stopped = true;
> >  }
> > diff --git a/gdbstub.c b/gdbstub.c
> > index 2a94030d3b..8c76f54117 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -2006,6 +2006,10 @@ int gdbserver_start(const char *device)
> >      s->mon_chr = mon_chr;
> >      s->current_syscall_cb = NULL;
> >
> > +    if (!register_excp_debug_handler(gdb_set_stop_cpu)) {
> > +        exit(1);
> > +    }
> > +
> >      return 0;
> >  }
> >
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index b21369672a..34588c99b4 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -193,6 +193,8 @@ QemuOpts *qemu_get_machine_opts(void);
> >
> >  bool defaults_enabled(void);
> >
> > +bool register_excp_debug_handler(void (*handler)(CPUState *cpu));
> > +
> >  extern QemuOptsList qemu_legacy_drive_opts;
> >  extern QemuOptsList qemu_common_drive_opts;
> >  extern QemuOptsList qemu_drive_opts;
> > diff --git a/windbgstub.c b/windbgstub.c
> > index 489abe6d6c..b33f412659 100755
> > --- a/windbgstub.c
> > +++ b/windbgstub.c
> > @@ -115,16 +115,20 @@ static void windbg_send_control_packet(uint16_t 
> > type)
> >      windbg_state->ctrl_packet_id ^= 1;
> >  }
> >
> > -static void windbg_vm_stop(void)
> > +static void windbg_bp_handler(CPUState *cpu)
> >  {
> > -    CPUState *cpu = qemu_get_cpu(0);
> > -    vm_stop(RUN_STATE_PAUSED);
> > -
> >      SizedBuf buf = kd_gen_exception_sc(cpu);
> >      windbg_send_data_packet(buf.data, buf.size, 
> > PACKET_TYPE_KD_STATE_CHANGE64);
> >      SBUF_FREE(buf);
> >  }
> >
> > +static void windbg_vm_stop(void)
> > +{
> > +    CPUState *cpu = qemu_get_cpu(0);
> > +    vm_stop(RUN_STATE_PAUSED);
> > +    windbg_bp_handler(cpu);
> > +}
> > +
> >  static void windbg_process_manipulate_packet(ParsingContext *ctx)
> >  {
> >      CPUState *cpu;
> > @@ -432,6 +436,10 @@ int windbg_server_start(const char *device)
> >
> >      qemu_register_reset(windbg_handle_reset, NULL);
> >
> > +    if (!register_excp_debug_handler(windbg_bp_handler)) {
> > +        exit(1);
> > +    }
> > +
> >      atexit(windbg_exit);
> >      return 0;
> >  }