[PATCH 3/6] hv_utils: fix TimeSync work on pre-TimeSync-v4 hosts

[kys]

From: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>

It was found that ICTIMESYNCFLAG_SYNC packets are handled incorrectly
on WS2012R2, e.g. after the guest is paused and resumed its time is set
to something different from host's time. The problem is that we call
adj_guesttime() with reftime=0 for these old hosts and we don't account
for that in 'if (adj_flags & ICTIMESYNCFLAG_SYNC)' branch and
hv_set_host_time().

While we could've solved this by adding a check like
'if (ts_srv_version > TS_VERSION_3)' to hv_set_host_time() I prefer
to do some refactoring. We don't need to have two separate containers
for host samples, struct host_ts which we use for PTP is enough.

Throw away 'struct adj_time_work' and create hv_get_adj_host_time()
accessor to host_ts to avoid code duplication.

Fixes: 3716a49a81ba ("hv_utils: implement Hyper-V PTP source")
Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
---
 drivers/hv/hv_util.c |  128 +++++++++++++++++++++-----------------------------
 1 files changed, 54 insertions(+), 74 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 2849143..14dce25 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -202,27 +202,39 @@ static void shutdown_onchannelcallback(void *context)
 /*
  * Set the host time in a process context.
  */
+static struct work_struct adj_time_work;
 
-struct adj_time_work {
-	struct work_struct work;
-	u64	host_time;
-	u64	ref_time;
-	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 void hv_set_host_time(struct work_struct *work)
+static struct timespec64 hv_get_adj_host_time(void)
 {
-	struct adj_time_work *wrk;
-	struct timespec64 host_ts;
-	u64 reftime, newtime;
-
-	wrk = container_of(work, struct adj_time_work, work);
+	struct timespec64 ts;
+	u64 newtime, reftime;
+	unsigned long flags;
 
+	spin_lock_irqsave(&host_ts.lock, flags);
 	reftime = hyperv_cs->read(hyperv_cs);
-	newtime = wrk->host_time + (reftime - wrk->ref_time);
-	host_ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
+	newtime = host_ts.host_time + (reftime - host_ts.ref_time);
+	ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
+	spin_unlock_irqrestore(&host_ts.lock, flags);
 
-	do_settimeofday64(&host_ts);
+	return ts;
+}
+
+static void hv_set_host_time(struct work_struct *work)
+{
+	struct timespec64 ts = hv_get_adj_host_time();
+
+	do_settimeofday64(&ts);
 }
 
 /*
@@ -238,60 +250,35 @@ 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 struct adj_time_work  wrk;
-
-/*
- * 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;
 	u64 cur_reftime;
 
 	/*
-	 * This check is safe since we are executing in the
-	 * interrupt context and time synch messages are always
-	 * delivered on the same CPU.
+	 * Save the adjusted time sample from the host and the snapshot
+	 * of the current system time.
 	 */
-	if (adj_flags & ICTIMESYNCFLAG_SYNC) {
-		/* Queue a job to do do_settimeofday64() */
-		if (work_pending(&wrk.work))
-			return;
-
-		wrk.host_time = hosttime;
-		wrk.ref_time = reftime;
-		wrk.flags = adj_flags;
-		schedule_work(&wrk.work);
-	} else {
-		/*
-		 * Save the adjusted time sample from the host and the snapshot
-		 * of the current system time for PTP device.
-		 */
-		spin_lock_irqsave(&host_ts.lock, flags);
-
-		cur_reftime = hyperv_cs->read(hyperv_cs);
-		host_ts.host_time = hosttime;
-		host_ts.ref_time = cur_reftime;
-
-		/*
-		 * TimeSync v4 messages contain reference time (guest's Hyper-V
-		 * clocksource read when the time sample was generated), we can
-		 * improve the precision by adding the delta between now and the
-		 * time of generation.
-		 */
-		if (ts_srv_version > TS_VERSION_3)
-			host_ts.host_time += (cur_reftime - reftime);
-
-		spin_unlock_irqrestore(&host_ts.lock, flags);
-	}
+	spin_lock_irqsave(&host_ts.lock, flags);
+
+	cur_reftime = hyperv_cs->read(hyperv_cs);
+	host_ts.host_time = hosttime;
+	host_ts.ref_time = cur_reftime;
+
+	/*
+	 * TimeSync v4 messages contain reference time (guest's Hyper-V
+	 * clocksource read when the time sample was generated), we can
+	 * improve the precision by adding the delta between now and the
+	 * time of generation. For older protocols we set
+	 * reftime == cur_reftime on call.
+	 */
+	host_ts.host_time += (cur_reftime - reftime);
+
+	spin_unlock_irqrestore(&host_ts.lock, flags);
+
+	/* Schedule work to do do_settimeofday64() */
+	if (adj_flags & ICTIMESYNCFLAG_SYNC)
+		schedule_work(&adj_time_work);
 }
 
 /*
@@ -339,8 +326,8 @@ static void timesync_onchannelcallback(void *context)
 					sizeof(struct vmbuspipe_hdr) +
 					sizeof(struct icmsg_hdr)];
 				adj_guesttime(timedatap->parenttime,
-						0,
-						timedatap->flags);
+					      hyperv_cs->read(hyperv_cs),
+					      timedatap->flags);
 			}
 		}
 
@@ -524,14 +511,7 @@ static int hv_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 
 static int hv_ptp_gettime(struct ptp_clock_info *info, struct timespec64 *ts)
 {
-	unsigned long flags;
-	u64 newtime, reftime;
-
-	spin_lock_irqsave(&host_ts.lock, flags);
-	reftime = hyperv_cs->read(hyperv_cs);
-	newtime = host_ts.host_time + (reftime - host_ts.ref_time);
-	*ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
-	spin_unlock_irqrestore(&host_ts.lock, flags);
+	*ts = hv_get_adj_host_time();
 
 	return 0;
 }
@@ -556,7 +536,7 @@ static int hv_timesync_init(struct hv_util_service *srv)
 
 	spin_lock_init(&host_ts.lock);
 
-	INIT_WORK(&wrk.work, hv_set_host_time);
+	INIT_WORK(&adj_time_work, hv_set_host_time);
 
 	/*
 	 * ptp_clock_register() returns NULL when CONFIG_PTP_1588_CLOCK is
@@ -577,7 +557,7 @@ static void hv_timesync_deinit(void)
 {
 	if (hv_ptp_clock)
 		ptp_clock_unregister(hv_ptp_clock);
-	cancel_work_sync(&wrk.work);
+	cancel_work_sync(&adj_time_work);
 }
 
 static int __init init_hyperv_utils(void)
-- 
1.7.1