Patchwork [2/4] KVM: Rework VCPU state writeback API

login
register
mail settings
Submitter Jan Kiszka
Date March 1, 2010, 6:10 p.m.
Message ID <2257cac7474705fc35d74fcdff0566fb7dc0770e.1267467030.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/46582/
State New
Headers show

Comments

Jan Kiszka - March 1, 2010, 6:10 p.m.
This grand cleanup drops all reset and vmsave/load related
synchronization points in favor of four(!) generic hooks:

- cpu_synchronize_all_states in qemu_savevm_state_complete
  (initial sync from kernel before vmsave)
- cpu_synchronize_all_post_init in qemu_loadvm_state
  (writeback after vmload)
- cpu_synchronize_all_post_init in main after machine init
- cpu_synchronize_all_post_reset in qemu_system_reset
  (writeback after system reset)

These writeback points + the existing one of VCPU exec after
cpu_synchronize_state map on three levels of writeback:

- KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run)
- KVM_PUT_RESET_STATE   (on synchronous system reset, all VCPUs stopped)
- KVM_PUT_FULL_STATE    (on init or vmload, all VCPUs stopped as well)

This level is passed to the arch-specific VCPU state writing function
that will decide which concrete substates need to be written. That way,
no writer of load, save or reset functions that interact with in-kernel
KVM states will ever have to worry about synchronization again. That
also means that a lot of reasons for races, segfaults and deadlocks are
eliminated.

cpu_synchronize_state remains untouched, just as Anthony suggested. We
continue to need it before reading or writing of VCPU states that are
also tracked by in-kernel KVM subsystems.

Consequently, this patch removes many cpu_synchronize_state calls that
are now redundant, just like remaining explicit register syncs.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 exec.c                |   17 -----------------
 hw/apic.c             |    2 --
 hw/ppc_newworld.c     |    3 ---
 hw/ppc_oldworld.c     |    3 ---
 hw/s390-virtio.c      |    1 -
 kvm-all.c             |   19 +++++++++++++------
 kvm.h                 |   25 ++++++++++++++++++++++++-
 savevm.c              |    4 ++++
 sysemu.h              |    4 ++++
 target-i386/kvm.c     |    2 +-
 target-i386/machine.c |   11 -----------
 target-ppc/kvm.c      |    2 +-
 target-ppc/machine.c  |    4 ----
 target-s390x/kvm.c    |    3 +--
 vl.c                  |   29 +++++++++++++++++++++++++++++
 15 files changed, 77 insertions(+), 52 deletions(-)
Marcelo Tosatti - March 2, 2010, 12:14 a.m.
On Mon, Mar 01, 2010 at 07:10:30PM +0100, Jan Kiszka wrote:
> This grand cleanup drops all reset and vmsave/load related
> synchronization points in favor of four(!) generic hooks:
> 
> - cpu_synchronize_all_states in qemu_savevm_state_complete
>   (initial sync from kernel before vmsave)
> - cpu_synchronize_all_post_init in qemu_loadvm_state
>   (writeback after vmload)
> - cpu_synchronize_all_post_init in main after machine init
> - cpu_synchronize_all_post_reset in qemu_system_reset
>   (writeback after system reset)
> 
> These writeback points + the existing one of VCPU exec after
> cpu_synchronize_state map on three levels of writeback:
> 
> - KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run)
> - KVM_PUT_RESET_STATE   (on synchronous system reset, all VCPUs stopped)
> - KVM_PUT_FULL_STATE    (on init or vmload, all VCPUs stopped as well)
> 
> This level is passed to the arch-specific VCPU state writing function
> that will decide which concrete substates need to be written. That way,
> no writer of load, save or reset functions that interact with in-kernel
> KVM states will ever have to worry about synchronization again. That
> also means that a lot of reasons for races, segfaults and deadlocks are
> eliminated.
> 
> cpu_synchronize_state remains untouched, just as Anthony suggested. We
> continue to need it before reading or writing of VCPU states that are
> also tracked by in-kernel KVM subsystems.
> 
> Consequently, this patch removes many cpu_synchronize_state calls that
> are now redundant, just like remaining explicit register syncs.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Jan,

This patch breaks system reset of WinXP.32 install (more easily
reproducible without iothread enabled).

