Patchwork kvm: Change prototype of kvm_update_guest_debug

login
register
mail settings
Submitter Stefan Weil
Date July 25, 2013, 6:50 p.m.
Message ID <1374778221-24011-1-git-send-email-sw@weilnetz.de>
Download mbox | patch
Permalink /patch/261873/
State Accepted
Headers show

Comments

Stefan Weil - July 25, 2013, 6:50 p.m.
Passing a CPUState pointer instead of a CPUArchState pointer eliminates
the last target dependent data type in sysemu/kvm.h.

It also simplifies the code.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

With this patch (and a previous one which is already queued),
kvm.h can be included without errors from "poisoned" names.

 exec.c               |    5 ++---
 include/sysemu/kvm.h |    2 +-
 kvm-all.c            |   17 +++++------------
 kvm-stub.c           |    2 +-
 target-i386/kvm.c    |    2 +-
 5 files changed, 10 insertions(+), 18 deletions(-)
Andreas Färber - July 26, 2013, 12:33 p.m.
Am 25.07.2013 20:50, schrieb Stefan Weil:
> Passing a CPUState pointer instead of a CPUArchState pointer eliminates
> the last target dependent data type in sysemu/kvm.h.
> 
> It also simplifies the code.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> With this patch (and a previous one which is already queued),
> kvm.h can be included without errors from "poisoned" names.

Thanks for doing this, it was on my TODO list. If Paolo acks, I would
happily put that in qom-cpu for final 1.6 pull.

Which other queued patch are you referring to?

Andreas

> 
>  exec.c               |    5 ++---
>  include/sysemu/kvm.h |    2 +-
>  kvm-all.c            |   17 +++++------------
>  kvm-stub.c           |    2 +-
>  target-i386/kvm.c    |    2 +-
>  5 files changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 7997002..68ffb6b 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -590,15 +590,14 @@ void cpu_breakpoint_remove_all(CPUArchState *env, int mask)
>  void cpu_single_step(CPUState *cpu, int enabled)
>  {
>  #if defined(TARGET_HAS_ICE)
> -    CPUArchState *env = cpu->env_ptr;
> -
>      if (cpu->singlestep_enabled != enabled) {
>          cpu->singlestep_enabled = enabled;
>          if (kvm_enabled()) {
> -            kvm_update_guest_debug(env, 0);
> +            kvm_update_guest_debug(cpu, 0);
>          } else {
>              /* must flush all the translated code to avoid inconsistencies */
>              /* XXX: only flush what is necessary */
> +            CPUArchState *env = cpu->env_ptr;
>              tb_flush(env);
>          }
>      }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index f8ac448..de74411 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -174,7 +174,7 @@ int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
>  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(CPUArchState *env, unsigned long reinject_trap);
> +int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
>  #ifndef _WIN32
>  int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset);
>  #endif
> diff --git a/kvm-all.c b/kvm-all.c
> index 4fb4ccb..716860f 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1883,9 +1883,8 @@ static void kvm_invoke_set_guest_debug(void *data)
>                                     &dbg_data->dbg);
>  }
>  
> -int kvm_update_guest_debug(CPUArchState *env, unsigned long reinject_trap)
> +int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>  {
> -    CPUState *cpu = ENV_GET_CPU(env);
>      struct kvm_set_guest_debug_data data;
>  
>      data.dbg.control = reinject_trap;
> @@ -1935,9 +1934,7 @@ int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
>      }
>  
>      for (cpu = first_cpu; cpu != NULL; cpu = cpu->next_cpu) {
> -        CPUArchState *env = cpu->env_ptr;
> -
> -        err = kvm_update_guest_debug(env, 0);
> +        err = kvm_update_guest_debug(cpu, 0);
>          if (err) {
>              return err;
>          }
> @@ -1977,9 +1974,7 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
>      }
>  
>      for (cpu = first_cpu; cpu != NULL; cpu = cpu->next_cpu) {
> -        CPUArchState *env = cpu->env_ptr;
> -
> -        err = kvm_update_guest_debug(env, 0);
> +        err = kvm_update_guest_debug(cpu, 0);
>          if (err) {
>              return err;
>          }
> @@ -2007,15 +2002,13 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
>      kvm_arch_remove_all_hw_breakpoints();
>  
>      for (cpu = first_cpu; cpu != NULL; cpu = cpu->next_cpu) {
> -        CPUArchState *env = cpu->env_ptr;
> -
> -        kvm_update_guest_debug(env, 0);
> +        kvm_update_guest_debug(cpu, 0);
>      }
>  }
>  
>  #else /* !KVM_CAP_SET_GUEST_DEBUG */
>  
> -int kvm_update_guest_debug(CPUArchState *env, unsigned long reinject_trap)
> +int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>  {
>      return -EINVAL;
>  }
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 7b2233a..771360b 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -78,7 +78,7 @@ void kvm_setup_guest_memory(void *start, size_t size)
>  {
>  }
>  
> -int kvm_update_guest_debug(CPUArchState *env, unsigned long reinject_trap)
> +int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>  {
>      return -ENOSYS;
>  }
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 3c9d10a..376fc70 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1618,7 +1618,7 @@ static int kvm_guest_debug_workarounds(X86CPU *cpu)
>       */
>      if (reinject_trap ||
>          (!kvm_has_robust_singlestep() && cs->singlestep_enabled)) {
> -        ret = kvm_update_guest_debug(env, reinject_trap);
> +        ret = kvm_update_guest_debug(cs, reinject_trap);
>      }
>      return ret;
>  }
>
Paolo Bonzini - July 26, 2013, 12:45 p.m.
Il 26/07/2013 14:33, Andreas Färber ha scritto:
> Am 25.07.2013 20:50, schrieb Stefan Weil:
>> Passing a CPUState pointer instead of a CPUArchState pointer eliminates
>> the last target dependent data type in sysemu/kvm.h.
>>
>> It also simplifies the code.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> With this patch (and a previous one which is already queued),
>> kvm.h can be included without errors from "poisoned" names.
> 
> Thanks for doing this, it was on my TODO list. If Paolo acks, I would
> happily put that in qom-cpu for final 1.6 pull.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Andreas Färber - July 26, 2013, 3:46 p.m.
Am 26.07.2013 14:45, schrieb Paolo Bonzini:
> Il 26/07/2013 14:33, Andreas Färber ha scritto:
>> Am 25.07.2013 20:50, schrieb Stefan Weil:
>>> Passing a CPUState pointer instead of a CPUArchState pointer eliminates
>>> the last target dependent data type in sysemu/kvm.h.
>>>
>>> It also simplifies the code.
>>>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>>
>>> With this patch (and a previous one which is already queued),
>>> kvm.h can be included without errors from "poisoned" names.
>>
>> Thanks for doing this, it was on my TODO list. If Paolo acks, I would
>> happily put that in qom-cpu for final 1.6 pull.
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas
Stefan Weil - July 26, 2013, 6:14 p.m.
Am 26.07.2013 14:33, schrieb Andreas Färber:
> Am 25.07.2013 20:50, schrieb Stefan Weil:
>> Passing a CPUState pointer instead of a CPUArchState pointer eliminates
>> the last target dependent data type in sysemu/kvm.h.
>>
>> It also simplifies the code.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> With this patch (and a previous one which is already queued),
>> kvm.h can be included without errors from "poisoned" names.
> Thanks for doing this, it was on my TODO list. If Paolo acks, I would
> happily put that in qom-cpu for final 1.6 pull.
>
> Which other queued patch are you referring to?
>
> Andreas

