diff mbox series

[RFC,v2,2/3] kvm-all: Introduce kvm_set_singlestep

Message ID 20181121181347.24035-3-farosas@linux.ibm.com
State New
Headers show
Series target/ppc: single step for KVM HV | expand

Commit Message

Fabiano Rosas Nov. 21, 2018, 6:13 p.m. UTC
This will be used in a future patch to implement an
architecture-specific single step mechanism for POWER.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 accel/kvm/kvm-all.c  | 10 ++++++++++
 exec.c               |  1 +
 include/sysemu/kvm.h |  4 ++++
 target/arm/kvm.c     |  4 ++++
 target/i386/kvm.c    |  4 ++++
 target/mips/kvm.c    |  4 ++++
 target/ppc/kvm.c     |  4 ++++
 target/s390x/kvm.c   |  4 ++++
 8 files changed, 35 insertions(+)

Comments

Philippe Mathieu-Daudé Nov. 21, 2018, 6:40 p.m. UTC | #1
On 21/11/18 19:13, Fabiano Rosas wrote:
> This will be used in a future patch to implement an
> architecture-specific single step mechanism for POWER.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/kvm/kvm-all.c  | 10 ++++++++++
>   exec.c               |  1 +
>   include/sysemu/kvm.h |  4 ++++
>   target/arm/kvm.c     |  4 ++++
>   target/i386/kvm.c    |  4 ++++
>   target/mips/kvm.c    |  4 ++++
>   target/ppc/kvm.c     |  4 ++++
>   target/s390x/kvm.c   |  4 ++++
>   8 files changed, 35 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 4880a05399..4fb7199a15 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2313,6 +2313,11 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>       return data.err;
>   }
>   
> +void kvm_set_singlestep(CPUState *cs, int enabled)
> +{
> +    kvm_arch_set_singlestep(cs, enabled);
> +}
> +
>   int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
>                             target_ulong len, int type)
>   {
> @@ -2439,6 +2444,11 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
>   void kvm_remove_all_breakpoints(CPUState *cpu)
>   {
>   }
> +
> +void kvm_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>   #endif /* !KVM_CAP_SET_GUEST_DEBUG */
>   
>   static int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset)
> diff --git a/exec.c b/exec.c
> index bb6170dbff..55614822c3 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1233,6 +1233,7 @@ void cpu_single_step(CPUState *cpu, int enabled)
>       if (cpu->singlestep_enabled != enabled) {
>           cpu->singlestep_enabled = enabled;
>           if (kvm_enabled()) {
> +            kvm_set_singlestep(cpu, enabled);
>               kvm_update_guest_debug(cpu, 0);
>           } else {
>               /* must flush all the translated code to avoid inconsistencies */
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 97d8d9d0d5..a01a8d58dd 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -259,6 +259,8 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
>   void kvm_remove_all_breakpoints(CPUState *cpu);
>   int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
>   
> +void kvm_set_singlestep(CPUState *cpu, int enabled);
> +
>   int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>   int kvm_on_sigbus(int code, void *addr);
>   
> @@ -431,6 +433,8 @@ void kvm_arch_remove_all_hw_breakpoints(void);
>   
>   void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg);
>   
> +void kvm_arch_set_singlestep(CPUState *cpu, int enabled);
> +
>   bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
>   
>   int kvm_check_extension(KVMState *s, unsigned int extension);
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 44dd0ce6ce..dd8e43ab7e 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -670,6 +670,10 @@ int kvm_arch_process_async_events(CPUState *cs)
>       return 0;
>   }
>   
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>   /* The #ifdef protections are until 32bit headers are imported and can
>    * be removed once both 32 and 64 bit reach feature parity.
>    */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index f524e7d929..ba56f2ee1f 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -3521,6 +3521,10 @@ static int kvm_handle_debug(X86CPU *cpu,
>       return ret;
>   }
>   
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>   void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
>   {
>       const uint8_t type_code[] = {
> diff --git a/target/mips/kvm.c b/target/mips/kvm.c
> index 8e72850962..8035262131 100644
> --- a/target/mips/kvm.c
> +++ b/target/mips/kvm.c
> @@ -119,6 +119,10 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
>       return 0;
>   }
>   
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>   static inline int cpu_mips_io_interrupts_pending(MIPSCPU *cpu)
>   {
>       CPUMIPSState *env = &cpu->env;
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index f81327d6cd..9d0b4f1f3f 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -1551,6 +1551,10 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>       nb_hw_breakpoint = nb_hw_watchpoint = 0;
>   }
>   
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>   void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
>   {
>       int n;
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 2ebf26adfe..4bde183458 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -975,6 +975,10 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>       hw_breakpoints = NULL;
>   }
>   
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>   void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
>   {
>       int i;
>
Cornelia Huck Nov. 23, 2018, 8:57 a.m. UTC | #2
On Wed, 21 Nov 2018 16:13:46 -0200
Fabiano Rosas <farosas@linux.ibm.com> wrote:

> This will be used in a future patch to implement an
> architecture-specific single step mechanism for POWER.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  accel/kvm/kvm-all.c  | 10 ++++++++++
>  exec.c               |  1 +
>  include/sysemu/kvm.h |  4 ++++
>  target/arm/kvm.c     |  4 ++++
>  target/i386/kvm.c    |  4 ++++
>  target/mips/kvm.c    |  4 ++++
>  target/ppc/kvm.c     |  4 ++++
>  target/s390x/kvm.c   |  4 ++++
>  8 files changed, 35 insertions(+)
> 

> @@ -431,6 +433,8 @@ void kvm_arch_remove_all_hw_breakpoints(void);
>  
>  void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg);
>  
> +void kvm_arch_set_singlestep(CPUState *cpu, int enabled);

Might be useful to add a comment here that describes what common code
expects the arch-specific function to do here so they don't step on
each others toes.

> +
>  bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
>  
>  int kvm_check_extension(KVMState *s, unsigned int extension);
David Gibson Nov. 25, 2018, 7:54 a.m. UTC | #3
On Wed, Nov 21, 2018 at 04:13:46PM -0200, Fabiano Rosas wrote:
> This will be used in a future patch to implement an
> architecture-specific single step mechanism for POWER.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  accel/kvm/kvm-all.c  | 10 ++++++++++
>  exec.c               |  1 +
>  include/sysemu/kvm.h |  4 ++++
>  target/arm/kvm.c     |  4 ++++
>  target/i386/kvm.c    |  4 ++++
>  target/mips/kvm.c    |  4 ++++
>  target/ppc/kvm.c     |  4 ++++
>  target/s390x/kvm.c   |  4 ++++
>  8 files changed, 35 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 4880a05399..4fb7199a15 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2313,6 +2313,11 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>      return data.err;
>  }
>  
> +void kvm_set_singlestep(CPUState *cs, int enabled)
> +{
> +    kvm_arch_set_singlestep(cs, enabled);
> +}
> +
>  int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
>                            target_ulong len, int type)
>  {
> @@ -2439,6 +2444,11 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
>  void kvm_remove_all_breakpoints(CPUState *cpu)
>  {
>  }
> +
> +void kvm_set_singlestep(CPUState *cs, int enabled)
> +{
> +}

You could use stubs to avoid having to put this empty implementation
in every arch.

It also seems like it might be a good idea to report an error here,
rather than having set single step silently do nothing on arches which
don't support it yet.

>  #endif /* !KVM_CAP_SET_GUEST_DEBUG */
>  
>  static int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset)
> diff --git a/exec.c b/exec.c
> index bb6170dbff..55614822c3 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1233,6 +1233,7 @@ void cpu_single_step(CPUState *cpu, int enabled)
>      if (cpu->singlestep_enabled != enabled) {
>          cpu->singlestep_enabled = enabled;
>          if (kvm_enabled()) {
> +            kvm_set_singlestep(cpu, enabled);
>              kvm_update_guest_debug(cpu, 0);
>          } else {
>              /* must flush all the translated code to avoid inconsistencies */
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 97d8d9d0d5..a01a8d58dd 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -259,6 +259,8 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
>  void kvm_remove_all_breakpoints(CPUState *cpu);
>  int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
>  
> +void kvm_set_singlestep(CPUState *cpu, int enabled);
> +
>  int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>  int kvm_on_sigbus(int code, void *addr);
>  
> @@ -431,6 +433,8 @@ void kvm_arch_remove_all_hw_breakpoints(void);
>  
>  void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg);
>  
> +void kvm_arch_set_singlestep(CPUState *cpu, int enabled);
> +
>  bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
>  
>  int kvm_check_extension(KVMState *s, unsigned int extension);
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 44dd0ce6ce..dd8e43ab7e 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -670,6 +670,10 @@ int kvm_arch_process_async_events(CPUState *cs)
>      return 0;
>  }
>  
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>  /* The #ifdef protections are until 32bit headers are imported and can
>   * be removed once both 32 and 64 bit reach feature parity.
>   */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index f524e7d929..ba56f2ee1f 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -3521,6 +3521,10 @@ static int kvm_handle_debug(X86CPU *cpu,
>      return ret;
>  }
>  
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>  void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
>  {
>      const uint8_t type_code[] = {
> diff --git a/target/mips/kvm.c b/target/mips/kvm.c
> index 8e72850962..8035262131 100644
> --- a/target/mips/kvm.c
> +++ b/target/mips/kvm.c
> @@ -119,6 +119,10 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
>      return 0;
>  }
>  
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>  static inline int cpu_mips_io_interrupts_pending(MIPSCPU *cpu)
>  {
>      CPUMIPSState *env = &cpu->env;
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index f81327d6cd..9d0b4f1f3f 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -1551,6 +1551,10 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>      nb_hw_breakpoint = nb_hw_watchpoint = 0;
>  }
>  
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>  void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
>  {
>      int n;
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 2ebf26adfe..4bde183458 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -975,6 +975,10 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>      hw_breakpoints = NULL;
>  }
>  
> +void kvm_arch_set_singlestep(CPUState *cs, int enabled)
> +{
> +}
> +
>  void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
>  {
>      int i;
diff mbox series

Patch

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 4880a05399..4fb7199a15 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2313,6 +2313,11 @@  int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
     return data.err;
 }
 
+void kvm_set_singlestep(CPUState *cs, int enabled)
+{
+    kvm_arch_set_singlestep(cs, enabled);
+}
+
 int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
                           target_ulong len, int type)
 {
@@ -2439,6 +2444,11 @@  int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
 void kvm_remove_all_breakpoints(CPUState *cpu)
 {
 }
+
+void kvm_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
 #endif /* !KVM_CAP_SET_GUEST_DEBUG */
 
 static int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset)