Screenshot attached.
Jan Kiszka - March 2, 2010, 8 a.m.
Marcelo Tosatti wrote:
> On Mon, Mar 01, 2010 at 07:10:30PM +0100, Jan Kiszka wrote:
>> This grand cleanup drops all reset and vmsave/load related
>> synchronization points in favor of four(!) generic hooks:
>>
>> - cpu_synchronize_all_states in qemu_savevm_state_complete
>>   (initial sync from kernel before vmsave)
>> - cpu_synchronize_all_post_init in qemu_loadvm_state
>>   (writeback after vmload)
>> - cpu_synchronize_all_post_init in main after machine init
>> - cpu_synchronize_all_post_reset in qemu_system_reset
>>   (writeback after system reset)
>>
>> These writeback points + the existing one of VCPU exec after
>> cpu_synchronize_state map on three levels of writeback:
>>
>> - KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run)
>> - KVM_PUT_RESET_STATE   (on synchronous system reset, all VCPUs stopped)
>> - KVM_PUT_FULL_STATE    (on init or vmload, all VCPUs stopped as well)
>>
>> This level is passed to the arch-specific VCPU state writing function
>> that will decide which concrete substates need to be written. That way,
>> no writer of load, save or reset functions that interact with in-kernel
>> KVM states will ever have to worry about synchronization again. That
>> also means that a lot of reasons for races, segfaults and deadlocks are
>> eliminated.
>>
>> cpu_synchronize_state remains untouched, just as Anthony suggested. We
>> continue to need it before reading or writing of VCPU states that are
>> also tracked by in-kernel KVM subsystems.
>>
>> Consequently, this patch removes many cpu_synchronize_state calls that
>> are now redundant, just like remaining explicit register syncs.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Jan,
> 
> This patch breaks system reset of WinXP.32 install (more easily
> reproducible without iothread enabled).
> 
> Screenshot attached.
> 

Strange - no issues with qemu-kvm? Any special command line switch? /me
goes scrounging for some installation XP32 CD in the meantime...

Jan
Marcelo Tosatti - March 2, 2010, 11:55 a.m.
On Tue, Mar 02, 2010 at 09:00:04AM +0100, Jan Kiszka wrote:
> Marcelo Tosatti wrote:
> > On Mon, Mar 01, 2010 at 07:10:30PM +0100, Jan Kiszka wrote:
> >> This grand cleanup drops all reset and vmsave/load related
> >> synchronization points in favor of four(!) generic hooks:
> >>
> >> - cpu_synchronize_all_states in qemu_savevm_state_complete
> >>   (initial sync from kernel before vmsave)
> >> - cpu_synchronize_all_post_init in qemu_loadvm_state
> >>   (writeback after vmload)
> >> - cpu_synchronize_all_post_init in main after machine init
> >> - cpu_synchronize_all_post_reset in qemu_system_reset
> >>   (writeback after system reset)
> >>
> >> These writeback points + the existing one of VCPU exec after
> >> cpu_synchronize_state map on three levels of writeback:
> >>
> >> - KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run)
> >> - KVM_PUT_RESET_STATE   (on synchronous system reset, all VCPUs stopped)
> >> - KVM_PUT_FULL_STATE    (on init or vmload, all VCPUs stopped as well)
> >>
> >> This level is passed to the arch-specific VCPU state writing function
> >> that will decide which concrete substates need to be written. That way,
> >> no writer of load, save or reset functions that interact with in-kernel
> >> KVM states will ever have to worry about synchronization again. That
> >> also means that a lot of reasons for races, segfaults and deadlocks are
> >> eliminated.
> >>
> >> cpu_synchronize_state remains untouched, just as Anthony suggested. We
> >> continue to need it before reading or writing of VCPU states that are
> >> also tracked by in-kernel KVM subsystems.
> >>
> >> Consequently, this patch removes many cpu_synchronize_state calls that
> >> are now redundant, just like remaining explicit register syncs.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Jan,
> > 
> > This patch breaks system reset of WinXP.32 install (more easily
> > reproducible without iothread enabled).
> > 
> > Screenshot attached.
> > 
> 
> Strange - no issues with qemu-kvm? Any special command line switch? /me
> goes scrounging for some installation XP32 CD in the meantime...

No issues with qemu-kvm. Could not spot anything obvious.

