On Tue, Sep 16, 2014 at 06:22:15PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 18:07, Marcelo Tosatti ha scritto: > >> > 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. > > Ok. But why are they not close in time?
Scenario 1 A. s->clock = get_clock() B. env->tsc = rdtsc() Scenario 2 A. s->clock = get_clock() C. VM callbacks, bdrv_flush, ... B. env->tsc = rdtsc() They are not "close in time" because of C. > Could we have the opposite situation where env->tsc is loaded a long > time _after_ s->clock, and something breaks? This particular read avoids an overflow. See + assert(time.tsc_timestamp <= migration_tsc); About your question, perhaps, would have to make up an env->tsc, s->clock pair which breaks the code or a guest application. > Also, there is no reason to do kvmclock_current_nsec() during normal > execution of the VM. Is the s->clock sent by the source ever good > across migration, and could the source send kvmclock_current_nsec() > instead of whatever KVM_GET_CLOCK returns? Yes it could. What difference does it make? > I don't understand this code very well, but it seems to me that the > migration handling and VM state change handler are mixed up... I don't see what the problem is. I am sure you can understand the code.