diff --git a/exec.c b/exec.c
index bb6170dbff..55614822c3 100644
--- a/exec.c
+++ b/exec.c
@@ -1233,6 +1233,7 @@  void cpu_single_step(CPUState *cpu, int enabled)
     if (cpu->singlestep_enabled != enabled) {
         cpu->singlestep_enabled = enabled;
         if (kvm_enabled()) {
+            kvm_set_singlestep(cpu, enabled);
             kvm_update_guest_debug(cpu, 0);
         } else {
             /* must flush all the translated code to avoid inconsistencies */
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 97d8d9d0d5..a01a8d58dd 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -259,6 +259,8 @@  int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
 void kvm_remove_all_breakpoints(CPUState *cpu);
 int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
 
+void kvm_set_singlestep(CPUState *cpu, int enabled);
+
 int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
 int kvm_on_sigbus(int code, void *addr);
 
@@ -431,6 +433,8 @@  void kvm_arch_remove_all_hw_breakpoints(void);
 
 void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg);
 
+void kvm_arch_set_singlestep(CPUState *cpu, int enabled);
+
 bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
 
 int kvm_check_extension(KVMState *s, unsigned int extension);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 44dd0ce6ce..dd8e43ab7e 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -670,6 +670,10 @@  int kvm_arch_process_async_events(CPUState *cs)
     return 0;
 }
 
+void kvm_arch_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
 /* The #ifdef protections are until 32bit headers are imported and can
  * be removed once both 32 and 64 bit reach feature parity.
  */
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f524e7d929..ba56f2ee1f 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -3521,6 +3521,10 @@  static int kvm_handle_debug(X86CPU *cpu,
     return ret;
 }
 
+void kvm_arch_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
 void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
 {
     const uint8_t type_code[] = {
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 8e72850962..8035262131 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -119,6 +119,10 @@  int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
     return 0;
 }
 
+void kvm_arch_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
 static inline int cpu_mips_io_interrupts_pending(MIPSCPU *cpu)
 {
     CPUMIPSState *env = &cpu->env;
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index f81327d6cd..9d0b4f1f3f 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1551,6 +1551,10 @@  void kvm_arch_remove_all_hw_breakpoints(void)
     nb_hw_breakpoint = nb_hw_watchpoint = 0;
 }
 
+void kvm_arch_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
 void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
 {
     int n;
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 2ebf26adfe..4bde183458 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -975,6 +975,10 @@  void kvm_arch_remove_all_hw_breakpoints(void)
     hw_breakpoints = NULL;
 }
 
+void kvm_arch_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
 void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
 {
     int i;