> 
> Jan
>
Jan Kiszka - March 2, 2010, 4:31 p.m.
Marcelo Tosatti wrote:
> On Tue, Mar 02, 2010 at 09:00:04AM +0100, Jan Kiszka wrote:
>> Marcelo Tosatti wrote:
>>> On Mon, Mar 01, 2010 at 07:10:30PM +0100, Jan Kiszka wrote:
>>>> This grand cleanup drops all reset and vmsave/load related
>>>> synchronization points in favor of four(!) generic hooks:
>>>>
>>>> - cpu_synchronize_all_states in qemu_savevm_state_complete
>>>>   (initial sync from kernel before vmsave)
>>>> - cpu_synchronize_all_post_init in qemu_loadvm_state
>>>>   (writeback after vmload)
>>>> - cpu_synchronize_all_post_init in main after machine init
>>>> - cpu_synchronize_all_post_reset in qemu_system_reset
>>>>   (writeback after system reset)
>>>>
>>>> These writeback points + the existing one of VCPU exec after
>>>> cpu_synchronize_state map on three levels of writeback:
>>>>
>>>> - KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run)
>>>> - KVM_PUT_RESET_STATE   (on synchronous system reset, all VCPUs stopped)
>>>> - KVM_PUT_FULL_STATE    (on init or vmload, all VCPUs stopped as well)
>>>>
>>>> This level is passed to the arch-specific VCPU state writing function
>>>> that will decide which concrete substates need to be written. That way,
>>>> no writer of load, save or reset functions that interact with in-kernel
>>>> KVM states will ever have to worry about synchronization again. That
>>>> also means that a lot of reasons for races, segfaults and deadlocks are
>>>> eliminated.
>>>>
>>>> cpu_synchronize_state remains untouched, just as Anthony suggested. We
>>>> continue to need it before reading or writing of VCPU states that are
>>>> also tracked by in-kernel KVM subsystems.
>>>>
>>>> Consequently, this patch removes many cpu_synchronize_state calls that
>>>> are now redundant, just like remaining explicit register syncs.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> Jan,
>>>
>>> This patch breaks system reset of WinXP.32 install (more easily
>>> reproducible without iothread enabled).
>>>
>>> Screenshot attached.
>>>
>> Strange - no issues with qemu-kvm? Any special command line switch? /me
>> goes scrounging for some installation XP32 CD in the meantime...
> 
> No issues with qemu-kvm. Could not spot anything obvious.
> 

And, of course, my WinXP installation did not trigger any reset issue,
even in non-iothreaded mode. :(

Jan
Avi Kivity - March 11, 2010, 8:32 a.m.
On 03/02/2010 02:14 AM, Marcelo Tosatti wrote:
> On Mon, Mar 01, 2010 at 07:10:30PM +0100, Jan Kiszka wrote:
>    
>> This grand cleanup drops all reset and vmsave/load related
>> synchronization points in favor of four(!) generic hooks:
>>
>> - cpu_synchronize_all_states in qemu_savevm_state_complete
>>    (initial sync from kernel before vmsave)
>> - cpu_synchronize_all_post_init in qemu_loadvm_state
>>    (writeback after vmload)
>> - cpu_synchronize_all_post_init in main after machine init
>> - cpu_synchronize_all_post_reset in qemu_system_reset
>>    (writeback after system reset)
>>
>> These writeback points + the existing one of VCPU exec after
>> cpu_synchronize_state map on three levels of writeback:
>>
>> - KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run)
>> - KVM_PUT_RESET_STATE   (on synchronous system reset, all VCPUs stopped)
>> - KVM_PUT_FULL_STATE    (on init or vmload, all VCPUs stopped as well)
>>
>> This level is passed to the arch-specific VCPU state writing function
>> that will decide which concrete substates need to be written. That way,
>> no writer of load, save or reset functions that interact with in-kernel
>> KVM states will ever have to worry about synchronization again. That
>> also means that a lot of reasons for races, segfaults and deadlocks are
>> eliminated.
>>
>> cpu_synchronize_state remains untouched, just as Anthony suggested. We
>> continue to need it before reading or writing of VCPU states that are
>> also tracked by in-kernel KVM subsystems.
>>
>> Consequently, this patch removes many cpu_synchronize_state calls that
>> are now redundant, just like remaining explicit register syncs.
>>
>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
>>      
> Jan,
>
> This patch breaks system reset of WinXP.32 install (more easily
> reproducible without iothread enabled).
>
>    

