[PATCH 0/6] Drivers: hv: vmbus: Some miscellaneous fixes

[PATCH 0/6] Drivers: hv: vmbus: Some miscellaneous fixes

[kys]

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

Some miscellaneous fixes.

Andy Shevchenko (1):
  vmbus: Reuse uuid_le_to_bin() helper

K. Y. Srinivasan (2):
  Drivers: hv: vmbus: Get the current time from the current clocksource
  drivers: hv: vmbus: Increase the time between retries in
    vmbus_post_msg()

Michael Kelley (1):
  Drivers: hv: vmbus: Close timing hole that can corrupt per-cpu page

Vitaly Kuznetsov (2):
  hv_utils: drop .getcrosststamp() support from PTP driver
  hv_utils: fix TimeSync work on pre-TimeSync-v4 hosts

 arch/x86/include/asm/mshyperv.h |    1 -
 drivers/hv/connection.c         |    2 +-
 drivers/hv/hv.c                 |    9 ++-
 drivers/hv/hv_util.c            |  164 +++++++++++++--------------------------
 drivers/hv/vmbus_drv.c          |   51 +++----------
 5 files changed, 72 insertions(+), 155 deletions(-)

[PATCH 1/6] Drivers: hv: vmbus: Get the current time from the current clocksource

[kys]

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

The current code uses the MSR based mechanism to get the current tick.
Use the current clock source as that might be more optimal.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/include/asm/mshyperv.h |    1 -
 drivers/hv/hv.c                 |    2 +-
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 7c9c895..7b07853 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -137,7 +137,6 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
 	}
 }
 
-#define hv_get_current_tick(tick) rdmsrl(HV_X64_MSR_TIME_REF_COUNT, tick)
 #define hv_init_timer(timer, tick) wrmsrl(timer, tick)
 #define hv_init_timer_config(config, val) wrmsrl(config, val)
 
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 12e7bae..61fc8ce 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -96,7 +96,7 @@ static int hv_ce_set_next_event(unsigned long delta,
 
 	WARN_ON(!clockevent_state_oneshot(evt));
 
-	hv_get_current_tick(current_tick);
+	current_tick = hyperv_cs->read(NULL);
 	current_tick += delta;
 	hv_init_timer(HV_X64_MSR_STIMER0_COUNT, current_tick);
 	return 0;
-- 
1.7.1

[PATCH 2/6] hv_utils: drop .getcrosststamp() support from PTP driver

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Turns out that our implementation of .getcrosststamp() never actually
worked. Hyper-V is sending time samples every 5 seconds and this is
too much for get_device_system_crosststamp() as it's interpolation
algorithm (which nobody is currently using in kernel, btw) accounts
for a 'slow' device but we're not slow in Hyper-V, our time reference
is too far away.

.getcrosststamp() is not currently used, get_device_system_crosststamp()
almost always returns -EINVAL and client falls back to using PTP_SYS_OFFSET
so this patch doesn't change much. I also tried doing interpolation
manually (e.g. the same way hv_ptp_gettime() works and it turns out that
we're getting even lower quality:

PTP_SYS_OFFSET_PRECISE with manual interpolation:
* PHC0                     0   3    37     4  -3974ns[-5338ns] +/-  977ns
* PHC0                     0   3    77     7  +2227ns[+3184ns] +/-  576ns
* PHC0                     0   3   177    10  +3060ns[+5220ns] +/-  548ns
* PHC0                     0   3   377    12  +3937ns[+4371ns] +/- 1414ns
* PHC0                     0   3   377     6   +764ns[+1240ns] +/- 1047ns
* PHC0                     0   3   377     7  -1210ns[-3731ns] +/-  479ns
* PHC0                     0   3   377     9   +153ns[-1019ns] +/-  406ns
* PHC0                     0   3   377    12   -872ns[-1793ns] +/-  443ns
* PHC0                     0   3   377     5   +701ns[+3599ns] +/-  426ns
* PHC0                     0   3   377     5   -923ns[ -375ns] +/- 1062ns

PTP_SYS_OFFSET:
* PHC0                     0   3     7     5    +72ns[+8020ns] +/-  251ns
* PHC0                     0   3    17     5   -885ns[-3661ns] +/-  254ns
* PHC0                     0   3    37     6   -454ns[-5732ns] +/-  258ns
* PHC0                     0   3    77    10  +1183ns[+3754ns] +/-  164ns
* PHC0                     0   3   377     5   +579ns[+1137ns] +/-  110ns
* PHC0                     0   3   377     7   +501ns[+1064ns] +/-   96ns
* PHC0                     0   3   377     9  +1641ns[+3342ns] +/-  106ns
* PHC0                     0   3   377     8    -47ns[  +77ns] +/-  160ns
* PHC0                     0   3   377     5    +54ns[ +107ns] +/-  102ns
* PHC0                     0   3   377     8   -354ns[ -617ns] +/-   89ns

This fact wasn't noticed during the initial testing of the PTP device
somehow but got revealed now. Let's just drop .getcrosststamp()
implementation for now as it doesn't seem to be suitable for us.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_util.c |   36 ------------------------------------
 1 files changed, 0 insertions(+), 36 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 186b100..2849143 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -248,7 +248,6 @@ static void hv_set_host_time(struct work_struct *work)
 static struct {
 	u64				host_time;
 	u64				ref_time;
-	struct system_time_snapshot	snap;
 	spinlock_t			lock;
 } host_ts;
 
@@ -281,7 +280,6 @@ static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
 		cur_reftime = hyperv_cs->read(hyperv_cs);
 		host_ts.host_time = hosttime;
 		host_ts.ref_time = cur_reftime;
-		ktime_get_snapshot(&host_ts.snap);
 
 		/*
 		 * TimeSync v4 messages contain reference time (guest's Hyper-V
@@ -538,46 +536,12 @@ static int hv_ptp_gettime(struct ptp_clock_info *info, struct timespec64 *ts)
 	return 0;
 }
 
-static int hv_ptp_get_syncdevicetime(ktime_t *device,
-				     struct system_counterval_t *system,
-				     void *ctx)
-{
-	system->cs = hyperv_cs;
-	system->cycles = host_ts.ref_time;
-	*device = ns_to_ktime((host_ts.host_time - WLTIMEDELTA) * 100);
-
-	return 0;
-}
-
-static int hv_ptp_getcrosststamp(struct ptp_clock_info *ptp,
-				 struct system_device_crosststamp *xtstamp)
-{
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&host_ts.lock, flags);
-
-	/*
-	 * host_ts contains the last time sample from the host and the snapshot
-	 * of system time. We don't need to calculate the time delta between
-	 * the reception and now as get_device_system_crosststamp() does the
-	 * required interpolation.
-	 */
-	ret = get_device_system_crosststamp(hv_ptp_get_syncdevicetime,
-					    NULL, &host_ts.snap, xtstamp);
-
-	spin_unlock_irqrestore(&host_ts.lock, flags);
-
-	return ret;
-}
-
 static 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,
-	.getcrosststamp = hv_ptp_getcrosststamp,
 	.settime64      = hv_ptp_settime,
 	.owner		= THIS_MODULE,
 };
