On Wed, May 03, 2017 at 03:08:46PM +0200, Paolo Bonzini wrote:
>
>
> On 02/05/2017 23:36, 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).
Its not a rounding error.
> > - now_ns = get_kvmclock_ns(kvm);
> > - kvm->arch.kvmclock_offset += user_ns.clock - now_ns;
> > +
> > kvm_gen_update_masterclock(kvm);
> > + if (ka->use_master_clock) {
> > + /*
> > + * 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).
> > + */
> > + kvm->arch.kvmclock_offset = user_ns.clock -
> > + ka->master_kernel_ns;
> > +
>
>
> This needs to hold ka->pvclock_gtod_sync_lock, I think.
>
> > + 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.
So if you change that value, you have to request the vcpus to
recalculate their kvmclock areas using the new kvmclock_offset.
Resending v2, thanks.