What's the conclusion here?  The patch is innocent of the regression?
Marcelo Tosatti - March 11, 2010, 6:49 p.m.
On Thu, Mar 11, 2010 at 10:32:50AM +0200, Avi Kivity wrote:
> On 03/02/2010 02:14 AM, Marcelo Tosatti wrote:
> >On Mon, Mar 01, 2010 at 07:10:30PM +0100, Jan Kiszka wrote:
> >>This grand cleanup drops all reset and vmsave/load related
> >>synchronization points in favor of four(!) generic hooks:
> >>
> >>- cpu_synchronize_all_states in qemu_savevm_state_complete
> >>   (initial sync from kernel before vmsave)
> >>- cpu_synchronize_all_post_init in qemu_loadvm_state
> >>   (writeback after vmload)
> >>- cpu_synchronize_all_post_init in main after machine init
> >>- cpu_synchronize_all_post_reset in qemu_system_reset
> >>   (writeback after system reset)
> >>
> >>These writeback points + the existing one of VCPU exec after
> >>cpu_synchronize_state map on three levels of writeback:
> >>
> >>- KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run)
> >>- KVM_PUT_RESET_STATE   (on synchronous system reset, all VCPUs stopped)
> >>- KVM_PUT_FULL_STATE    (on init or vmload, all VCPUs stopped as well)
> >>
> >>This level is passed to the arch-specific VCPU state writing function
> >>that will decide which concrete substates need to be written. That way,
> >>no writer of load, save or reset functions that interact with in-kernel
> >>KVM states will ever have to worry about synchronization again. That
> >>also means that a lot of reasons for races, segfaults and deadlocks are
> >>eliminated.
> >>
> >>cpu_synchronize_state remains untouched, just as Anthony suggested. We
> >>continue to need it before reading or writing of VCPU states that are
> >>also tracked by in-kernel KVM subsystems.
> >>
> >>Consequently, this patch removes many cpu_synchronize_state calls that
> >>are now redundant, just like remaining explicit register syncs.
> >>
> >>Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> >Jan,
> >
> >This patch breaks system reset of WinXP.32 install (more easily
> >reproducible without iothread enabled).
> >
> 
> What's the conclusion here?  The patch is innocent of the regression?

Yes, it is. The problem was caused by a recent seabios change, now
fixed.

Patch

diff --git a/exec.c b/exec.c
index 8616ff9..50a2e46 100644
--- a/exec.c
+++ b/exec.c
@@ -518,21 +518,6 @@  void cpu_exec_init_all(unsigned long tb_size)
 
 #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
 
