On Wed, Dec 6, 2017 at 8:29 AM, Mihail Abakumov
<mikhail.abaku...@ispras.ru> wrote:
> Ladi Prosek писал 2017-11-29 10:13:
>
>> On Tue, Nov 21, 2017 at 3:10 PM, Mihail Abakumov
>> <mikhail.abaku...@ispras.ru> wrote:
>>>
>>> Added handler registration of gdb debug exception. Its exception also can
>>> be used for windbg.
>>>
>>> Signed-off-by: Mihail Abakumov <mikhail.abaku...@ispras.ru>
>>> Signed-off-by: Pavel Dovgalyuk <dovga...@ispras.ru>
>>> Signed-off-by: Dmitriy Koltunov <koltu...@ispras.ru>
>>> ---
>>> 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?
Yes, "'-gdb' and '-windbg' cannot be used at the same time" would be
nicer. By being more explicit I mean issuing the error from the code
that parses the options.
In other words, I can imagine an implementation of
register_excp_debug_handler() that allows registering multiple
handlers. But running windbg and gdb at the same would probably still
not be a good idea.
Maybe I'm nit-picking and it's ok for the error to stay here.
Wondering what other reviewers think.
>>
>>> + 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;
>>> }
>>>
>
