From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56688) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tm35z-0003rU-UY for qemu-devel@nongnu.org; Fri, 21 Dec 2012 08:57:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tm35r-0004w7-A5 for qemu-devel@nongnu.org; Fri, 21 Dec 2012 08:57:03 -0500 Received: from e34.co.us.ibm.com ([32.97.110.152]:45078) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tm35r-0004w2-3N for qemu-devel@nongnu.org; Fri, 21 Dec 2012 08:56:55 -0500 Received: from /spool/local by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 21 Dec 2012 06:56:54 -0700 Received: from d03relay01.boulder.ibm.com (d03relay01.boulder.ibm.com [9.17.195.226]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 38C313E40042 for ; Fri, 21 Dec 2012 06:56:46 -0700 (MST) Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay01.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qBLDubgi206572 for ; Fri, 21 Dec 2012 06:56:37 -0700 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qBLDuZlM010565 for ; Fri, 21 Dec 2012 06:56:36 -0700 From: "Jason J. Herne" Date: Fri, 21 Dec 2012 08:56:31 -0500 Message-Id: <1356098191-4998-1-git-send-email-jjherne@us.ibm.com> Subject: [Qemu-devel] [PATCH 7/7] KVM regsync: Fix do_kvm_cpu_synchronize_state data integrity issue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: borntraeger@de.ibm.com, qemu-devel@nongnu.org, jan.kiszka@siemens.com, agraf@suse.de, jfrei@linux.vnet.ibm.com, graalfs@linux.vnet.ibm.com Cc: "Jason J. Herne" From: "Jason J. Herne" 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 Reviewed-by: Christian Borntraeger --- 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