-static void cpu_common_pre_save(void *opaque)
-{
-    CPUState *env = opaque;
-
-    cpu_synchronize_state(env);
-}
-
-static int cpu_common_pre_load(void *opaque)
-{
-    CPUState *env = opaque;
-
-    cpu_synchronize_state(env);
-    return 0;
-}
-
 static int cpu_common_post_load(void *opaque, int version_id)
 {
     CPUState *env = opaque;
@@ -550,8 +535,6 @@  static const VMStateDescription vmstate_cpu_common = {
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
-    .pre_save = cpu_common_pre_save,
-    .pre_load = cpu_common_pre_load,
     .post_load = cpu_common_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_UINT32(halted, CPUState),
diff --git a/hw/apic.c b/hw/apic.c
index 87e7dc0..3c90f4c 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -938,8 +938,6 @@  static void apic_reset(void *opaque)
     APICState *s = opaque;
     int bsp;
 
-    cpu_synchronize_state(s->cpu_env);
-
     bsp = cpu_is_bsp(s->cpu_env);
     s->apicbase = 0xfee00000 |
         (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index bc86c85..d4f9013 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -167,9 +167,6 @@  static void ppc_core99_init (ram_addr_t ram_size,
         envs[i] = env;
     }
 
-    /* Make sure all register sets take effect */
-    cpu_synchronize_state(env);
-
     /* allocate RAM */
     ram_offset = qemu_ram_alloc(ram_size);
     cpu_register_physical_memory(0, ram_size, ram_offset);
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 04a7835..93c95ba 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -165,9 +165,6 @@  static void ppc_heathrow_init (ram_addr_t ram_size,
         envs[i] = env;
     }
 
-    /* Make sure all register sets take effect */
-    cpu_synchronize_state(env);
-
     /* allocate RAM */
     if (ram_size > (2047 << 20)) {
         fprintf(stderr,
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 3582728..ad3386f 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -185,7 +185,6 @@  static void s390_init(ram_addr_t ram_size,
             exit(1);
         }
 
-        cpu_synchronize_state(env);
         env->psw.addr = KERN_IMAGE_START;
         env->psw.mask = 0x0000000180000000ULL;
     }
diff --git a/kvm-all.c b/kvm-all.c
index 2f7e33a..534ead0 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -156,10 +156,6 @@  static void kvm_reset_vcpu(void *opaque)
     CPUState *env = opaque;
 
     kvm_arch_reset_vcpu(env);
-    if (kvm_arch_put_registers(env)) {
-        fprintf(stderr, "Fatal: kvm vcpu reset failed\n");
-        abort();
-    }
 }
 
 int kvm_irqchip_in_kernel(void)
@@ -214,7 +210,6 @@  int kvm_init_vcpu(CPUState *env)
     if (ret == 0) {
         qemu_register_reset(kvm_reset_vcpu, env);
         kvm_arch_reset_vcpu(env);
-        ret = kvm_arch_put_registers(env);
     }
 err:
     return ret;
@@ -753,6 +748,18 @@  void kvm_cpu_synchronize_state(CPUState *env)
     }
 }
 
+void kvm_cpu_synchronize_post_reset(CPUState *env)
+{
+    kvm_arch_put_registers(env, KVM_PUT_RESET_STATE);
+    env->kvm_vcpu_dirty = 0;
+}
+
+void kvm_cpu_synchronize_post_init(CPUState *env)
+{
+    kvm_arch_put_registers(env, KVM_PUT_FULL_STATE);
+    env->kvm_vcpu_dirty = 0;
+}
+
 int kvm_cpu_exec(CPUState *env)
 {
     struct kvm_run *run = env->kvm_run;
@@ -770,7 +777,7 @@  int kvm_cpu_exec(CPUState *env)
 #endif
 
         if (env->kvm_vcpu_dirty) {
-            kvm_arch_put_registers(env);
+            kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
             env->kvm_vcpu_dirty = 0;
         }
 
diff --git a/kvm.h b/kvm.h
index a602e45..b2937b9 100644
--- a/kvm.h
+++ b/kvm.h
@@ -82,7 +82,14 @@  int kvm_arch_pre_run(CPUState *env, struct kvm_run *run);
 
 int kvm_arch_get_registers(CPUState *env);
 
-int kvm_arch_put_registers(CPUState *env);
+/* state subset only touched by the VCPU itself during runtime */
+#define KVM_PUT_RUNTIME_STATE   1
+/* state subset modified during VCPU reset */
+#define KVM_PUT_RESET_STATE     2
+/* full state set, modified during initialization or on vmload */
+#define KVM_PUT_FULL_STATE      3
+
+int kvm_arch_put_registers(CPUState *env, int level);
 
 int kvm_arch_init(KVMState *s, int smp_cpus);
 
@@ -126,6 +133,8 @@  int kvm_check_extension(KVMState *s, unsigned int extension);
 uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
                                       int reg);
 void kvm_cpu_synchronize_state(CPUState *env);
+void kvm_cpu_synchronize_post_reset(CPUState *env);
+void kvm_cpu_synchronize_post_init(CPUState *env);
 
 /* generic hooks - to be moved/refactored once there are more users */
 
@@ -136,4 +145,18 @@  static inline void cpu_synchronize_state(CPUState *env)
     }
 }
 
+static inline void cpu_synchronize_post_reset(CPUState *env)
+{
+    if (kvm_enabled()) {
+        kvm_cpu_synchronize_post_reset(env);
+    }
+}
+
+static inline void cpu_synchronize_post_init(CPUState *env)
+{
+    if (kvm_enabled()) {
+        kvm_cpu_synchronize_post_init(env);
+    }
+}
+
 #endif
diff --git a/savevm.c b/savevm.c
index 4b58663..a6e774b 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1345,6 +1345,8 @@  int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
 {
     SaveStateEntry *se;
 
+    cpu_synchronize_all_states();
+
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         if (se->save_live_state == NULL)
             continue;
@@ -1545,6 +1547,8 @@  int qemu_loadvm_state(QEMUFile *f)
         }
     }
 
+    cpu_synchronize_all_post_init();
+
     ret = 0;
 
 out:
diff --git a/sysemu.h b/sysemu.h
index 8ba618e..d77344c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -58,6 +58,10 @@  int load_vmstate(Monitor *mon, const char *name);
 void do_delvm(Monitor *mon, const QDict *qdict);
 void do_info_snapshots(Monitor *mon);
 
+void cpu_synchronize_all_states(void);
+void cpu_synchronize_all_post_reset(void);
+void cpu_synchronize_all_post_init(void);
+
 void qemu_announce_self(void);
 
 void main_loop_wait(int timeout);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2365ca3..a4767b2 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -882,7 +882,7 @@  static int kvm_guest_debug_workarounds(CPUState *env)
     return ret;
 }
 
