[PATCH v4 0/2] hv_util: adjust system time smoothly

[PATCH v4 0/2] hv_util: adjust system time smoothly

[Vitaly Kuznetsov]

With TimeSync version 4 protocol support we started updating system time
continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
there is a time sample from the host which triggers do_settimeofday[64]().
While the time from the host is very accurate such adjustments may cause
issues:
- Time is jumping forward and backward, some applications may misbehave.
- In case an NTP server runs in parallel and uses something else for time
  sync (network, PTP,...) system time will never converge.
- Systemd starts annoying you by printing "Time has been changed" every 5
  seconds to the system log.

Instead of doing in-kernel time adjustments offload the work to an
NTP client by exposing TimeSync messages as a PTP device. Users may now
decide what they want to use as a source.

Changes since v3:
- Minor style changes [Stephen Hemminger]
- Added a comment explaining why we don't fail the driver registration
  when ptp_clock_register() fails [Thomas Gleixner]
- Add Thomas' A-b to PATCH1

Vitaly Kuznetsov (2):
  hv_util: switch to using timespec64
  hv_utils: implement Hyper-V PTP source

 drivers/hv/hv_util.c | 146 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 120 insertions(+), 26 deletions(-)

-- 
2.9.3

[PATCH v4 1/2] hv_util: switch to using timespec64

[Vitaly Kuznetsov]

do_settimeofday() is deprecated, use do_settimeofday64() instead.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Acked-by: John Stultz <john.stultz@linaro.org>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/hv/hv_util.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index e770774..94719eb 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -184,7 +184,7 @@ static void hv_set_host_time(struct work_struct *work)
 	struct adj_time_work	*wrk;
 	s64 host_tns;
 	u64 newtime;
-	struct timespec host_ts;
+	struct timespec64 host_ts;
 
 	wrk = container_of(work, struct adj_time_work, work);
 
@@ -203,9 +203,9 @@ static void hv_set_host_time(struct work_struct *work)
 		newtime += (current_tick - wrk->ref_time);
 	}
 	host_tns = (newtime - WLTIMEDELTA) * 100;
-	host_ts = ns_to_timespec(host_tns);
+	host_ts = ns_to_timespec64(host_tns);
 
-	do_settimeofday(&host_ts);
+	do_settimeofday64(&host_ts);
 }
 
 /*
-- 
2.9.3

[PATCH v4 2/2] hv_utils: implement Hyper-V PTP source

[Vitaly Kuznetsov]

With TimeSync version 4 protocol support we started updating system time
continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
there is a time sample from the host which triggers do_settimeofday[64]().
While the time from the host is very accurate such adjustments may cause
issues:
- Time is jumping forward and backward, some applications may misbehave.
- In case an NTP server runs in parallel and uses something else for time
  sync (network, PTP,...) system time will never converge.
- Systemd starts annoying you by printing "Time has been changed" every 5
  seconds to the system log.

Instead of doing in-kernel time adjustments offload the work to an
NTP client by exposing TimeSync messages as a PTP device. Users may now
decide what they want to use as a source.

I tested the solution with chrony, the config was:

 refclock PHC /dev/ptp0 poll 3 precision 1e-9

The result I'm seeing is accurate enough, the time delta between the guest
and the host is almost always within [-10us, +10us], the in-kernel solution
was giving us comparable results.

I also tried implementing PPS device instead of PTP by using not currently
used Hyper-V synthetic timers (we use only one of four for clockevent) but
with PPS source only chrony wasn't able to give me the required accuracy,
the delta often more that 100us.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/hv/hv_util.c | 144 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 119 insertions(+), 25 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 94719eb..e1211b8 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -27,6 +27,7 @@
 #include <linux/sysctl.h>
 #include <linux/reboot.h>
 #include <linux/hyperv.h>
+#include <linux/ptp_clock_kernel.h>
 
 #include "hyperv_vmbus.h"
 
@@ -179,31 +180,34 @@ struct adj_time_work {
 	u8	flags;
 };
 
+static u64 get_timeadj_latency(u64 ref_time)
+{
+	u64 current_tick;
+
+	if (ts_srv_version <= TS_VERSION_3)
+		return 0;
+
+	/*
+	 * Some latency has been introduced since Hyper-V generated
+	 * its time sample. Take that latency into account before
+	 * using TSC reference time sample from Hyper-V.
+	 *
+	 * This sample is given by TimeSync v4 and above hosts.
+	 */
+	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+	return current_tick - ref_time;
+}
+
 static void hv_set_host_time(struct work_struct *work)
 {
 	struct adj_time_work	*wrk;
-	s64 host_tns;
-	u64 newtime;
 	struct timespec64 host_ts;
+	u64 newtime;
 
 	wrk = container_of(work, struct adj_time_work, work);
 
-	newtime = wrk->host_time;
-	if (ts_srv_version > TS_VERSION_3) {
-		/*
-		 * Some latency has been introduced since Hyper-V generated
-		 * its time sample. Take that latency into account before
-		 * using TSC reference time sample from Hyper-V.
-		 *
-		 * This sample is given by TimeSync v4 and above hosts.
-		 */
-		u64 current_tick;
-
-		rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
-		newtime += (current_tick - wrk->ref_time);
-	}
-	host_tns = (newtime - WLTIMEDELTA) * 100;
-	host_ts = ns_to_timespec64(host_tns);
+	newtime = wrk->host_time + get_timeadj_latency(wrk->ref_time);
+	host_ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
 
 	do_settimeofday64(&host_ts);
 }