http://patchwork.ozlabs.org/patch/260525/ removes 'env' from poison.h.

This allows moving the kvm stubs to the stubs directory with less trickery
(see http://patchwork.ozlabs.org/patch/260512/ for the old version).

Stefan

Patch

diff --git a/exec.c b/exec.c
index 7997002..68ffb6b 100644
--- a/exec.c
+++ b/exec.c
@@ -590,15 +590,14 @@  void cpu_breakpoint_remove_all(CPUArchState *env, int mask)
 void cpu_single_step(CPUState *cpu, int enabled)
 {
 #if defined(TARGET_HAS_ICE)
-    CPUArchState *env = cpu->env_ptr;
-
     if (cpu->singlestep_enabled != enabled) {
         cpu->singlestep_enabled = enabled;
         if (kvm_enabled()) {
-            kvm_update_guest_debug(env, 0);
+            kvm_update_guest_debug(cpu, 0);
         } else {
             /* must flush all the translated code to avoid inconsistencies */
             /* XXX: only flush what is necessary */
+            CPUArchState *env = cpu->env_ptr;
             tb_flush(env);
         }
     }
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index f8ac448..de74411 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -174,7 +174,7 @@  int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
 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(CPUArchState *env, unsigned long reinject_trap);
+int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
 #ifndef _WIN32
 int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset);
 #endif
diff --git a/kvm-all.c b/kvm-all.c
index 4fb4ccb..716860f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1883,9 +1883,8 @@  static void kvm_invoke_set_guest_debug(void *data)
                                    &dbg_data->dbg);
 }
 
-int kvm_update_guest_debug(CPUArchState *env, unsigned long reinject_trap)
+int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
     struct kvm_set_guest_debug_data data;
 
     data.dbg.control = reinject_trap;
@@ -1935,9 +1934,7 @@  int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
     }
 
     for (cpu = first_cpu; cpu != NULL; cpu = cpu->next_cpu) {
-        CPUArchState *env = cpu->env_ptr;
-
-        err = kvm_update_guest_debug(env, 0);
+        err = kvm_update_guest_debug(cpu, 0);
         if (err) {
             return err;
         }
@@ -1977,9 +1974,7 @@  int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
     }
 
     for (cpu = first_cpu; cpu != NULL; cpu = cpu->next_cpu) {
-        CPUArchState *env = cpu->env_ptr;
-
-        err = kvm_update_guest_debug(env, 0);
+        err = kvm_update_guest_debug(cpu, 0);
         if (err) {
             return err;
         }
@@ -2007,15 +2002,13 @@  void kvm_remove_all_breakpoints(CPUState *cpu)
     kvm_arch_remove_all_hw_breakpoints();
 
     for (cpu = first_cpu; cpu != NULL; cpu = cpu->next_cpu) {
-        CPUArchState *env = cpu->env_ptr;
-
-        kvm_update_guest_debug(env, 0);
+        kvm_update_guest_debug(cpu, 0);
     }
 }
 
 #else /* !KVM_CAP_SET_GUEST_DEBUG */
 
-int kvm_update_guest_debug(CPUArchState *env, unsigned long reinject_trap)
+int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
 {
     return -EINVAL;
 }
diff --git a/kvm-stub.c b/kvm-stub.c
index 7b2233a..771360b 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -78,7 +78,7 @@  void kvm_setup_guest_memory(void *start, size_t size)
 {
 }
 
-int kvm_update_guest_debug(CPUArchState *env, unsigned long reinject_trap)
+int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
 {
     return -ENOSYS;
 }
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 3c9d10a..376fc70 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1618,7 +1618,7 @@  static int kvm_guest_debug_workarounds(X86CPU *cpu)
      */
     if (reinject_trap ||
         (!kvm_has_robust_singlestep() && cs->singlestep_enabled)) {
-        ret = kvm_update_guest_debug(env, reinject_trap);
+        ret = kvm_update_guest_debug(cs, reinject_trap);
     }
     return ret;
 }