[PATCH 0/3] Drivers: hv: util: Support the latest time synch protocol from the host

[PATCH 0/3] Drivers: hv: util: Support the latest time synch protocol from the host

[kys]

From: K. Y. Srinivasan <kys@microsoft.com>

WS2016 has introduced a new protocol for conditioning guest clock.
Support this new protocol.

Alex Ng (3):
  Drivers: hv: utils: Rename version definitions to reflect protocol
    version.
  Drivers: hv: utils: Use TimeSync samples to adjust the clock after
    boot.
  Drivers: hv: utils: Support TimeSync version 4.0 protocol samples.

 drivers/hv/hv_util.c   |  122 ++++++++++++++++++++++++++++++------------------
 include/linux/hyperv.h |    9 ++++
 2 files changed, 85 insertions(+), 46 deletions(-)

-- 
1.7.4.1

[PATCH 1/3] Drivers: hv: utils: Rename version definitions to reflect protocol version.

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

Different Windows host versions may reuse the same protocol version when
negotiating the TimeSync, Shutdown, and Heartbeat protocols. We should only
refer to the protocol version to avoid conflating the two concepts.

Signed-off-by: Alex Ng <alexng@messages.microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_util.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index d5acaa2..b27a8ee 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -34,22 +34,22 @@
 #define SD_MINOR	0
 #define SD_VERSION	(SD_MAJOR << 16 | SD_MINOR)
 
-#define SD_WS2008_MAJOR		1
-#define SD_WS2008_VERSION	(SD_WS2008_MAJOR << 16 | SD_MINOR)
+#define SD_MAJOR_1	1
+#define SD_VERSION_1	(SD_MAJOR_1 << 16 | SD_MINOR)
 
 #define TS_MAJOR	3
 #define TS_MINOR	0
 #define TS_VERSION	(TS_MAJOR << 16 | TS_MINOR)
 
-#define TS_WS2008_MAJOR		1
-#define TS_WS2008_VERSION	(TS_WS2008_MAJOR << 16 | TS_MINOR)
+#define TS_MAJOR_1	1
+#define TS_VERSION_1	(TS_MAJOR_1 << 16 | TS_MINOR)
 
 #define HB_MAJOR	3
-#define HB_MINOR 0
+#define HB_MINOR	0
 #define HB_VERSION	(HB_MAJOR << 16 | HB_MINOR)
 
-#define HB_WS2008_MAJOR	1
-#define HB_WS2008_VERSION	(HB_WS2008_MAJOR << 16 | HB_MINOR)
+#define HB_MAJOR_1	1
+#define HB_VERSION_1	(HB_MAJOR_1 << 16 | HB_MINOR)
 
 static int sd_srv_version;
 static int ts_srv_version;