@@ -222,22 +226,52 @@ static void hv_set_host_time(struct work_struct *work)
  * to discipline the clock.
  */
 static struct adj_time_work  wrk;
-static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 flags)
+
+/*
+ * The last time sample, received from the host. PTP device responds to
+ * requests by using this data and the current partition-wide time reference
+ * count.
+ */
+static struct {
+	u64		host_time;
+	u64		ref_time;
+	spinlock_t	lock;
+} host_ts;
+
+static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
 {
+	unsigned long flags;
 
 	/*
 	 * This check is safe since we are executing in the
 	 * interrupt context and time synch messages arre always
 	 * delivered on the same CPU.
 	 */
-	if (work_pending(&wrk.work))
-		return;
+	if (adj_flags & ICTIMESYNCFLAG_SYNC) {
+		if (work_pending(&wrk.work))
+			return;
 
-	wrk.host_time = hosttime;
-	wrk.ref_time = reftime;
-	wrk.flags = flags;
-	if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) != 0) {
+		wrk.host_time = hosttime;
+		wrk.ref_time = reftime;
+		wrk.flags = adj_flags;
 		schedule_work(&wrk.work);
+	} else {
+		spin_lock_irqsave(&host_ts.lock, flags);
+		host_ts.host_time = hosttime;
+
+		/*
+		 * Prior to version 4 TimeSync messages from the host don't
+		 * contain any reference time (the time when the time sample
+		 * was generated), save the current time reference count
+		 * instead. This adds a small delta between the time sample
+		 * generation and the reception of the sample here to the result
+		 * but it's the best thing we can do.
+		 */
+		if (ts_srv_version <= TS_VERSION_3)
+			rdmsrl(HV_X64_MSR_TIME_REF_COUNT, host_ts.ref_time);
+		else
+			host_ts.ref_time = reftime;
+		spin_unlock_irqrestore(&host_ts.lock, flags);
 	}
 }
 
@@ -470,14 +504,74 @@ static  struct hv_driver util_drv = {
 	.remove =  util_remove,
 };
 
+static int hv_ptp_enable(struct ptp_clock_info *info,
+			 struct ptp_clock_request *request, int on)
+{
+	return -EOPNOTSUPP;
+}
+
+static int hv_ptp_settime(struct ptp_clock_info *p, const struct timespec64 *ts)
+{
+	return -EOPNOTSUPP;
+}
+
+static int hv_ptp_adjfreq(struct ptp_clock_info *ptp, s32 delta)
+{
+	return -EOPNOTSUPP;
+}
+static int hv_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	return -EOPNOTSUPP;
+}
+
+static int hv_ptp_gettime(struct ptp_clock_info *info, struct timespec64 *ts)
+{
+	u64 newtime;
+	unsigned long flags;
+
+	spin_lock_irqsave(&host_ts.lock, flags);
+	newtime = host_ts.host_time + get_timeadj_latency(host_ts.ref_time);
+	*ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
+	spin_unlock_irqrestore(&host_ts.lock, flags);
+
+	return 0;
+}
+
+static const struct ptp_clock_info ptp_hyperv_info = {
+	.name		= "hyperv",
+	.enable         = hv_ptp_enable,
+	.adjtime        = hv_ptp_adjtime,
+	.adjfreq        = hv_ptp_adjfreq,
+	.gettime64      = hv_ptp_gettime,
+	.settime64      = hv_ptp_settime,
+	.owner		= THIS_MODULE,
+};
+
+static struct ptp_clock *hv_ptp_clock;
+
 static int hv_timesync_init(struct hv_util_service *srv)
 {
 	INIT_WORK(&wrk.work, hv_set_host_time);
+
+	/*
+	 * ptp_clock_register() returns NULL when CONFIG_PTP_1588_CLOCK is
+	 * disabled but the driver is still useful without the PTP device
+	 * as it still handles the ICTIMESYNCFLAG_SYNC case.
+	 */
+	hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL);
+	if (IS_ERR_OR_NULL(hv_ptp_clock)) {
+		pr_err("cannot register PTP clock: %ld\n",
+		       PTR_ERR(hv_ptp_clock));
+		hv_ptp_clock = NULL;
+	}
+
 	return 0;
 }
 
 static void hv_timesync_deinit(void)
 {
+	if (hv_ptp_clock)
+		ptp_clock_unregister(hv_ptp_clock);
 	cancel_work_sync(&wrk.work);
 }
 
-- 
2.9.3

Re: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]

Hi Vitaly,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.10-rc4 next-20170119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vitaly-Kuznetsov/hv_util-adjust-system-time-smoothly/20170120-011342
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/hv/hv_util.c: In function 'hv_timesync_init':
>> drivers/hv/hv_util.c:561:36: warning: passing argument 1 of 'ptp_clock_register' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL);
                                       ^
   In file included from drivers/hv/hv_util.c:30:0:
   include/linux/ptp_clock_kernel.h:172:26: note: expected 'struct ptp_clock_info *' but argument is of type 'const struct ptp_clock_info *'
    extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
                             ^~~~~~~~~~~~~~~~~~

vim +561 drivers/hv/hv_util.c

   545		.gettime64      = hv_ptp_gettime,
   546		.settime64      = hv_ptp_settime,
   547		.owner		= THIS_MODULE,
   548	};
   549	
   550	static struct ptp_clock *hv_ptp_clock;
   551	
   552	static int hv_timesync_init(struct hv_util_service *srv)
   553	{
   554		INIT_WORK(&wrk.work, hv_set_host_time);
   555	
   556		/*
   557		 * ptp_clock_register() returns NULL when CONFIG_PTP_1588_CLOCK is
   558		 * disabled but the driver is still useful without the PTP device
   559		 * as it still handles the ICTIMESYNCFLAG_SYNC case.
   560		 */
 > 561		hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL);
   562		if (IS_ERR_OR_NULL(hv_ptp_clock)) {
   563			pr_err("cannot register PTP clock: %ld\n",
   564			       PTR_ERR(hv_ptp_clock));
   565			hv_ptp_clock = NULL;
   566		}
   567	
   568		return 0;
   569	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38264 bytes --]

Re: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source

[Vitaly Kuznetsov]

kbuild test robot <lkp@intel.com> writes:

> Hi Vitaly,
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v4.10-rc4 next-20170119]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Vitaly-Kuznetsov/hv_util-adjust-system-time-smoothly/20170120-011342
> config: x86_64-rhel (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
>
> All warnings (new ones prefixed by >>):
>
>    drivers/hv/hv_util.c: In function 'hv_timesync_init':
>>> drivers/hv/hv_util.c:561:36: warning: passing argument 1 of 'ptp_clock_register' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
>      hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL);
>                                        ^
>    In file included from drivers/hv/hv_util.c:30:0:
>    include/linux/ptp_clock_kernel.h:172:26: note: expected 'struct ptp_clock_info *' but argument is of type 'const struct ptp_clock_info *'
>     extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
>                              ^~~~~~~~~~~~~~~~~~
>

