linux-kernel - Re: [PATCH 2/2] x86/vdso: Add VCLOCK

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Feb 2017 12:06:47 +0100
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     x86@...nel.org, Andy Lutomirski <luto@...capital.net>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Dexuan Cui <decui@...rosoft.com>, linux-kernel@...r.kernel.org,
        devel@...uxdriverproject.org,
        virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method

Thomas Gleixner <tglx@...utronix.de> 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