@@ -350,9 +350,9 @@ static int util_probe(struct hv_device *dev,
 	switch (vmbus_proto_version) {
 	case (VERSION_WS2008):
 		util_fw_version = UTIL_WS2K8_FW_VERSION;
-		sd_srv_version = SD_WS2008_VERSION;
-		ts_srv_version = TS_WS2008_VERSION;
-		hb_srv_version = HB_WS2008_VERSION;
+		sd_srv_version = SD_VERSION_1;
+		ts_srv_version = TS_VERSION_1;
+		hb_srv_version = HB_VERSION_1;
 		break;
 
 	default:
-- 
1.7.4.1

[PATCH 2/3] Drivers: hv: utils: Use TimeSync samples to adjust the clock after boot.

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

Only the first 50 samples after boot were being used to discipline the
clock. After the first 50 samples, any samples from the host were ignored
and the guest clock would eventually drift from the host clock.

This patch allows TimeSync-enabled guests to continuously synchronize the
clock with the host clock, even after the first 50 samples.

Signed-off-by: Alex Ng <alexng@messages.microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_util.c |   20 +++++++-------------
 1 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index b27a8ee..4002b71 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -198,29 +198,23 @@ static void hv_set_host_time(struct work_struct *work)
  * ICTIMESYNCFLAG_SYNC flag bit indicates reboot, restore events of the VM.
  * After reboot the flag ICTIMESYNCFLAG_SYNC is included in the first time
  * message after the timesync channel is opened. Since the hv_utils module is
- * loaded after hv_vmbus, the first message is usually missed. The other
- * thing is, systime is automatically set to emulated hardware clock which may
- * not be UTC time or in the same time zone. So, to override these effects, we
- * use the first 50 time samples for initial system time setting.
+ * loaded after hv_vmbus, the first message is usually missed. This bit is
+ * considered a hard request to discipline the clock.
+ *
+ * ICTIMESYNCFLAG_SAMPLE bit indicates a time sample from host. This is
+ * typically used as a hint to the guest. The guest is under no obligation
+ * to discipline the clock.
  */
 static inline void adj_guesttime(u64 hosttime, u8 flags)
 {
 	struct adj_time_work    *wrk;
-	static s32 scnt = 50;
 
 	wrk = kmalloc(sizeof(struct adj_time_work), GFP_ATOMIC);
 	if (wrk == NULL)
 		return;
 
 	wrk->host_time = hosttime;
-	if ((flags & ICTIMESYNCFLAG_SYNC) != 0) {
-		INIT_WORK(&wrk->work, hv_set_host_time);
-		schedule_work(&wrk->work);
-		return;
-	}
-
-	if ((flags & ICTIMESYNCFLAG_SAMPLE) != 0 && scnt > 0) {
-		scnt--;
+	if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) != 0) {
 		INIT_WORK(&wrk->work, hv_set_host_time);
 		schedule_work(&wrk->work);
 	} else
-- 
1.7.4.1

[PATCH 3/3] Drivers: hv: utils: Support TimeSync version 4.0 protocol samples.

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

This enables support for more accurate TimeSync v4 samples when hosted
under Windows Server 2016 and newer hosts.

The new time samples include a "vmreferencetime" field that represents
the guest's TSC value when the host generated its time sample. This value
lets the guest calculate the latency in receiving the time sample. The
latency is added to the sample host time prior to updating the clock.

Signed-off-by: Alex Ng <alexng@messages.microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_util.c   |   82 ++++++++++++++++++++++++++++++++++-------------
 include/linux/hyperv.h |    9 +++++
 2 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 4002b71..6286bdc 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -37,13 +37,16 @@
 #define SD_MAJOR_1	1
 #define SD_VERSION_1	(SD_MAJOR_1 << 16 | SD_MINOR)
 
-#define TS_MAJOR	3
+#define TS_MAJOR	4
 #define TS_MINOR	0
 #define TS_VERSION	(TS_MAJOR << 16 | TS_MINOR)
 
 #define TS_MAJOR_1	1
 #define TS_VERSION_1	(TS_MAJOR_1 << 16 | TS_MINOR)
 
+#define TS_MAJOR_3	3
+#define TS_VERSION_3	(TS_MAJOR_3 << 16 | TS_MINOR)
+
 #define HB_MAJOR	3
 #define HB_MINOR	0
 #define HB_VERSION	(HB_MAJOR << 16 | HB_MINOR)
@@ -161,34 +164,43 @@ static void shutdown_onchannelcallback(void *context)
 }
 
 /*
- * Set guest time to host UTC time.
- */
-static inline void do_adj_guesttime(u64 hosttime)
-{
-	s64 host_tns;
-	struct timespec host_ts;
-
-	host_tns = (hosttime - WLTIMEDELTA) * 100;
-	host_ts = ns_to_timespec(host_tns);
-
-	do_settimeofday(&host_ts);
-}
-
-/*
  * Set the host time in a process context.
  */
 
 struct adj_time_work {
 	struct work_struct work;
 	u64	host_time;
+	u64	ref_time;
+	u8	flags;
 };
 
 static void hv_set_host_time(struct work_struct *work)
 {
 	struct adj_time_work	*wrk;
+	s64 host_tns;
+	u64 newtime;
+	struct timespec host_ts;
 
 	wrk = container_of(work, struct adj_time_work, work);
-	do_adj_guesttime(wrk->host_time);
+
+	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_timespec(host_tns);
+
+	do_settimeofday(&host_ts);
 	kfree(wrk);
 }
 
@@ -205,7 +217,7 @@ static void hv_set_host_time(struct work_struct *work)
  * typically used as a hint to the guest. The guest is under no obligation
  * to discipline the clock.
  */
-static inline void adj_guesttime(u64 hosttime, u8 flags)
+static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 flags)
 {
 	struct adj_time_work    *wrk;
 
@@ -214,6 +226,8 @@ static inline void adj_guesttime(u64 hosttime, u8 flags)
 		return;
 
 	wrk->host_time = hosttime;
+	wrk->ref_time = reftime;
+	wrk->flags = flags;
 	if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) != 0) {
 		INIT_WORK(&wrk->work, hv_set_host_time);
 		schedule_work(&wrk->work);
@@ -231,6 +245,7 @@ static void timesync_onchannelcallback(void *context)
 	u64 requestid;
 	struct icmsg_hdr *icmsghdrp;
 	struct ictimesync_data *timedatap;
+	struct ictimesync_ref_data *refdata;
 	u8 *time_txf_buf = util_timesynch.recv_buffer;
 	struct icmsg_negotiate *negop = NULL;
 
@@ -246,11 +261,27 @@ static void timesync_onchannelcallback(void *context)
 						time_txf_buf,
 						util_fw_version,
 						ts_srv_version);
+			pr_info("Using TimeSync version %d.%d\n",
+				ts_srv_version >> 16, ts_srv_version & 0xFFFF);
 		} else {
-			timedatap = (struct ictimesync_data *)&time_txf_buf[
-				sizeof(struct vmbuspipe_hdr) +
-				sizeof(struct icmsg_hdr)];
-			adj_guesttime(timedatap->parenttime, timedatap->flags);
+			if (ts_srv_version > TS_VERSION_3) {
+				refdata = (struct ictimesync_ref_data *)
+					&time_txf_buf[
+					sizeof(struct vmbuspipe_hdr) +
+					sizeof(struct icmsg_hdr)];
+
+				adj_guesttime(refdata->parenttime,
+						refdata->vmreferencetime,
+						refdata->flags);
+			} else {
+				timedatap = (struct ictimesync_data *)
+					&time_txf_buf[
+					sizeof(struct vmbuspipe_hdr) +
+					sizeof(struct icmsg_hdr)];
+				adj_guesttime(timedatap->parenttime,
+						0,
+						timedatap->flags);
+			}
 		}
 
 		icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
@@ -348,12 +379,17 @@ static int util_probe(struct hv_device *dev,
 		ts_srv_version = TS_VERSION_1;
 		hb_srv_version = HB_VERSION_1;
 		break;
-
-	default:
+	case(VERSION_WIN10):
 		util_fw_version = UTIL_FW_VERSION;
 		sd_srv_version = SD_VERSION;
 		ts_srv_version = TS_VERSION;
 		hb_srv_version = HB_VERSION;
+		break;
+	default:
+		util_fw_version = UTIL_FW_VERSION;
+		sd_srv_version = SD_VERSION;
+		ts_srv_version = TS_VERSION_3;
+		hb_srv_version = HB_VERSION;
 	}
 
 	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 0,
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 430619a..7d7cbff 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1423,6 +1423,15 @@ struct ictimesync_data {
 	u8 flags;
 } __packed;
 
+struct ictimesync_ref_data {
+	u64 parenttime;
+	u64 vmreferencetime;
+	u8 flags;
+	char leapflags;
+	char stratum;
+	u8 reserved[3];
+} __packed;
+
 struct hyperv_service_callback {
 	u8 msg_type;
 	char *log_msg;
-- 
1.7.4.1

Re: [PATCH 3/3] Drivers: hv: utils: Support TimeSync version 4.0 protocol samples.

[Olaf Hering]

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

On Thu, Sep 08, kys@exchange.microsoft.com wrote:

> -	default:
> +	case(VERSION_WIN10):
>  		util_fw_version = UTIL_FW_VERSION;
>  		sd_srv_version = SD_VERSION;
>  		ts_srv_version = TS_VERSION;
>  		hb_srv_version = HB_VERSION;
> +		break;
> +	default:
> +		util_fw_version = UTIL_FW_VERSION;
> +		sd_srv_version = SD_VERSION;
> +		ts_srv_version = TS_VERSION_3;
> +		hb_srv_version = HB_VERSION;

Is this correct? An old kernel on a newer host would use the old
protocol. I assume that new host will also know about the old protocol?
Perhaps a better approach would be to list the known existing hosts and
use the new protocol for upcoming, unknown hosts via 'default:'.

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

RE: [PATCH 3/3] Drivers: hv: utils: Support TimeSync version 4.0 protocol samples.

[Alex Ng (LIS)]

> On Thu, Sep 08, kys@exchange.microsoft.com wrote:
> 
> > -	default:
> > +	case(VERSION_WIN10):
> >  		util_fw_version = UTIL_FW_VERSION;
> >  		sd_srv_version = SD_VERSION;
> >  		ts_srv_version = TS_VERSION;
> >  		hb_srv_version = HB_VERSION;
> > +		break;
> > +	default:
> > +		util_fw_version = UTIL_FW_VERSION;
> > +		sd_srv_version = SD_VERSION;
> > +		ts_srv_version = TS_VERSION_3;
> > +		hb_srv_version = HB_VERSION;
> 
> Is this correct? An old kernel on a newer host would use the old protocol. I
> assume that new host will also know about the old protocol?

This is correct. An old kernel uses the old protocol even with the new host.
New hosts understand the old protocol.

> Perhaps a better approach would be to list the known existing hosts and use
> the new protocol for upcoming, unknown hosts via 'default:'.

This is a good idea. I will create another patch that addresses this.

> 
> Olaf

Re: [PATCH 3/3] Drivers: hv: utils: Support TimeSync version 4.0 protocol samples.

[Olaf Hering]

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

On Tue, Sep 13, Alex Ng (LIS) wrote:

> > On Thu, Sep 08, kys@exchange.microsoft.com wrote:
> > Perhaps a better approach would be to list the known existing hosts and use
> > the new protocol for upcoming, unknown hosts via 'default:'.
> This is a good idea. I will create another patch that addresses this.

I think this variant would cover upcoming hosts for an old kernel:

        switch (vmbus_proto_version) {
        case VERSION_WS2008:
                util_fw_version = UTIL_WS2K8_FW_VERSION;
                sd_srv_version = SD_VERSION_1;
                ts_srv_version = TS_VERSION_1;
                hb_srv_version = HB_VERSION_1;
                break;
        case VERSION_WIN7:
        case VERSION_WIN8:
        case VERSION_WIN8_1:
                util_fw_version = UTIL_FW_VERSION;
                sd_srv_version = SD_VERSION;
                ts_srv_version = TS_VERSION_3;
                hb_srv_version = HB_VERSION;
                break;
        case VERSION_WIN10:
        default:
                util_fw_version = UTIL_FW_VERSION;
                sd_srv_version = SD_VERSION;
                ts_srv_version = TS_VERSION;
                hb_srv_version = HB_VERSION;
                break;
        }

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 163 bytes --]