On Wed, May 03, 2017 at 03:55:36PM +0200, Paolo Bonzini wrote: > > > On 03/05/2017 15:40, Marcelo Tosatti wrote: > >>> In the masterclock enabled case, kvmclock_offset must be adjusted so > >>> that user_ns.clock = master_kernel_ns + kvmclock_offset (that is, the > >>> value set from KVM_SET_CLOCK is the one visible at system_timestamp). > >>> > >>> This way the guest clock: > >>> > >>> 1. Starts counting when KVM_SET_CLOCK executes. > >>> 2. With the value provided by userspace. > >> > >> So this fixes rounding errors? > > > > No. It just does the correct > > > > "user_ns.clock = master_kernel_ns + kvmclock_offset = return value of > > get_kvmclock_ns(kvm) at moment of KVM_SET_CLOCK" > > > > (that is, when guest_rdtsc() == tsc_timestamp, so guest_rdtsc() - tsc_timestamp = 0, > > you want get_kvmclock_ns() to return user_ns.clock). > > Yep, and it currently doesn't match because the current code introduces > rounding errors? No. Consider the current code. When get_kvmclock_ns() is called, ka->masterclock = false. kvm_gen_update_masterclock() sets it to true (because at the time KVM_SET_CLOCK is called, the conditions necessary to do so are fulfilled). > >>> + kvm_for_each_vcpu(i, vcpu, kvm) > >>> + kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu); > >> kvm_gen_update_masterclock already does that, why did you move that to > >> before the assignment? > > Its after the assignment of kvmclock_offset because > > KVM_REQ_CLOCK_UPDATES processing by vcpus make use of kvmclock_offset. > > Sure, but why did you move kvm_gen_update_masterclock before? Explained above. > If you left kvm_gen_update_masterclock after the update of > kvm->arch.kvmclock_offset, the duplicate KVM_REQ_CLOCK_UPDATE would not > be necessary. No can do. Feel free to reorganize/rewrite the patch if you feel like it, as long as the same effects are achieved. Thanks. > > Paolo > > > So if you change that value, you have to request the vcpus to > > recalculate their kvmclock areas using the new kvmclock_offset.
