On Fri, Jan 10, 2020 at 12:13:43PM -0300, Fabiano Rosas wrote: > For single stepping (via KVM) of a guest vcpu to work, KVM needs not > only to support the SET_GUEST_DEBUG ioctl but to also recognize the > KVM_GUESTDBG_SINGLESTEP bit in the control field of the > kvm_guest_debug struct. > > This patch adds support for querying the single step capability so > that QEMU can decide what to do for the platforms that do not have > such support. > > This will allow architecture-specific implementations of a fallback > mechanism for single stepping in cases where KVM does not support it. > > Signed-off-by: Fabiano Rosas Reviewed-by: David Gibson and ppc parts Acked-by: David Gibson > --- > accel/kvm/kvm-all.c | 9 +++++++++ > accel/stubs/kvm-stub.c | 5 +++++ > exec.c | 2 +- > include/sysemu/kvm.h | 3 +++ > stubs/Makefile.objs | 1 + > stubs/kvm-arch-singlestep.c | 14 ++++++++++++++ > target/ppc/kvm.c | 15 +++++++++++++++ > 7 files changed, 48 insertions(+), 1 deletion(-) > create mode 100644 stubs/kvm-arch-singlestep.c > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index b2f1a5bcb5..a1ea8f9641 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2668,6 +2668,15 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) > return data.err; > } > > +void kvm_set_singlestep(CPUState *cs, int enabled) > +{ > + if (kvm_arch_can_singlestep(cs)) { > + kvm_update_guest_debug(cs, 0); > + } else { > + kvm_arch_emulate_singlestep(cs, enabled); > + } > +} > + > int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr, > target_ulong len, int type) > { > diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c > index 82f118d2df..8773c15713 100644 > --- a/accel/stubs/kvm-stub.c > +++ b/accel/stubs/kvm-stub.c > @@ -78,6 +78,11 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) > return -ENOSYS; > } > > +void kvm_set_singlestep(CPUState *cs, int enabled) > +{ > + g_assert_not_reached(); > +} > + > int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr, > target_ulong len, int type) > { > diff --git a/exec.c b/exec.c > index d4b769d0d4..10b61fccb8 100644 > --- a/exec.c > +++ b/exec.c > @@ -1204,7 +1204,7 @@ void cpu_single_step(CPUState *cpu, int enabled) > if (cpu->singlestep_enabled != enabled) { > cpu->singlestep_enabled = enabled; > if (kvm_enabled()) { > - kvm_update_guest_debug(cpu, 0); > + kvm_set_singlestep(cpu, enabled); > } else { > /* must flush all the translated code to avoid inconsistencies */ > /* XXX: only flush what is necessary */ > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 141342de98..43abdb8430 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -247,6 +247,8 @@ bool kvm_memcrypt_enabled(void); > */ > int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len); > > +bool kvm_arch_can_singlestep(CPUState *cs); > +void kvm_arch_emulate_singlestep(CPUState *cpu, int enabled); > > #ifdef NEED_CPU_H > #include "cpu.h" > @@ -259,6 +261,7 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr, > target_ulong len, int type); > void kvm_remove_all_breakpoints(CPUState *cpu); > int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap); > +void kvm_set_singlestep(CPUState *cs, int enabled); > > int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr); > int kvm_on_sigbus(int code, void *addr); > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs > index 8b0ff25508..e6310b878c 100644 > --- a/stubs/Makefile.objs > +++ b/stubs/Makefile.objs > @@ -12,6 +12,7 @@ stub-obj-y += get-vm-name.o > stub-obj-y += iothread.o > stub-obj-y += iothread-lock.o > stub-obj-y += is-daemonized.o > +stub-obj-y += kvm-arch-singlestep.o > stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o > stub-obj-y += machine-init-done.o > stub-obj-y += migr-blocker.o > diff --git a/stubs/kvm-arch-singlestep.c b/stubs/kvm-arch-singlestep.c > new file mode 100644 > index 0000000000..30ee278ba4 > --- /dev/null > +++ b/stubs/kvm-arch-singlestep.c > @@ -0,0 +1,14 @@ > +#include "qemu/osdep.h" > +#include "qemu/error-report.h" > +#include "sysemu/kvm.h" > + > +bool kvm_arch_can_singlestep(CPUState *cs) > +{ > + /* for backwards compatibility assume the feature is present */ > + return true; > +} > + > +void kvm_arch_emulate_singlestep(CPUState *cpu, int enabled) > +{ > + warn_report("KVM does not support single stepping"); > +} > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 0bd4a8d399..6fb8687126 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -88,6 +88,7 @@ static int cap_ppc_safe_indirect_branch; > static int cap_ppc_count_cache_flush_assist; > static int cap_ppc_nested_kvm_hv; > static int cap_large_decr; > +static int cap_ppc_singlestep; > > static uint32_t debug_inst_opcode; > > @@ -136,6 +137,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > kvmppc_get_cpu_characteristics(s); > cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV); > cap_large_decr = kvmppc_get_dec_bits(); > + cap_ppc_singlestep = kvm_vm_check_extension(s, KVM_CAP_PPC_GUEST_DEBUG_SSTEP); > /* > * Note: setting it to false because there is not such capability > * in KVM at this moment. > @@ -1383,6 +1385,19 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env, > return 0; > } > > +bool kvm_arch_can_singlestep(CPUState *cs) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + > + if (cap_ppc_singlestep) { > + return true; > + } > + > + /* Fallback for when the capability is not available */ > + return env->excp_model == POWERPC_EXCP_BOOKE || kvmppc_is_pr(kvm_state); > +} > + > int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > { > /* Mixed endian case is not handled */ -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson