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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1409042317560.3333@nanos>
Date:	Thu, 4 Sep 2014 23:27:49 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Paolo Bonzini <pbonzini@...hat.com>
cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	chris.j.arges@...onical.com, John Stultz <john.stultz@...aro.org>
Subject: Re: [PATCH] KVM: x86: fix kvmclock breakage from timers branch
 merge

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@...utronix.de>
> Cc: John Stultz <john.stultz@...aro.org>
> Reported-by: Chris J Arges <chris.j.arges@...onical.com>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/