It seems ptp_clock_register() could've been changed to take 'const
struct ptp_clock_info *' such a change doesn't belong this series, I'll
drop the 'const' qualifier in the next submission.

-- 
  Vitaly

Re: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source

[Radim Krcmar]

2017-01-19 15:16+0100, Vitaly Kuznetsov:
> With TimeSync version 4 protocol support we started updating system time
> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
> there is a time sample from the host which triggers do_settimeofday[64]().
> While the time from the host is very accurate such adjustments may cause
> issues:
> - Time is jumping forward and backward, some applications may misbehave.
> - In case an NTP server runs in parallel and uses something else for time
>   sync (network, PTP,...) system time will never converge.
> - Systemd starts annoying you by printing "Time has been changed" every 5
>   seconds to the system log.
> 
> Instead of doing in-kernel time adjustments offload the work to an
> NTP client by exposing TimeSync messages as a PTP device. Users may now
> decide what they want to use as a source.
> 
> I tested the solution with chrony, the config was:
> 
>  refclock PHC /dev/ptp0 poll 3 precision 1e-9
> 
> The result I'm seeing is accurate enough, the time delta between the guest
> and the host is almost always within [-10us, +10us], the in-kernel solution
> was giving us comparable results.
> 
> I also tried implementing PPS device instead of PTP by using not currently
> used Hyper-V synthetic timers (we use only one of four for clockevent) but
> with PPS source only chrony wasn't able to give me the required accuracy,
> the delta often more that 100us.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---

It is a nice coincidence that KVM is working on a PTP driver as well,
https://lkml.org/lkml/2017/1/20/247, and it uses more precise/accurate
method of converting the host time that Hyper-V could also use.

Hyper-V provides {host_time, ref_time} tuple, but gettime64() requires
that you return just host_time and a new "ref_time" is then computed to
be in the middle of two guest_time reads.
I recommend you use getcrosststamp PTP callback, which allows you to
provide the tuple.  Userspace can then use PTP_SYS_OFFSET_PRECISE ioctl.

KVM patches also proposes to change PTP_SYS_OFFSET, so when gettime64
callback is not implemented, the ioctl uses getcrosststamp instead,
which would avoid code duplication and improve precision/accuracy.

Re: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source

[Vitaly Kuznetsov]

Radim Krcmar <rkrcmar@redhat.com> writes:

> 2017-01-19 15:16+0100, Vitaly Kuznetsov:
>> With TimeSync version 4 protocol support we started updating system time
>> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
>> there is a time sample from the host which triggers do_settimeofday[64]().
>> While the time from the host is very accurate such adjustments may cause
>> issues:
>> - Time is jumping forward and backward, some applications may misbehave.
>> - In case an NTP server runs in parallel and uses something else for time
>>   sync (network, PTP,...) system time will never converge.
>> - Systemd starts annoying you by printing "Time has been changed" every 5
>>   seconds to the system log.
>> 
>> Instead of doing in-kernel time adjustments offload the work to an
>> NTP client by exposing TimeSync messages as a PTP device. Users may now
>> decide what they want to use as a source.
>> 
>> I tested the solution with chrony, the config was:
>> 
>>  refclock PHC /dev/ptp0 poll 3 precision 1e-9
>> 
>> The result I'm seeing is accurate enough, the time delta between the guest
>> and the host is almost always within [-10us, +10us], the in-kernel solution
>> was giving us comparable results.
>> 
>> I also tried implementing PPS device instead of PTP by using not currently
>> used Hyper-V synthetic timers (we use only one of four for clockevent) but
>> with PPS source only chrony wasn't able to give me the required accuracy,
>> the delta often more that 100us.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>
> It is a nice coincidence that KVM is working on a PTP driver as well,
> https://lkml.org/lkml/2017/1/20/247, and it uses more precise/accurate
> method of converting the host time that Hyper-V could also use.
>
> Hyper-V provides {host_time, ref_time} tuple, but gettime64() requires
> that you return just host_time and a new "ref_time" is then computed to
> be in the middle of two guest_time reads.
> I recommend you use getcrosststamp PTP callback, which allows you to
> provide the tuple.  Userspace can then use PTP_SYS_OFFSET_PRECISE
> ioctl.

