| Message ID | 20210401144152.1031282-3-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gdbstub: implement support for blocking interrupts on single stepping | expand |
Maxim Levitsky <mlevitsk@redhat.com> writes: > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > accel/kvm/kvm-all.c | 25 +++++++++++++++++++ > gdbstub.c | 59 ++++++++++++++++++++++++++++++++++++-------- > include/sysemu/kvm.h | 13 ++++++++++ > 3 files changed, 87 insertions(+), 10 deletions(-) > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index b6d9f92f15..bc7955fb19 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -147,6 +147,8 @@ bool kvm_vm_attributes_allowed; > bool kvm_direct_msi_allowed; > bool kvm_ioeventfd_any_length_allowed; > bool kvm_msi_use_devid; > +bool kvm_has_guest_debug; > +int kvm_sstep_flags; > static bool kvm_immediate_exit; > static hwaddr kvm_max_slot_size = ~0; > > @@ -2186,6 +2188,25 @@ static int kvm_init(MachineState *ms) > kvm_ioeventfd_any_length_allowed = > (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0); > > + kvm_has_guest_debug = > + (kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG) > 0); > + > + kvm_sstep_flags = 0; > + > + if (kvm_has_guest_debug) { > + /* Assume that single stepping is supported */ > + kvm_sstep_flags = SSTEP_ENABLE; > + > + int guest_debug_flags = > + kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG2); > + > + if (guest_debug_flags > 0) { > + if (guest_debug_flags & KVM_GUESTDBG_BLOCKIRQ) { > + kvm_sstep_flags |= SSTEP_NOIRQ; > + } > + } > + } > + > kvm_state = s; > > ret = kvm_arch_init(ms, s); > @@ -2796,6 +2817,10 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) > > if (cpu->singlestep_enabled) { > data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; > + > + if (cpu->singlestep_enabled & SSTEP_NOIRQ) { > + data.dbg.control |= KVM_GUESTDBG_BLOCKIRQ; > + } > } > kvm_arch_update_guest_debug(cpu, &data.dbg); > > diff --git a/gdbstub.c b/gdbstub.c > index 054665e93e..f789ded99d 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -369,12 +369,11 @@ typedef struct GDBState { > gdb_syscall_complete_cb current_syscall_cb; > GString *str_buf; > GByteArray *mem_buf; > + int sstep_flags; > + int supported_sstep_flags; > } GDBState; > > -/* By default use no IRQs and no timers while single stepping so as to > - * make single stepping like an ICE HW step. > - */ > -static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER; > +static GDBState gdbserver_state; > > /* Retrieves flags for single step mode. */ > static int get_sstep_flags(void) > @@ -386,11 +385,10 @@ static int get_sstep_flags(void) > if (replay_mode != REPLAY_MODE_NONE) { > return SSTEP_ENABLE; > } else { > - return sstep_flags; > + return gdbserver_state.sstep_flags; > } > } > > -static GDBState gdbserver_state; > > static void init_gdbserver_state(void) > { > @@ -400,6 +398,23 @@ static void init_gdbserver_state(void) > gdbserver_state.str_buf = g_string_new(NULL); > gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH); > gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4); > + > + > + if (kvm_enabled()) { > + gdbserver_state.supported_sstep_flags = > kvm_get_supported_sstep_flags(); This falls over as soon as you build something without KVM support (like a TCG only build or an emulation only target): [10/1152] Compiling C object libqemu-riscv32-softmmu.fa.p/gdbstub.c.o FAILED: libqemu-riscv32-softmmu.fa.p/gdbstub.c.o cc -Ilibqemu-riscv32-softmmu.fa.p -I. -I../.. -Itarget/riscv -I../../target/riscv -Idtc/libfdt -I../../dtc/libfdt -I../../capstone/include/capstone -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/libdrm -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g -isystem /home/alex/lsrc/qemu.git/linux-headers -isystem linux-headers -iquote . -iquote /home/alex/lsrc/qemu.git -iquote /home/alex/lsrc/qemu.git/include -iquote /home/alex/lsrc/qemu.git/disas/libvixl -iquote /home/alex/lsrc/qemu.git/tcg/i386 -iquote /home/alex/lsrc/qemu.git/accel/tcg -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -DLEGACY_RDMA_REG_MR -fPIC -isystem../../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="riscv32-softmmu-config-target.h"' '-DCONFIG_DEVICES="riscv32-softmmu-config-devices.h"' -MD -MQ libqemu-riscv32-softmmu.fa.p/gdbstub.c.o -MF libqemu-riscv32-softmmu.fa.p/gdbstub.c.o.d -o libqemu-riscv32-softmmu.fa.p/gdbstub.c.o -c ../../gdbstub.c ../../gdbstub.c: In function ‘init_gdbserver_state’: ../../gdbstub.c:404:49: error: implicit declaration of function ‘kvm_get_supported_sstep_flags’; did you mean ‘hvf_get_supported_cpuid’? [-Werror=implicit-function-declaration] gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags(); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ hvf_get_supported_cpuid ../../gdbstub.c:404:49: error: nested extern declaration of ‘kvm_get_supported_sstep_flags’ [-Werror=nested-externs] ../../gdbstub.c: In function ‘gdbserver_start’: ../../gdbstub.c:3536:27: error: implicit declaration of function ‘kvm_supports_guest_debug’; did you mean ‘kvm_update_guest_debug’? [-Werror=implicit-function-declaration] if (kvm_enabled() && !kvm_supports_guest_debug()) { ^~~~~~~~~~~~~~~~~~~~~~~~ kvm_update_guest_debug ../../gdbstub.c:3536:27: error: nested extern declaration of ‘kvm_supports_guest_debug’ [-Werror=nested-externs] cc1: all warnings being treated as errors > + } else { > + gdbserver_state.supported_sstep_flags = > + SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER; > + } > + > + /* > + * By default use no IRQs and no timers while single stepping so as to > + * make single stepping like an ICE HW step. > + */ > + > + gdbserver_state.sstep_flags = SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER; > + gdbserver_state.sstep_flags &= gdbserver_state.supported_sstep_flags; > + > } > > #ifndef CONFIG_USER_ONLY > @@ -2023,24 +2038,43 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx) > > static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx) > { > - g_string_printf(gdbserver_state.str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", > - SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER); > + g_string_printf(gdbserver_state.str_buf, "ENABLE=%x", SSTEP_ENABLE); > + > + if (gdbserver_state.supported_sstep_flags & SSTEP_NOIRQ) { > + g_string_append_printf(gdbserver_state.str_buf, ",NOIRQ=%x", > + SSTEP_NOIRQ); > + } > + > + if (gdbserver_state.supported_sstep_flags & SSTEP_NOTIMER) { > + g_string_append_printf(gdbserver_state.str_buf, ",NOTIMER=%x", > + SSTEP_NOTIMER); > + } > + > put_strbuf(); > } > > static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) > { > + int new_sstep_flags; > if (!gdb_ctx->num_params) { > return; > } > > - sstep_flags = gdb_ctx->params[0].val_ul; > + new_sstep_flags = gdb_ctx->params[0].val_ul; > + > + if (new_sstep_flags & ~gdbserver_state.supported_sstep_flags) { > + put_packet("E22"); > + return; > + } > + > + gdbserver_state.sstep_flags = new_sstep_flags; > put_packet("OK"); > } > > static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) > { > - g_string_printf(gdbserver_state.str_buf, "0x%x", sstep_flags); > + g_string_printf(gdbserver_state.str_buf, "0x%x", > + gdbserver_state.sstep_flags); > put_strbuf(); > } > > @@ -3499,6 +3533,11 @@ int gdbserver_start(const char *device) > return -1; > } > > + if (kvm_enabled() && !kvm_supports_guest_debug()) { > + error_report("gdbstub: KVM doesn't support guest debugging"); > + return -1; > + } > + <snip> Otherwise it looks fine as far as it goes, however it would be nice to have some sort of test in for this. The gdbstub has a hand-rolled gdb script in tests/guest-debug/test-gdbstub.py but it's not integrated with the rest of the testing. As I suspect you need a) KVM enabled, b) a recent enough kernel and c) some sort of guest kernel that is going to enable timers and IRQs this might be something worth porting to the acceptance tests. We have an example in tests/acceptance/reverse_debugging.py which is run as part of check-acceptance. It's TCG only but perhaps is a template for how such a test could be implemented.
On Mon, 2021-04-19 at 17:29 +0100, Alex Bennée wrote: > Maxim Levitsky <mlevitsk@redhat.com> writes: > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > accel/kvm/kvm-all.c | 25 +++++++++++++++++++ > > gdbstub.c | 59 ++++++++++++++++++++++++++++++++++++-------- > > include/sysemu/kvm.h | 13 ++++++++++ > > 3 files changed, 87 insertions(+), 10 deletions(-) > > > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > > index b6d9f92f15..bc7955fb19 100644 > > --- a/accel/kvm/kvm-all.c > > +++ b/accel/kvm/kvm-all.c > > @@ -147,6 +147,8 @@ bool kvm_vm_attributes_allowed; > > bool kvm_direct_msi_allowed; > > bool kvm_ioeventfd_any_length_allowed; > > bool kvm_msi_use_devid; > > +bool kvm_has_guest_debug; > > +int kvm_sstep_flags; > > static bool kvm_immediate_exit; > > static hwaddr kvm_max_slot_size = ~0; > > > > @@ -2186,6 +2188,25 @@ static int kvm_init(MachineState *ms) > > kvm_ioeventfd_any_length_allowed = > > (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0); > > > > + kvm_has_guest_debug = > > + (kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG) > 0); > > + > > + kvm_sstep_flags = 0; > > + > > + if (kvm_has_guest_debug) { > > + /* Assume that single stepping is supported */ > > + kvm_sstep_flags = SSTEP_ENABLE; > > + > > + int guest_debug_flags = > > + kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG2); > > + > > + if (guest_debug_flags > 0) { > > + if (guest_debug_flags & KVM_GUESTDBG_BLOCKIRQ) { > > + kvm_sstep_flags |= SSTEP_NOIRQ; > > + } > > + } > > + } > > + > > kvm_state = s; > > > > ret = kvm_arch_init(ms, s); > > @@ -2796,6 +2817,10 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) > > > > if (cpu->singlestep_enabled) { > > data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; > > + > > + if (cpu->singlestep_enabled & SSTEP_NOIRQ) { > > + data.dbg.control |= KVM_GUESTDBG_BLOCKIRQ; > > + } > > } > > kvm_arch_update_guest_debug(cpu, &data.dbg); > > > > diff --git a/gdbstub.c b/gdbstub.c > > index 054665e93e..f789ded99d 100644 > > --- a/gdbstub.c > > +++ b/gdbstub.c > > @@ -369,12 +369,11 @@ typedef struct GDBState { > > gdb_syscall_complete_cb current_syscall_cb; > > GString *str_buf; > > GByteArray *mem_buf; > > + int sstep_flags; > > + int supported_sstep_flags; > > } GDBState; > > > > -/* By default use no IRQs and no timers while single stepping so as to > > - * make single stepping like an ICE HW step. > > - */ > > -static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER; > > +static GDBState gdbserver_state; > > > > /* Retrieves flags for single step mode. */ > > static int get_sstep_flags(void) > > @@ -386,11 +385,10 @@ static int get_sstep_flags(void) > > if (replay_mode != REPLAY_MODE_NONE) { > > return SSTEP_ENABLE; > > } else { > > - return sstep_flags; > > + return gdbserver_state.sstep_flags; > > } > > } > > > > -static GDBState gdbserver_state; > > > > static void init_gdbserver_state(void) > > { > > @@ -400,6 +398,23 @@ static void init_gdbserver_state(void) > > gdbserver_state.str_buf = g_string_new(NULL); > > gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH); > > gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4); > > + > > + > > + if (kvm_enabled()) { > > + gdbserver_state.supported_sstep_flags = > > kvm_get_supported_sstep_flags(); > > This falls over as soon as you build something without KVM support (like > a TCG only build or an emulation only target): This is something I'll check from now on before sending patches. > > [10/1152] Compiling C object libqemu-riscv32-softmmu.fa.p/gdbstub.c.o > FAILED: libqemu-riscv32-softmmu.fa.p/gdbstub.c.o > cc -Ilibqemu-riscv32-softmmu.fa.p -I. -I../.. -Itarget/riscv -I../../target/riscv -Idtc/libfdt -I../../dtc/libfdt -I../../capstone/include/capstone -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/libdrm -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g -isystem /home/alex/lsrc/qemu.git/linux-headers -isystem linux-headers -iquote . -iquote /home/alex/lsrc/qemu.git -iquote /home/alex/lsrc/qemu.git/include -iquote /home/alex/lsrc/qemu.git/disas/libvixl -iquote /home/alex/lsrc/qemu.git/tcg/i386 -iquote /home/alex/lsrc/qemu.git/accel/tcg -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -DLEGACY_RDMA_REG_MR -fPIC -isystem../../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="riscv32-softmmu-config-target.h"' '-DCONFIG_DEVICES="riscv32-softmmu-config-devices.h"' -MD -MQ libqemu-riscv32-softmmu.fa.p/gdbstub.c.o -MF libqemu-riscv32-softmmu.fa.p/gdbstub.c.o.d -o libqemu-riscv32-softmmu.fa.p/gdbstub.c.o -c ../../gdbstub.c > ../../gdbstub.c: In function ‘init_gdbserver_state’: > ../../gdbstub.c:404:49: error: implicit declaration of function ‘kvm_get_supported_sstep_flags’; did you mean ‘hvf_get_supported_cpuid’? [-Werror=implicit-function-declaration] > gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags(); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > hvf_get_supported_cpuid > ../../gdbstub.c:404:49: error: nested extern declaration of ‘kvm_get_supported_sstep_flags’ [-Werror=nested-externs] > ../../gdbstub.c: In function ‘gdbserver_start’: > ../../gdbstub.c:3536:27: error: implicit declaration of function ‘kvm_supports_guest_debug’; did you mean ‘kvm_update_guest_debug’? [-Werror=implicit-function-declaration] > if (kvm_enabled() && !kvm_supports_guest_debug()) { > ^~~~~~~~~~~~~~~~~~~~~~~~ > kvm_update_guest_debug > ../../gdbstub.c:3536:27: error: nested extern declaration of ‘kvm_supports_guest_debug’ [-Werror=nested-externs] > cc1: all warnings being treated as errors > > > > + } else { > > + gdbserver_state.supported_sstep_flags = > > + SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER; > > + } > > + > > + /* > > + * By default use no IRQs and no timers while single stepping so as to > > + * make single stepping like an ICE HW step. > > + */ > > + > > + gdbserver_state.sstep_flags = SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER; > > + gdbserver_state.sstep_flags &= gdbserver_state.supported_sstep_flags; > > + > > } > > > > #ifndef CONFIG_USER_ONLY > > @@ -2023,24 +2038,43 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx) > > > > static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx) > > { > > - g_string_printf(gdbserver_state.str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", > > - SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER); > > + g_string_printf(gdbserver_state.str_buf, "ENABLE=%x", SSTEP_ENABLE); > > + > > + if (gdbserver_state.supported_sstep_flags & SSTEP_NOIRQ) { > > + g_string_append_printf(gdbserver_state.str_buf, ",NOIRQ=%x", > > + SSTEP_NOIRQ); > > + } > > + > > + if (gdbserver_state.supported_sstep_flags & SSTEP_NOTIMER) { > > + g_string_append_printf(gdbserver_state.str_buf, ",NOTIMER=%x", > > + SSTEP_NOTIMER); > > + } > > + > > put_strbuf(); > > } > > > > static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) > > { > > + int new_sstep_flags; > > if (!gdb_ctx->num_params) { > > return; > > } > > > > - sstep_flags = gdb_ctx->params[0].val_ul; > > + new_sstep_flags = gdb_ctx->params[0].val_ul; > > + > > + if (new_sstep_flags & ~gdbserver_state.supported_sstep_flags) { > > + put_packet("E22"); > > + return; > > + } > > + > > + gdbserver_state.sstep_flags = new_sstep_flags; > > put_packet("OK"); > > } > > > > static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) > > { > > - g_string_printf(gdbserver_state.str_buf, "0x%x", sstep_flags); > > + g_string_printf(gdbserver_state.str_buf, "0x%x", > > + gdbserver_state.sstep_flags); > > put_strbuf(); > > } > > > > @@ -3499,6 +3533,11 @@ int gdbserver_start(const char *device) > > return -1; > > } > > > > + if (kvm_enabled() && !kvm_supports_guest_debug()) { > > + error_report("gdbstub: KVM doesn't support guest debugging"); > > + return -1; > > + } > > + > <snip> > > Otherwise it looks fine as far as it goes, however it would be nice to > have some sort of test in for this. The gdbstub has a hand-rolled gdb > script in tests/guest-debug/test-gdbstub.py but it's not integrated with > the rest of the testing. I'll take a look! > > As I suspect you need a) KVM enabled, b) a recent enough kernel and c) > some sort of guest kernel that is going to enable timers and IRQs this > might be something worth porting to the acceptance tests. > > We have an example in tests/acceptance/reverse_debugging.py which is run > as part of check-acceptance. It's TCG only but perhaps is a template for > how such a test could be implemented. Thanks for the review, Best regards, Maxim Levitsky >
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index b6d9f92f15..bc7955fb19 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -147,6 +147,8 @@ bool kvm_vm_attributes_allowed; bool kvm_direct_msi_allowed; bool kvm_ioeventfd_any_length_allowed; bool kvm_msi_use_devid; +bool kvm_has_guest_debug; +int kvm_sstep_flags; static bool kvm_immediate_exit; static hwaddr kvm_max_slot_size = ~0; @@ -2186,6 +2188,25 @@ static int kvm_init(MachineState *ms) kvm_ioeventfd_any_length_allowed = (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0); + kvm_has_guest_debug = + (kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG) > 0); + + kvm_sstep_flags = 0; + + if (kvm_has_guest_debug) { + /* Assume that single stepping is supported */ + kvm_sstep_flags = SSTEP_ENABLE; + + int guest_debug_flags = + kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG2); + + if (guest_debug_flags > 0) { + if (guest_debug_flags & KVM_GUESTDBG_BLOCKIRQ) { + kvm_sstep_flags |= SSTEP_NOIRQ; + } + } + } + kvm_state = s; ret = kvm_arch_init(ms, s); @@ -2796,6 +2817,10 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) if (cpu->singlestep_enabled) { data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; + + if (cpu->singlestep_enabled & SSTEP_NOIRQ) { + data.dbg.control |= KVM_GUESTDBG_BLOCKIRQ; + } } kvm_arch_update_guest_debug(cpu, &data.dbg); diff --git a/gdbstub.c b/gdbstub.c index 054665e93e..f789ded99d 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -369,12 +369,11 @@ typedef struct GDBState { gdb_syscall_complete_cb current_syscall_cb; GString *str_buf; GByteArray *mem_buf; + int sstep_flags; + int supported_sstep_flags; } GDBState; -/* By default use no IRQs and no timers while single stepping so as to - * make single stepping like an ICE HW step. - */ -static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER; +static GDBState gdbserver_state; /* Retrieves flags for single step mode. */ static int get_sstep_flags(void) @@ -386,11 +385,10 @@ static int get_sstep_flags(void) if (replay_mode != REPLAY_MODE_NONE) { return SSTEP_ENABLE; } else { - return sstep_flags; + return gdbserver_state.sstep_flags; } } -static GDBState gdbserver_state; static void init_gdbserver_state(void) { @@ -400,6 +398,23 @@ static void init_gdbserver_state(void) gdbserver_state.str_buf = g_string_new(NULL); gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH); gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4); + + + if (kvm_enabled()) { + gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags(); + } else { + gdbserver_state.supported_sstep_flags = + SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER; + } + + /* + * By default use no IRQs and no timers while single stepping so as to + * make single stepping like an ICE HW step. + */ + + gdbserver_state.sstep_flags = SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER; + gdbserver_state.sstep_flags &= gdbserver_state.supported_sstep_flags; + } #ifndef CONFIG_USER_ONLY @@ -2023,24 +2038,43 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx) static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx) { - g_string_printf(gdbserver_state.str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", - SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER); + g_string_printf(gdbserver_state.str_buf, "ENABLE=%x", SSTEP_ENABLE); + + if (gdbserver_state.supported_sstep_flags & SSTEP_NOIRQ) { + g_string_append_printf(gdbserver_state.str_buf, ",NOIRQ=%x", + SSTEP_NOIRQ); + } + + if (gdbserver_state.supported_sstep_flags & SSTEP_NOTIMER) { + g_string_append_printf(gdbserver_state.str_buf, ",NOTIMER=%x", + SSTEP_NOTIMER); + } + put_strbuf(); } static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) { + int new_sstep_flags; if (!gdb_ctx->num_params) { return; } - sstep_flags = gdb_ctx->params[0].val_ul; + new_sstep_flags = gdb_ctx->params[0].val_ul; + + if (new_sstep_flags & ~gdbserver_state.supported_sstep_flags) { + put_packet("E22"); + return; + } + + gdbserver_state.sstep_flags = new_sstep_flags; put_packet("OK"); } static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) { - g_string_printf(gdbserver_state.str_buf, "0x%x", sstep_flags); + g_string_printf(gdbserver_state.str_buf, "0x%x", + gdbserver_state.sstep_flags); put_strbuf(); } @@ -3499,6 +3533,11 @@ int gdbserver_start(const char *device) return -1; } + if (kvm_enabled() && !kvm_supports_guest_debug()) { + error_report("gdbstub: KVM doesn't support guest debugging"); + return -1; + } + if (!device) return -1; if (strcmp(device, "none") != 0) { diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index a1ab1ee12d..42f9ce6344 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -46,6 +46,8 @@ extern bool kvm_readonly_mem_allowed; extern bool kvm_direct_msi_allowed; extern bool kvm_ioeventfd_any_length_allowed; extern bool kvm_msi_use_devid; +extern bool kvm_has_guest_debug; +extern int kvm_sstep_flags; #define kvm_enabled() (kvm_allowed) /** @@ -167,6 +169,17 @@ extern bool kvm_msi_use_devid; */ #define kvm_msi_devid_required() (kvm_msi_use_devid) +/* + * Does KVM support guest debugging + */ +#define kvm_supports_guest_debug() (kvm_has_guest_debug) + +/* + * kvm_supported_sstep_flags + * Returns: SSTEP_* flags that KVM supports for guest debug + */ +#define kvm_get_supported_sstep_flags() (kvm_sstep_flags) + #else #define kvm_enabled() (0)
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- accel/kvm/kvm-all.c | 25 +++++++++++++++++++ gdbstub.c | 59 ++++++++++++++++++++++++++++++++++++-------- include/sysemu/kvm.h | 13 ++++++++++ 3 files changed, 87 insertions(+), 10 deletions(-)