-int kvm_arch_put_registers(CPUState *env)
+int kvm_arch_put_registers(CPUState *env, int level)
 {
     int ret;
 
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 8770491..b547e2a 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -321,8 +321,6 @@  static void cpu_pre_save(void *opaque)
     CPUState *env = opaque;
     int i;
 
-    cpu_synchronize_state(env);
-
     /* FPU */
     env->fpus_vmstate = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
     env->fptag_vmstate = 0;
@@ -337,14 +335,6 @@  static void cpu_pre_save(void *opaque)
 #endif
 }
 
-static int cpu_pre_load(void *opaque)
-{
-    CPUState *env = opaque;
-
-    cpu_synchronize_state(env);
-    return 0;
-}
-
 static int cpu_post_load(void *opaque, int version_id)
 {
     CPUState *env = opaque;
@@ -373,7 +363,6 @@  static const VMStateDescription vmstate_cpu = {
     .minimum_version_id = 3,
     .minimum_version_id_old = 3,
     .pre_save = cpu_pre_save,
-    .pre_load = cpu_pre_load,
     .post_load = cpu_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_UINTTL_ARRAY(regs, CPUState, CPU_NB_REGS),
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 8ad0037..aa3d432 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -73,7 +73,7 @@  void kvm_arch_reset_vcpu(CPUState *env)
 {
 }
 
-int kvm_arch_put_registers(CPUState *env)
+int kvm_arch_put_registers(CPUState *env, int level)
 {
     struct kvm_regs regs;
     int ret;
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 4897c8a..67de951 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -7,8 +7,6 @@  void cpu_save(QEMUFile *f, void *opaque)
     CPUState *env = (CPUState *)opaque;
     unsigned int i, j;
 
-    cpu_synchronize_state(env);
-
     for (i = 0; i < 32; i++)
         qemu_put_betls(f, &env->gpr[i]);
 #if !defined(TARGET_PPC64)
@@ -96,8 +94,6 @@  int cpu_load(QEMUFile *f, void *opaque, int version_id)
     CPUState *env = (CPUState *)opaque;
     unsigned int i, j;
 
-    cpu_synchronize_state(env);
-
     for (i = 0; i < 32; i++)
         qemu_get_betls(f, &env->gpr[i]);
 #if !defined(TARGET_PPC64)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 0199a65..72e77b0 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -91,7 +91,7 @@  void kvm_arch_reset_vcpu(CPUState *env)
     /* FIXME: add code to reset vcpu. */
 }
 
-int kvm_arch_put_registers(CPUState *env)
+int kvm_arch_put_registers(CPUState *env, int level)
 {
     struct kvm_regs regs;
     int ret;
@@ -296,7 +296,6 @@  static int handle_hypercall(CPUState *env, struct kvm_run *run)
 
     cpu_synchronize_state(env);
     r = s390_virtio_hypercall(env);
-    kvm_arch_put_registers(env);
 
     return r;
 }
diff --git a/vl.c b/vl.c
index 66e477a..67143a7 100644
--- a/vl.c
+++ b/vl.c
@@ -3002,6 +3002,33 @@  static void nographic_update(void *opaque)
     qemu_mod_timer(nographic_timer, interval + qemu_get_clock(rt_clock));
 }
 
+void cpu_synchronize_all_states(void)
+{
+    CPUState *cpu;
+
+    for (cpu = first_cpu; cpu; cpu = cpu->next_cpu) {
+        cpu_synchronize_state(cpu);
+    }
+}
+
+void cpu_synchronize_all_post_reset(void)
+{
+    CPUState *cpu;
+
+    for (cpu = first_cpu; cpu; cpu = cpu->next_cpu) {
+        cpu_synchronize_post_reset(cpu);
+    }
+}
+
+void cpu_synchronize_all_post_init(void)
+{
+    CPUState *cpu;
+
+    for (cpu = first_cpu; cpu; cpu = cpu->next_cpu) {
+        cpu_synchronize_post_init(cpu);
+    }
+}
+
 struct vm_change_state_entry {
     VMChangeStateHandler *cb;
     void *opaque;
@@ -3143,6 +3170,7 @@  void qemu_system_reset(void)
     QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
         re->func(re->opaque);
     }
+    cpu_synchronize_all_post_reset();
 }
 
 void qemu_system_reset_request(void)
@@ -5927,6 +5955,7 @@  int main(int argc, char **argv, char **envp)
     machine->init(ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
+    cpu_synchronize_all_post_init();
 
 #ifndef _WIN32
     /* must be after terminal init, SDL library changes signal handlers */