Thanks, good suggestion,

as far as I see PTP_SYS_OFFSET_PRECISE support was just added to chrony:
https://git.tuxfamily.org/chrony/chrony.git/commit/?id=31b6a14444a8f23147077df3c6a64518d082c35e

I'll implement getcrosststamp() as well but I'll probably have to wait
till K. Y.'s restructuring (https://lkml.org/lkml/2016/12/30/260) lands
and do my series on top of that. We'll also be using the TSC page
clocksource (when available) to not do the unneeded vmexit.

>
> KVM patches also proposes to change PTP_SYS_OFFSET, so when gettime64
> callback is not implemented, the ioctl uses getcrosststamp instead,
> which would avoid code duplication and improve precision/accuracy.

It's probably still worth it to have the 'lightweight' gettime64()
implementation as going through the getcrosststamp() routine (see
get_device_system_crosststamp() which we'll probably use for Hyper-V
also -- it's not very simple) every time and throwing away half of the
result doesn't look optimal.

-- 
  Vitaly

RE: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source

[KY Srinivasan]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Thursday, January 19, 2017 6:17 AM
> To: devel@linuxdriverproject.org
> Cc: Thomas Gleixner <tglx@linutronix.de>; linux-kernel@vger.kernel.org;
> Haiyang Zhang <haiyangz@microsoft.com>; KY Srinivasan
> <kys@microsoft.com>; John Stultz <john.stultz@linaro.org>; Alex Ng (LIS)
> <alexng@microsoft.com>; Stephen Hemminger
> <stephen@networkplumber.org>; Olaf Hering <olaf@aepfle.de>; Richard
> Cochran <richardcochran@gmail.com>; Radim Krcmar
> <rkrcmar@redhat.com>
> Subject: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source
> 
> With TimeSync version 4 protocol support we started updating system time
> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
> there is a time sample from the host which triggers do_settimeofday[64]().
> While the time from the host is very accurate such adjustments may cause
> issues:
> - Time is jumping forward and backward, some applications may misbehave.
> - In case an NTP server runs in parallel and uses something else for time
>   sync (network, PTP,...) system time will never converge.
> - Systemd starts annoying you by printing "Time has been changed" every 5
>   seconds to the system log.
> 
> Instead of doing in-kernel time adjustments offload the work to an
> NTP client by exposing TimeSync messages as a PTP device. Users may now
> decide what they want to use as a source.
> 
> I tested the solution with chrony, the config was:
> 
>  refclock PHC /dev/ptp0 poll 3 precision 1e-9
> 
> The result I'm seeing is accurate enough, the time delta between the guest
> and the host is almost always within [-10us, +10us], the in-kernel solution
> was giving us comparable results.
> 
> I also tried implementing PPS device instead of PTP by using not currently
> used Hyper-V synthetic timers (we use only one of four for clockevent) but
> with PPS source only chrony wasn't able to give me the required accuracy,
> the delta often more that 100us.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Vitaly,

This patch does not apply. Please rebase and send.

K. Y
> ---
>  drivers/hv/hv_util.c | 144
> ++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 119 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 94719eb..e1211b8 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -27,6 +27,7 @@
>  #include <linux/sysctl.h>
>  #include <linux/reboot.h>
>  #include <linux/hyperv.h>
> +#include <linux/ptp_clock_kernel.h>
> 
>  #include "hyperv_vmbus.h"
> 
> @@ -179,31 +180,34 @@ struct adj_time_work {
>  	u8	flags;
>  };
> 
> +static u64 get_timeadj_latency(u64 ref_time)
> +{
> +	u64 current_tick;
> +
> +	if (ts_srv_version <= TS_VERSION_3)
> +		return 0;
> +
> +	/*
> +	 * Some latency has been introduced since Hyper-V generated
> +	 * its time sample. Take that latency into account before
> +	 * using TSC reference time sample from Hyper-V.
> +	 *
> +	 * This sample is given by TimeSync v4 and above hosts.
> +	 */
> +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> +	return current_tick - ref_time;
> +}
> +
>  static void hv_set_host_time(struct work_struct *work)
>  {
>  	struct adj_time_work	*wrk;
> -	s64 host_tns;
> -	u64 newtime;
>  	struct timespec64 host_ts;
> +	u64 newtime;
> 
>  	wrk = container_of(work, struct adj_time_work, work);
> 
> -	newtime = wrk->host_time;
> -	if (ts_srv_version > TS_VERSION_3) {
> -		/*
> -		 * Some latency has been introduced since Hyper-V
> generated
> -		 * its time sample. Take that latency into account before
> -		 * using TSC reference time sample from Hyper-V.
> -		 *
> -		 * This sample is given by TimeSync v4 and above hosts.
> -		 */
> -		u64 current_tick;
> -
> -		rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> -		newtime += (current_tick - wrk->ref_time);
> -	}
> -	host_tns = (newtime - WLTIMEDELTA) * 100;
> -	host_ts = ns_to_timespec64(host_tns);
> +	newtime = wrk->host_time + get_timeadj_latency(wrk->ref_time);
> +	host_ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
> 
>  	do_settimeofday64(&host_ts);
>  }
> @@ -222,22 +226,52 @@ static void hv_set_host_time(struct work_struct
> *work)
>   * to discipline the clock.
>   */
>  static struct adj_time_work  wrk;
> -static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 flags)
> +
> +/*
> + * The last time sample, received from the host. PTP device responds to
> + * requests by using this data and the current partition-wide time reference
> + * count.
> + */
> +static struct {
> +	u64		host_time;
> +	u64		ref_time;
> +	spinlock_t	lock;
> +} host_ts;
> +
> +static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
>  {
> +	unsigned long flags;
> 
>  	/*
>  	 * This check is safe since we are executing in the
>  	 * interrupt context and time synch messages arre always
>  	 * delivered on the same CPU.
>  	 */
> -	if (work_pending(&wrk.work))
> -		return;
> +	if (adj_flags & ICTIMESYNCFLAG_SYNC) {
> +		if (work_pending(&wrk.work))
> +			return;
> 
> -	wrk.host_time = hosttime;
> -	wrk.ref_time = reftime;
> -	wrk.flags = flags;
> -	if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) !=
> 0) {
> +		wrk.host_time = hosttime;
> +		wrk.ref_time = reftime;
> +		wrk.flags = adj_flags;
>  		schedule_work(&wrk.work);
> +	} else {
> +		spin_lock_irqsave(&host_ts.lock, flags);
> +		host_ts.host_time = hosttime;
> +
> +		/*
> +		 * Prior to version 4 TimeSync messages from the host don't
> +		 * contain any reference time (the time when the time
> sample
> +		 * was generated), save the current time reference count
> +		 * instead. This adds a small delta between the time sample
> +		 * generation and the reception of the sample here to the
> result
> +		 * but it's the best thing we can do.
> +		 */
> +		if (ts_srv_version <= TS_VERSION_3)
> +			rdmsrl(HV_X64_MSR_TIME_REF_COUNT,
> host_ts.ref_time);
> +		else
> +			host_ts.ref_time = reftime;
> +		spin_unlock_irqrestore(&host_ts.lock, flags);
>  	}
>  }
> 
> @@ -470,14 +504,74 @@ static  struct hv_driver util_drv = {
>  	.remove =  util_remove,
>  };
> 
> +static int hv_ptp_enable(struct ptp_clock_info *info,
> +			 struct ptp_clock_request *request, int on)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int hv_ptp_settime(struct ptp_clock_info *p, const struct timespec64
> *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int hv_ptp_adjfreq(struct ptp_clock_info *ptp, s32 delta)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static int hv_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int hv_ptp_gettime(struct ptp_clock_info *info, struct timespec64
> *ts)
> +{
> +	u64 newtime;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&host_ts.lock, flags);
> +	newtime = host_ts.host_time +
> get_timeadj_latency(host_ts.ref_time);
> +	*ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
> +	spin_unlock_irqrestore(&host_ts.lock, flags);
> +
> +	return 0;
> +}
> +
> +static const struct ptp_clock_info ptp_hyperv_info = {
> +	.name		= "hyperv",
> +	.enable         = hv_ptp_enable,
> +	.adjtime        = hv_ptp_adjtime,
> +	.adjfreq        = hv_ptp_adjfreq,
> +	.gettime64      = hv_ptp_gettime,
> +	.settime64      = hv_ptp_settime,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static struct ptp_clock *hv_ptp_clock;
> +
>  static int hv_timesync_init(struct hv_util_service *srv)
>  {
>  	INIT_WORK(&wrk.work, hv_set_host_time);
> +
> +	/*
> +	 * ptp_clock_register() returns NULL when
> CONFIG_PTP_1588_CLOCK is
> +	 * disabled but the driver is still useful without the PTP device
> +	 * as it still handles the ICTIMESYNCFLAG_SYNC case.
> +	 */
> +	hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL);
> +	if (IS_ERR_OR_NULL(hv_ptp_clock)) {
> +		pr_err("cannot register PTP clock: %ld\n",
> +		       PTR_ERR(hv_ptp_clock));
> +		hv_ptp_clock = NULL;
> +	}
> +
>  	return 0;
>  }
> 
>  static void hv_timesync_deinit(void)
>  {
> +	if (hv_ptp_clock)
> +		ptp_clock_unregister(hv_ptp_clock);
>  	cancel_work_sync(&wrk.work);
>  }
> 
> --
> 2.9.3

Re: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source

[Vitaly Kuznetsov]

KY Srinivasan <kys@microsoft.com> writes:

>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> Sent: Thursday, January 19, 2017 6:17 AM
>> To: devel@linuxdriverproject.org
>> Cc: Thomas Gleixner <tglx@linutronix.de>; linux-kernel@vger.kernel.org;
>> Haiyang Zhang <haiyangz@microsoft.com>; KY Srinivasan
>> <kys@microsoft.com>; John Stultz <john.stultz@linaro.org>; Alex Ng (LIS)
>> <alexng@microsoft.com>; Stephen Hemminger
>> <stephen@networkplumber.org>; Olaf Hering <olaf@aepfle.de>; Richard
>> Cochran <richardcochran@gmail.com>; Radim Krcmar
>> <rkrcmar@redhat.com>
>> Subject: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source
>> 
>> With TimeSync version 4 protocol support we started updating system time
>> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
>> there is a time sample from the host which triggers do_settimeofday[64]().
>> While the time from the host is very accurate such adjustments may cause
>> issues:
>> - Time is jumping forward and backward, some applications may misbehave.
>> - In case an NTP server runs in parallel and uses something else for time
>>   sync (network, PTP,...) system time will never converge.
>> - Systemd starts annoying you by printing "Time has been changed" every 5
>>   seconds to the system log.
>> 
>> Instead of doing in-kernel time adjustments offload the work to an
>> NTP client by exposing TimeSync messages as a PTP device. Users may now
>> decide what they want to use as a source.
>> 
>> I tested the solution with chrony, the config was:
>> 
>>  refclock PHC /dev/ptp0 poll 3 precision 1e-9
>> 
>> The result I'm seeing is accurate enough, the time delta between the guest
>> and the host is almost always within [-10us, +10us], the in-kernel solution
>> was giving us comparable results.
>> 
>> I also tried implementing PPS device instead of PTP by using not currently
>> used Hyper-V synthetic timers (we use only one of four for clockevent) but
>> with PPS source only chrony wasn't able to give me the required accuracy,
>> the delta often more that 100us.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>
> Vitaly,
>
> This patch does not apply. Please rebase and send.
>

Sure, I'm currently testing v5 which includes the .getcrosststamp()
callback support. Will be sending it out shortly.

-- 
  Vitaly