Re: [PATCH] KVM: x86: fix kvmclock breakage from timers branch merge

Thomas Gleixner
2014-09-04T21:27:49Z

On Thu, 4 Sep 2014, Paolo Bonzini wrote:

> Commit cbcf2dd3b3d4 (x86: kvm: Make kvm_get_time_and_clockread() nanoseconds
> based, 2014-07-16) used the wrong formula for boot_ns, thus breaking kvmclock on
> hosts that have a reliable TSC.
>
> To find the right formula, let's first backport the switch to nanoseconds
> to 3.16-era timekeeping logic. The full patch (which works) is at
> https://lkml.org/lkml/2014/9/4/462. The key line here is
>
> boot_ns = timespec_to_ns(&tk->total_sleep_time)
> + timespec_to_ns(&tk->wall_to_monotonic)
> + tk->xtime_sec * (u64)NSEC_PER_SEC;
>
> Because the above patch works, the conclusion is that the above formula
> is not the same as commit cbcf2dd3b3d4's
>
> boot_ns = ktime_to_ns(ktime_add(tk->tkr.base_mono, tk->offs_boot));
>
> As to what is the right one, commit 02cba1598a2a (timekeeping: Simplify getboottime(),
> 2014-07-16) provides a hint:
>
> offs_real = -wall-to_monotonic
> offs_boot = total_sleep_time
>
> offs_real - offs_boot = -wall_to_monotonic - total_sleep_time
>
> that is
>
> offs_boot - offs_real = wall_to_monotonic + total_sleep_time
>
> which is what this patch uses, adding xtime_sec separately. The "boot_ns"
> moniker is not too clear, so rename boot_ns to nsec_base and the existing
> nsec_base to snsec_base.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Reported-by: Chris J Arges <chris.j.arges@canonical.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> Thomas/John, the problem with the above explanation is that
> tk_update_ktime_data has "base_mono = xtime_sec + wtm", and from
> there "base_mono + offs_boot = xtime_sec + wtm + total_sleep_time".
> Except that doesn't work, so something must be wrong in
> tk_update_ktime_data's comment.

Right. I'm staring into it and we need to fix the core code issue and
not the usage site.

Thanks,

tglx