Re: [PATCH] call kvm_cpu_synchronize_state() on target vcpu

[Jan Kiszka <jan.kiszka@siemens.com>]

Gleb Natapov wrote:
> regs_modified logic doesn't work if io thread calls
> kvm_cpu_synchronize_state() since kvm_arch_get_registers()
> returns only after vcpu thread is back to kernel. Setting
> regs_modified to 1 at this stage causes loading of wrong vcpu
> state on the next vcpu_run().

We need this upstream too, right? Could you file the corresponding patch?

Thanks,
Jan

> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 06efd41..9ab0cec 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -874,14 +874,6 @@ int kvm_set_mpstate(kvm_vcpu_context_t vcpu, struct kvm_mp_state *mp_state)
>  }
>  #endif
>  
> -void kvm_cpu_synchronize_state(CPUState *env)
> -{
> -    if (!env->kvm_cpu_state.regs_modified) {
> -        kvm_arch_get_registers(env);
> -        env->kvm_cpu_state.regs_modified = 1;
> -    }
> -}
> -
>  static int handle_mmio(kvm_vcpu_context_t vcpu)
>  {
>      unsigned long addr = vcpu->run->mmio.phys_addr;
> @@ -1539,6 +1531,22 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>          qemu_cond_wait(&qemu_work_cond);
>  }
>  
> +static void do_kvm_cpu_synchronize_state(void *_env)
> +{
> +    CPUState *env = _env;
> +    if (!env->kvm_cpu_state.regs_modified) {
> +        kvm_arch_save_regs(env);
> +        kvm_arch_load_mpstate(env);
> +        env->kvm_cpu_state.regs_modified = 1;
> +    }
> +}
> +
> +void kvm_cpu_synchronize_state(CPUState *env)
> +{
> +    if (!env->kvm_cpu_state.regs_modified)
> +        on_vcpu(env, do_kvm_cpu_synchronize_state, env);
> +}
> +
>  static void inject_interrupt(void *data)
>  {
>      cpu_interrupt(current_env, (long) data);
> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index 2c1730b..32f74b3 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -1153,12 +1153,6 @@ static inline int kvm_sync_vcpus(void)
>      return 0;
>  }
>  
> -static inline void kvm_arch_get_registers(CPUState *env)
> -{
> -    kvm_save_registers(env);
> -    kvm_save_mpstate(env);
> -}
> -
>  static inline void kvm_arch_put_registers(CPUState *env)
>  {
>      kvm_load_registers(env);
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 4a16887..57c74a2 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -746,7 +746,7 @@ void cpu_dump_state(CPUState *env, FILE *f,
>      static const char *seg_name[6] = { "ES", "CS", "SS", "DS", "FS", "GS" };
>  
>      if (kvm_enabled())
> -        kvm_arch_get_registers(env);
> +        kvm_cpu_synchronize_state(env);
>  
>      eflags = env->eflags;
>  #ifdef TARGET_X86_64
> --
> 			Gleb.

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux