Re: [Qemu-devel] [PATCH] kvmclock: clarify usage of cpu_clean_all_dirty

[Paolo Bonzini]

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?

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.

I don't understand even the original patch without cpu_clean_all_dirty()...

Paolo