Re: [Qemu-devel] [svt-core] [PATCH] kvmclock: update system_time_msr address forcibly

[Paolo Bonzini]

On 25/05/2017 11:40, Roman Kagan wrote:
>>>> +    kvmclock_struct_pa = env->system_time_msr & ~1ULL;
>>>> +
>>>>      if (!(env->system_time_msr & 1ULL)) {
>>>>          /* KVM clock not active */
>>>>          return 0;
>>> Roman.
>> Can't you avoid that call to each CPU? (ie fix the synchronization 
>> of the system time address problem in some other way?)
> Sorry, what call do you mean?  On one hand I suggested exactly to only
> call cpu_synchronize_state on the current (== first) cpu.  On the other,
> cpu_synchronize_state is heavier than just fetching a single msr.
> 
> Anyway kvmclock_current_nsec is only called in kvmclock_vm_state_change
> callback which is certainly not performance-critical, so IMO less new
> code here is better than more efficiency.
> 
> Or maybe I misunderstand your reason to request that the synchronization
> problem is fixed in some other way?

Denis's patch is problematic in that KVM_GET_MSRS should run in the VCPU
thread (using run_on_cpu).  cpu_synchronize_state() is heavier, but
solves this problem nicely.  Since it's not performance critical as you
say, calling cpu_synchronize_state() from kvmclock_current_nsec() seems
the best solution.

Paolo