On Tue, Sep 16, 2014 at 06:03:40PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 17:55, Marcelo Tosatti ha scritto: > > On Tue, Sep 16, 2014 at 05:42:16PM +0200, Paolo Bonzini wrote: > >> Il 16/09/2014 17:14, Marcelo Tosatti ha scritto: > >>> + /* > >>> + * Make sure that CPU state is synchronized from KVM > >>> + * once every VM state change callback has finished. > >> > >> Which other callback could affect the in-kernel state, > > > > Marcin mentioned that APIC state was the culprit. > > > > Perhaps > > > > bdrv_drain_all(); > > ret = bdrv_flush_all(); > > > > Can change the interrupt state ? > > Ah, I thought Marcin was checking on the destination, not the source. > > > Then that should read "once VM stop has finished". > > But I still do not understand. > > The cpu_synchronize_all_states() call in kvmclock_vm_state_change() is > needed to make env->tsc up to date with the value on the source, right?
Its there to make sure the pair env->tsc, s->clock = data.clock are relative to point close in time. > But if the synchronize_all_states+clean_all_dirty pair runs on the > source, why is the cpu_synchronize_all_states() call in > qemu_savevm_state_complete() not enough? It runs even later than > kvmclock_vm_state_change. Because of the "pair of time values" explanation above. > I don't understand even the original patch without cpu_clean_all_dirty()... > > Paolo