-- 
1.7.1

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

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

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@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 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

[PATCH 4/6] drivers: hv: vmbus: Increase the time between retries in vmbus_post_msg()

[kys]

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

Commit c0bb03924f1a ("Drivers: hv: vmbus: Raise retry/wait limits in
vmbus_post_msg()") increased the retry/wait limits of vmbus_post_msg()
to address the new DoS protection policies in WS2016.
Increase the time between retries to make the
code more robust.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/connection.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index c2d74ee..59c11ff 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -390,7 +390,7 @@ int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep)
 		else
 			mdelay(usec / 1000);
 
-		if (usec < 256000)
+		if (retries < 22)
 			usec *= 2;
 	}
 	return ret;
-- 
1.7.1

[PATCH 5/6] vmbus: Reuse uuid_le_to_bin() helper

[kys]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Instead of open coded variant use generic helper to convert UUID strings
to binary format.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/vmbus_drv.c |   51 +++++++++--------------------------------------
 1 files changed, 10 insertions(+), 41 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 59bb3ef..ed84e96 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -608,40 +608,6 @@ static void vmbus_free_dynids(struct hv_driver *drv)
 	spin_unlock(&drv->dynids.lock);
 }
 
-/* Parse string of form: 1b4e28ba-2fa1-11d2-883f-b9a761bde3f */
-static int get_uuid_le(const char *str, uuid_le *uu)
-{
-	unsigned int b[16];
-	int i;
-
-	if (strlen(str) < 37)
-		return -1;
-
-	for (i = 0; i < 36; i++) {
-		switch (i) {
-		case 8: case 13: case 18: case 23:
-			if (str[i] != '-')
-				return -1;
-			break;
-		default:
-			if (!isxdigit(str[i]))
-				return -1;
-		}
-	}
-
-	/* unparse little endian output byte order */
-	if (sscanf(str,
-		   "%2x%2x%2x%2x-%2x%2x-%2x%2x-%2x%2x-%2x%2x%2x%2x%2x%2x",
-		   &b[3], &b[2], &b[1], &b[0],
-		   &b[5], &b[4], &b[7], &b[6], &b[8], &b[9],
-		   &b[10], &b[11], &b[12], &b[13], &b[14], &b[15]) != 16)
-		return -1;
-
-	for (i = 0; i < 16; i++)
-		uu->b[i] = b[i];
-	return 0;
-}
-
 /*
  * store_new_id - sysfs frontend to vmbus_add_dynid()
  *
@@ -651,11 +617,12 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
 			    size_t count)
 {
 	struct hv_driver *drv = drv_to_hv_drv(driver);
-	uuid_le guid = NULL_UUID_LE;
+	uuid_le guid;
 	ssize_t retval;
 
-	if (get_uuid_le(buf, &guid) != 0)
-		return -EINVAL;
+	retval = uuid_le_to_bin(buf, &guid);
+	if (retval)
+		return retval;
 
 	if (hv_vmbus_get_id(drv, &guid))
 		return -EEXIST;
@@ -677,12 +644,14 @@ static ssize_t remove_id_store(struct device_driver *driver, const char *buf,
 {
 	struct hv_driver *drv = drv_to_hv_drv(driver);
 	struct vmbus_dynid *dynid, *n;
-	uuid_le guid = NULL_UUID_LE;
-	size_t retval = -ENODEV;
+	uuid_le guid;
+	ssize_t retval;
 
-	if (get_uuid_le(buf, &guid))
-		return -EINVAL;
+	retval = uuid_le_to_bin(buf, &guid);
+	if (retval)
+		return retval;
 
+	retval = -ENODEV;
 	spin_lock(&drv->dynids.lock);
 	list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) {
 		struct hv_vmbus_device_id *id = &dynid->id;
-- 
1.7.1