[Qemu-devel] [PATCH 7/7] KVM regsync: Fix do_kvm_cpu_synchronize_state data integrity issue

[Jason J. Herne]

From: "Jason J. Herne" <address@hidden>

Modify syncing algorithm in do_kvm_cpu_synchronize_state to avoid
overwriting previously synced register data by calling
do_kvm_cpu_synchronize_state twice.

The problem occurs if the following sequence of events occurs:
1. kvm_arch_get_registers(env, KVM_REGSYNC_RUNTIME_STATE)
2. Use the runtime state
3. kvm_arch_get_registers(env, KVM_REGSYNC_FULL_STATE) (ignored)
4. Use the full state.

In step 4 the call to kvm_arch_get_registers() does nothing (to avoid squashing
local changes to the runtime registers), but the caller assumes the full
register state is now available.

This is fixed by encoding which registers are synced in env->kvm_vcpu_dirty and
calling kvm_arch_put_registers() to sync local changes back to KVM before
calling kvm_arch_get_registers() if we are expanding the set of synced
registers.

Signed-off-by: Jason J. Herne <address@hidden>
Reviewed-by: Christian Borntraeger <address@hidden>
---
 include/exec/cpu-defs.h |    6 ++++++
 kvm-all.c               |   14 ++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index aea0ece..af3b6aa 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -208,6 +208,12 @@ typedef struct CPUWatchpoint {
     struct KVMState *kvm_state;                                         \
     struct kvm_run *kvm_run;                                            \
     int kvm_fd;                                                         \
+                                                                        \
+    /* Register level indicating which vcpu registers have been synced  \
+       from KVM, are potentially dirty due to local modifications, and  \
+       will need to be written back to KVM.  Valid values are 0, which  \
+       indicates no registers are dirty, or any of the KVM_REGSYNC_*    \
+       constants defined in kvm.h */                                    \
     int kvm_vcpu_dirty;
 
 #endif
diff --git a/kvm-all.c b/kvm-all.c
index aee5bdd..858a636 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -230,7 +230,7 @@ int kvm_init_vcpu(CPUArchState *env)
 
     env->kvm_fd = ret;
     env->kvm_state = s;
-    env->kvm_vcpu_dirty = 1;
+    env->kvm_vcpu_dirty = KVM_REGSYNC_FULL_STATE;
 
     mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
     if (mmap_size < 0) {
@@ -1489,10 +1489,16 @@ void kvm_flush_coalesced_mmio_buffer(void)
 static void do_kvm_cpu_synchronize_state(void *_args)
 {
     struct kvm_cpu_syncstate_args *args = _args;
+    CPUArchState *env = args->env;
+    int register_level = args->register_level;
 
     if (!args->env->kvm_vcpu_dirty) {
-        kvm_arch_get_registers(args->env, args->register_level);
-        args->env->kvm_vcpu_dirty = 1;
+        kvm_arch_get_registers(env, register_level);
+        env->kvm_vcpu_dirty = register_level;
+    } else if (register_level > env->kvm_vcpu_dirty) {
+        kvm_arch_put_registers(env, env->kvm_vcpu_dirty);
+        kvm_arch_get_registers(env, register_level);
+        env->kvm_vcpu_dirty = register_level;
     }
 }
 
@@ -1535,7 +1541,7 @@ int kvm_cpu_exec(CPUArchState *env)
 
     do {
         if (env->kvm_vcpu_dirty) {
-            kvm_arch_put_registers(env, KVM_REGSYNC_RUNTIME_STATE);
+            kvm_arch_put_registers(env, env->kvm_vcpu_dirty);
             env->kvm_vcpu_dirty = 0;
         }
 
-- 
1.7.9.5