Patchwork [03/21] qemu-kvm: Clean up register access API

login
register
mail settings
Submitter Jan Kiszka
Date Feb. 2, 2010, 8:18 a.m.
Message ID <3d834dc34d1566a2409115f6afe3f43f49cbe2cd.1265098706.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/44245/
State New
Headers show

Comments

Jan Kiszka - Feb. 2, 2010, 8:18 a.m.
qemu-kvm's functios for accessing the VCPU registers are
kvm_arch_load/save_regs. Use them directly instead of going through
various wrappers. Specifically, we do not need on_vcpu wrapping as all
users either already run in the related thread or call while the vm is
stopped.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-kvm.c            |   37 +++----------------------------------
 qemu-kvm.h            |   11 -----------
 target-ia64/machine.c |    4 ++--
 3 files changed, 5 insertions(+), 47 deletions(-)
Gleb Natapov - Feb. 2, 2010, 11:06 a.m.
On Tue, Feb 02, 2010 at 09:18:49AM +0100, Jan Kiszka wrote:
> qemu-kvm's functios for accessing the VCPU registers are
> kvm_arch_load/save_regs. Use them directly instead of going through
> various wrappers. Specifically, we do not need on_vcpu wrapping as all
> users either already run in the related thread or call while the vm is
> stopped.
> 
Can we put check for that into those functions just to be sure.
Something like:
	assert(!vm_stopped && env->thread_id != pthread_id())

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  qemu-kvm.c            |   37 +++----------------------------------
>  qemu-kvm.h            |   11 -----------
>  target-ia64/machine.c |    4 ++--
>  3 files changed, 5 insertions(+), 47 deletions(-)
> 
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index a305907..97c098c 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -862,7 +862,7 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
>      kvm_arch_pre_run(env, env->kvm_run);
>  
>      if (env->kvm_cpu_state.regs_modified) {
> -        kvm_arch_put_registers(env);
> +        kvm_arch_load_regs(env);
>          env->kvm_cpu_state.regs_modified = 0;
>      }
>  
> @@ -1532,16 +1532,11 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>          qemu_cond_wait(&qemu_work_cond);
>  }
>  
> -void kvm_arch_get_registers(CPUState *env)
> -{
> -	kvm_arch_save_regs(env);
> -}
> -
>  static void do_kvm_cpu_synchronize_state(void *_env)
>  {
>      CPUState *env = _env;
>      if (!env->kvm_cpu_state.regs_modified) {
> -        kvm_arch_get_registers(env);
> +        kvm_arch_save_regs(env);
>          env->kvm_cpu_state.regs_modified = 1;
>      }
>  }
> @@ -1584,32 +1579,6 @@ void kvm_update_interrupt_request(CPUState *env)
>      }
>  }
>  
> -static void kvm_do_load_registers(void *_env)
> -{
> -    CPUState *env = _env;
> -
> -    kvm_arch_load_regs(env);
> -}
> -
> -void kvm_load_registers(CPUState *env)
> -{
> -    if (kvm_enabled() && qemu_system_ready)
> -        on_vcpu(env, kvm_do_load_registers, env);
> -}
> -
> -static void kvm_do_save_registers(void *_env)
> -{
> -    CPUState *env = _env;
> -
> -    kvm_arch_save_regs(env);
> -}
> -
> -void kvm_save_registers(CPUState *env)
> -{
> -    if (kvm_enabled())
> -        on_vcpu(env, kvm_do_save_registers, env);
> -}
> -
>  static void kvm_do_load_mpstate(void *_env)
>  {
>      CPUState *env = _env;
> @@ -2379,7 +2348,7 @@ static void kvm_invoke_set_guest_debug(void *data)
>      struct kvm_set_guest_debug_data *dbg_data = data;
>  
>      if (cpu_single_env->kvm_cpu_state.regs_modified) {
> -        kvm_arch_put_registers(cpu_single_env);
> +        kvm_arch_save_regs(cpu_single_env);
>          cpu_single_env->kvm_cpu_state.regs_modified = 0;
>      }
>      dbg_data->err =
> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index 6b3e5a1..1354227 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -902,8 +902,6 @@ int kvm_main_loop(void);
>  int kvm_init_ap(void);
>  #ifndef QEMU_KVM_NO_CPU
>  int kvm_vcpu_inited(CPUState *env);
> -void kvm_load_registers(CPUState *env);
> -void kvm_save_registers(CPUState *env);
>  void kvm_load_mpstate(CPUState *env);
>  void kvm_save_mpstate(CPUState *env);
>  int kvm_cpu_exec(CPUState *env);
> @@ -1068,8 +1066,6 @@ void kvm_load_tsc(CPUState *env);
>  #ifdef TARGET_I386
>  #define qemu_kvm_has_pit_state2() (0)
>  #endif
> -#define kvm_load_registers(env) do {} while(0)
> -#define kvm_save_registers(env) do {} while(0)
>  #define kvm_save_mpstate(env)   do {} while(0)
>  #define qemu_kvm_cpu_stop(env) do {} while(0)
>  static inline void kvm_init_vcpu(CPUState *env)
> @@ -1098,13 +1094,6 @@ static inline int kvm_sync_vcpus(void)
>  }
>  
>  #ifndef QEMU_KVM_NO_CPU
> -void kvm_arch_get_registers(CPUState *env);
> -
> -static inline void kvm_arch_put_registers(CPUState *env)
> -{
> -    kvm_load_registers(env);
> -}
> -
>  void kvm_cpu_synchronize_state(CPUState *env);
>  
>  static inline void cpu_synchronize_state(CPUState *env)
> diff --git a/target-ia64/machine.c b/target-ia64/machine.c
> index 70ef379..7d29575 100644
> --- a/target-ia64/machine.c
> +++ b/target-ia64/machine.c
> @@ -9,7 +9,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>      CPUState *env = opaque;
>  
>      if (kvm_enabled()) {
> -        kvm_save_registers(env);
> +        kvm_arch_save_regs(env);
>          kvm_arch_save_mpstate(env);
>      }
>  }
> @@ -19,7 +19,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>      CPUState *env = opaque;
>  
>      if (kvm_enabled()) {
> -        kvm_load_registers(env);
> +        kvm_arch_load_regs(env);
>          kvm_arch_load_mpstate(env);
>      }
>      return 0;
> -- 
> 1.6.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.
Jan Kiszka - Feb. 2, 2010, 11:18 a.m.
Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 09:18:49AM +0100, Jan Kiszka wrote:
>> qemu-kvm's functios for accessing the VCPU registers are
>> kvm_arch_load/save_regs. Use them directly instead of going through
>> various wrappers. Specifically, we do not need on_vcpu wrapping as all
>> users either already run in the related thread or call while the vm is
>> stopped.
>>
> Can we put check for that into those functions just to be sure.
> Something like:
> 	assert(!vm_stopped && env->thread_id != pthread_id())
> 

