From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752133AbdBJLIG (ORCPT ); Fri, 10 Feb 2017 06:08:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57516 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751477AbdBJLIF (ORCPT ); Fri, 10 Feb 2017 06:08:05 -0500 From: Vitaly Kuznetsov To: Thomas Gleixner Cc: x86@kernel.org, Andy Lutomirski , Ingo Molnar , "H. Peter Anvin" , "K. Y. Srinivasan" , Haiyang Zhang , Stephen Hemminger , Dexuan Cui , linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, virtualization@lists.linux-foundation.org Subject: Re: [PATCH 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method References: <20170209141052.18694-1-vkuznets@redhat.com> <20170209141052.18694-3-vkuznets@redhat.com> Date: Fri, 10 Feb 2017 12:06:47 +0100 In-Reply-To: (Thomas Gleixner's message of "Thu, 9 Feb 2017 18:08:22 +0100 (CET)") Message-ID: <87lgteqp88.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Fri, 10 Feb 2017 11:06:51 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Thomas Gleixner writes: > On Thu, 9 Feb 2017, Vitaly Kuznetsov wrote: >> +#ifdef CONFIG_HYPERV_TSCPAGE >> +static notrace u64 vread_hvclock(int *mode) >> +{ >> + const struct ms_hyperv_tsc_page *tsc_pg = >> + (const struct ms_hyperv_tsc_page *)&hvclock_page; >> + u64 sequence, scale, offset, current_tick, cur_tsc; >> + >> + while (1) { >> + sequence = READ_ONCE(tsc_pg->tsc_sequence); >> + if (!sequence) >> + break; >> + >> + scale = READ_ONCE(tsc_pg->tsc_scale); >> + offset = READ_ONCE(tsc_pg->tsc_offset); >> + rdtscll(cur_tsc); >> + >> + current_tick = mul_u64_u64_shr(cur_tsc, scale, 64) + offset; >> + >> + if (READ_ONCE(tsc_pg->tsc_sequence) == sequence) >> + return current_tick; > > That sequence stuff lacks still a sensible explanation. It's fundamentally > different from the sequence counting we do in the kernel, so documentation > for it is really required. Sure, do you think the following would do? diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c index 4af10b4..886b600 100644 --- a/arch/x86/entry/vdso/vclock_gettime.c +++ b/arch/x86/entry/vdso/vclock_gettime.c @@ -154,6 +154,22 @@ static notrace u64 vread_hvclock(int *mode) (const struct ms_hyperv_tsc_page *)&hvclock_page; u64 sequence, scale, offset, current_tick, cur_tsc; + /* + * The protocol for reading Hyper-V TSC page is specified in Hypervisor + * Top-Level Functional Specification ver. 3.0 and above. To get the + * reference time we must do the following: + * - READ ReferenceTscSequence + * A special '0' value indicates the time source is unreliable and we + * need to use something else. The currently published specification + * versions (up to 4.0b) contain a mistake and wrongly claim '-1' + * instead of '0' as the special value, see commit c35b82ef0294. + * - ReferenceTime = + * ((RDTSC() * ReferenceTscScale) >> 64) + ReferenceTscOffset + * - READ ReferenceTscSequence again. In case its value has changed + * since our first reading we need to discard ReferenceTime and repeat + * the whole sequence as the hypervisor was updating the page in + * between. + */ while (1) { sequence = READ_ONCE(tsc_pg->tsc_sequence); if (!sequence) -- Vitaly