Good idea. Will add this to a potential v2 or send an add-on patch. We
just need something else than vm_stopped (for reset, only the vcpu
threads are stopped, not the vm), probably env->stopped in qemu-kvm.

>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  qemu-kvm.c            |   37 +++----------------------------------
>>  qemu-kvm.h            |   11 -----------
>>  target-ia64/machine.c |    4 ++--
>>  3 files changed, 5 insertions(+), 47 deletions(-)
>>
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index a305907..97c098c 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -862,7 +862,7 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
>>      kvm_arch_pre_run(env, env->kvm_run);
>>  
>>      if (env->kvm_cpu_state.regs_modified) {
>> -        kvm_arch_put_registers(env);
>> +        kvm_arch_load_regs(env);
>>          env->kvm_cpu_state.regs_modified = 0;
>>      }
>>  
>> @@ -1532,16 +1532,11 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>>          qemu_cond_wait(&qemu_work_cond);
>>  }
>>  
>> -void kvm_arch_get_registers(CPUState *env)
>> -{
>> -	kvm_arch_save_regs(env);
>> -}
>> -
>>  static void do_kvm_cpu_synchronize_state(void *_env)
>>  {
>>      CPUState *env = _env;
>>      if (!env->kvm_cpu_state.regs_modified) {
>> -        kvm_arch_get_registers(env);
>> +        kvm_arch_save_regs(env);
>>          env->kvm_cpu_state.regs_modified = 1;
>>      }
>>  }
>> @@ -1584,32 +1579,6 @@ void kvm_update_interrupt_request(CPUState *env)
>>      }
>>  }
>>  
>> -static void kvm_do_load_registers(void *_env)
>> -{
>> -    CPUState *env = _env;
>> -
>> -    kvm_arch_load_regs(env);
>> -}
>> -
>> -void kvm_load_registers(CPUState *env)
>> -{
>> -    if (kvm_enabled() && qemu_system_ready)
>> -        on_vcpu(env, kvm_do_load_registers, env);
>> -}
>> -
>> -static void kvm_do_save_registers(void *_env)
>> -{
>> -    CPUState *env = _env;
>> -
>> -    kvm_arch_save_regs(env);
>> -}
>> -
>> -void kvm_save_registers(CPUState *env)
>> -{
>> -    if (kvm_enabled())
>> -        on_vcpu(env, kvm_do_save_registers, env);
>> -}
>> -
>>  static void kvm_do_load_mpstate(void *_env)
>>  {
>>      CPUState *env = _env;
>> @@ -2379,7 +2348,7 @@ static void kvm_invoke_set_guest_debug(void *data)
>>      struct kvm_set_guest_debug_data *dbg_data = data;
>>  
>>      if (cpu_single_env->kvm_cpu_state.regs_modified) {
>> -        kvm_arch_put_registers(cpu_single_env);
>> +        kvm_arch_save_regs(cpu_single_env);
>>          cpu_single_env->kvm_cpu_state.regs_modified = 0;
>>      }
>>      dbg_data->err =
>> diff --git a/qemu-kvm.h b/qemu-kvm.h
>> index 6b3e5a1..1354227 100644
>> --- a/qemu-kvm.h
>> +++ b/qemu-kvm.h
>> @@ -902,8 +902,6 @@ int kvm_main_loop(void);
>>  int kvm_init_ap(void);
>>  #ifndef QEMU_KVM_NO_CPU
>>  int kvm_vcpu_inited(CPUState *env);
>> -void kvm_load_registers(CPUState *env);
>> -void kvm_save_registers(CPUState *env);
>>  void kvm_load_mpstate(CPUState *env);
>>  void kvm_save_mpstate(CPUState *env);
>>  int kvm_cpu_exec(CPUState *env);
>> @@ -1068,8 +1066,6 @@ void kvm_load_tsc(CPUState *env);
>>  #ifdef TARGET_I386
>>  #define qemu_kvm_has_pit_state2() (0)
>>  #endif
>> -#define kvm_load_registers(env) do {} while(0)
>> -#define kvm_save_registers(env) do {} while(0)
>>  #define kvm_save_mpstate(env)   do {} while(0)
>>  #define qemu_kvm_cpu_stop(env) do {} while(0)
>>  static inline void kvm_init_vcpu(CPUState *env)
>> @@ -1098,13 +1094,6 @@ static inline int kvm_sync_vcpus(void)
>>  }
>>  
>>  #ifndef QEMU_KVM_NO_CPU
>> -void kvm_arch_get_registers(CPUState *env);
>> -
>> -static inline void kvm_arch_put_registers(CPUState *env)
>> -{
>> -    kvm_load_registers(env);
>> -}
>> -
>>  void kvm_cpu_synchronize_state(CPUState *env);
>>  
>>  static inline void cpu_synchronize_state(CPUState *env)
>> diff --git a/target-ia64/machine.c b/target-ia64/machine.c
>> index 70ef379..7d29575 100644
>> --- a/target-ia64/machine.c
>> +++ b/target-ia64/machine.c
>> @@ -9,7 +9,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>>      CPUState *env = opaque;
>>  
>>      if (kvm_enabled()) {
>> -        kvm_save_registers(env);
>> +        kvm_arch_save_regs(env);
>>          kvm_arch_save_mpstate(env);
>>      }
>>  }
>> @@ -19,7 +19,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>>      CPUState *env = opaque;
>>  
>>      if (kvm_enabled()) {
>> -        kvm_load_registers(env);
>> +        kvm_arch_load_regs(env);
>>          kvm_arch_load_mpstate(env);
>>      }
>>      return 0;
>> -- 
>> 1.6.0.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> 			Gleb.

Jan

Patch

diff --git a/qemu-kvm.c b/qemu-kvm.c
index a305907..97c098c 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -862,7 +862,7 @@  int pre_kvm_run(kvm_context_t kvm, CPUState *env)
     kvm_arch_pre_run(env, env->kvm_run);
 
     if (env->kvm_cpu_state.regs_modified) {
-        kvm_arch_put_registers(env);
+        kvm_arch_load_regs(env);
         env->kvm_cpu_state.regs_modified = 0;
     }
 
@@ -1532,16 +1532,11 @@  static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
         qemu_cond_wait(&qemu_work_cond);
 }
 
-void kvm_arch_get_registers(CPUState *env)
-{
-	kvm_arch_save_regs(env);
-}
-
 static void do_kvm_cpu_synchronize_state(void *_env)
 {
     CPUState *env = _env;
     if (!env->kvm_cpu_state.regs_modified) {
-        kvm_arch_get_registers(env);
+        kvm_arch_save_regs(env);
         env->kvm_cpu_state.regs_modified = 1;
     }
 }
@@ -1584,32 +1579,6 @@  void kvm_update_interrupt_request(CPUState *env)
     }
 }
 
-static void kvm_do_load_registers(void *_env)
-{
-    CPUState *env = _env;
-
-    kvm_arch_load_regs(env);
-}
-
-void kvm_load_registers(CPUState *env)
-{
-    if (kvm_enabled() && qemu_system_ready)
-        on_vcpu(env, kvm_do_load_registers, env);
-}
-
-static void kvm_do_save_registers(void *_env)
-{
-    CPUState *env = _env;
-
-    kvm_arch_save_regs(env);
-}
-
-void kvm_save_registers(CPUState *env)
-{
-    if (kvm_enabled())
-        on_vcpu(env, kvm_do_save_registers, env);
-}
-
 static void kvm_do_load_mpstate(void *_env)
 {
     CPUState *env = _env;
@@ -2379,7 +2348,7 @@  static void kvm_invoke_set_guest_debug(void *data)
     struct kvm_set_guest_debug_data *dbg_data = data;
 
     if (cpu_single_env->kvm_cpu_state.regs_modified) {
-        kvm_arch_put_registers(cpu_single_env);
+        kvm_arch_save_regs(cpu_single_env);
         cpu_single_env->kvm_cpu_state.regs_modified = 0;
     }
     dbg_data->err =
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 6b3e5a1..1354227 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -902,8 +902,6 @@  int kvm_main_loop(void);
 int kvm_init_ap(void);
 #ifndef QEMU_KVM_NO_CPU
 int kvm_vcpu_inited(CPUState *env);
-void kvm_load_registers(CPUState *env);
-void kvm_save_registers(CPUState *env);
 void kvm_load_mpstate(CPUState *env);
 void kvm_save_mpstate(CPUState *env);
 int kvm_cpu_exec(CPUState *env);
@@ -1068,8 +1066,6 @@  void kvm_load_tsc(CPUState *env);
 #ifdef TARGET_I386
 #define qemu_kvm_has_pit_state2() (0)
 #endif
-#define kvm_load_registers(env) do {} while(0)
-#define kvm_save_registers(env) do {} while(0)
 #define kvm_save_mpstate(env)   do {} while(0)
 #define qemu_kvm_cpu_stop(env) do {} while(0)
 static inline void kvm_init_vcpu(CPUState *env)
@@ -1098,13 +1094,6 @@  static inline int kvm_sync_vcpus(void)
 }
 
 #ifndef QEMU_KVM_NO_CPU
-void kvm_arch_get_registers(CPUState *env);
-
-static inline void kvm_arch_put_registers(CPUState *env)
-{
-    kvm_load_registers(env);
-}
-
 void kvm_cpu_synchronize_state(CPUState *env);
 
 static inline void cpu_synchronize_state(CPUState *env)
diff --git a/target-ia64/machine.c b/target-ia64/machine.c
index 70ef379..7d29575 100644
--- a/target-ia64/machine.c
+++ b/target-ia64/machine.c
@@ -9,7 +9,7 @@  void cpu_save(QEMUFile *f, void *opaque)
     CPUState *env = opaque;
 
     if (kvm_enabled()) {
-        kvm_save_registers(env);
+        kvm_arch_save_regs(env);
         kvm_arch_save_mpstate(env);
     }
 }
@@ -19,7 +19,7 @@  int cpu_load(QEMUFile *f, void *opaque, int version_id)
     CPUState *env = opaque;
 
     if (kvm_enabled()) {
-        kvm_load_registers(env);
+        kvm_arch_load_regs(env);
         kvm_arch_load_mpstate(env);
     }
     return 0;