[xenial][PATCH v2 0/8] [Hyper-V] Implement Hyper-V PTP Source

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 0/8] [Hyper-V] Implement Hyper-V PTP Source

Marcelo Henrique Cerri
BugLink: http://bugs.launchpad.net/bugs/1676635

Adding a few more patches that fix additional issues introduced by the
original patch ("hv_utils: implement Hyper-V PTP source").

v1:

The requested commit requires more than 50 additional commits in order
to be applied cleanly. To avoid that, I cherry-picked just the strict
necessary commits and adapted to the Xenial kernel the upstream change
that makes the Hyper-V clock souce available via hyperv_cs.

Dexuan Cui (1):
  Drivers: hv: util: don't forget to init host_ts.lock

K. Y. Srinivasan (2):
  Drivers: hv: util: Use hv_get_current_tick() to get current tick
  Drivers: hv: util: Fix a typo

Marcelo Henrique Cerri (1):
  UBUNTU: SAUCE: hv: make clocksource available for PTP device
    supporting

Vitaly Kuznetsov (4):
  hv_util: switch to using timespec64
  hv_utils: implement Hyper-V PTP source
  hv_utils: drop .getcrosststamp() support from PTP driver
  hv_utils: fix TimeSync work on pre-TimeSync-v4 hosts

 arch/x86/kernel/cpu/mshyperv.c |  23 ------
 drivers/hv/hv.c                |  36 ++++++++-
 drivers/hv/hv_util.c           | 168 +++++++++++++++++++++++++++++------------
 drivers/hv/hyperv_vmbus.h      |   3 +
 4 files changed, 158 insertions(+), 72 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 1/8] UBUNTU: SAUCE: hv: make clocksource available for PTP device supporting

Marcelo Henrique Cerri
BugLink: http://bugs.launchpad.net/bugs/1676635

Make the MSR and TSC clock sources available during the hv_init() and
export them via hyperv_cs in a similar way as the following upstream
commit does:

commit dee863b571b0 ("hv: export current Hyper-V clocksource")

Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 arch/x86/kernel/cpu/mshyperv.c | 23 -----------------------
 drivers/hv/hv.c                | 36 +++++++++++++++++++++++++++++++++++-
 drivers/hv/hyperv_vmbus.h      |  3 +++
 3 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 668df428f8ad..8ffd894d4b8e 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -134,26 +134,6 @@ static uint32_t  __init ms_hyperv_platform(void)
  return 0;
 }
 
-static cycle_t read_hv_clock(struct clocksource *arg)
-{
- cycle_t current_tick;
- /*
- * Read the partition counter to get the current tick count. This count
- * is set to 0 when the partition is created and is incremented in
- * 100 nanosecond units.
- */
- rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
- return current_tick;
-}
-
-static struct clocksource hyperv_cs = {
- .name = "hyperv_clocksource",
- .rating = 400, /* use this when running on Hyperv*/
- .read = read_hv_clock,
- .mask = CLOCKSOURCE_MASK(64),
- .flags = CLOCK_SOURCE_IS_CONTINUOUS,
-};
-
 static unsigned char hv_get_nmi_reason(void)
 {
  return 0;
@@ -209,9 +189,6 @@ static void __init ms_hyperv_init_platform(void)
      "hv_nmi_unknown");
 #endif
 
- if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
- clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
-
 #ifdef CONFIG_X86_IO_APIC
  no_timer_check = 1;
 #endif
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 176ee7d4c243..92fb168a6633 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -191,6 +191,28 @@ static struct clocksource hyperv_cs_tsc = {
 };
 #endif
 
+static cycle_t read_hv_clock_msr(struct clocksource *arg)
+{
+ cycle_t current_tick;
+ /*
+ * Read the partition counter to get the current tick count. This count
+ * is set to 0 when the partition is created and is incremented in
+ * 100 nanosecond units.
+ */
+ rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+ return current_tick;
+}
+
+static struct clocksource hyperv_cs_msr = {
+ .name = "hyperv_clocksource_msr",
+ .rating = 400, /* use this when running on Hyperv*/
+ .read = read_hv_clock_msr,
+ .mask = CLOCKSOURCE_MASK(64),
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+struct clocksource *hyperv_cs;
+EXPORT_SYMBOL_GPL(hyperv_cs);
 
 /*
  * hv_init - Main initialization routine.
@@ -254,9 +276,11 @@ int hv_init(void)
 
  va_tsc = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
  if (!va_tsc)
- goto cleanup;
+ goto register_msr_cs;
  hv_context.tsc_page = va_tsc;
 
+ hyperv_cs = &hyperv_cs_tsc;
+
  rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
 
  tsc_msr.enable = 1;
@@ -266,6 +290,16 @@ int hv_init(void)
  clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
  }
 #endif
+ /*
+ * For 32 bit guests just use the MSR based mechanism for reading
+ * the partition counter.
+ */
+
+register_msr_cs:
+ hyperv_cs = &hyperv_cs_msr;
+ if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
+ clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100);
+
  return 0;
 
 cleanup:
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 8d7f865c1133..f75729b5aed6 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -29,6 +29,7 @@
 #include <asm/sync_bitops.h>
 #include <linux/atomic.h>
 #include <linux/hyperv.h>
+#include <linux/clocksource.h>
 
 /*
  * Timeout for services such as KVP and fcopy.
@@ -719,4 +720,6 @@ enum hvutil_device_state {
  HVUTIL_DEVICE_DYING,     /* driver unload is in progress */
 };
 
+extern struct clocksource *hyperv_cs;
+
 #endif /* _HYPERV_VMBUS_H */
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 2/8] Drivers: hv: util: Use hv_get_current_tick() to get current tick

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
From: K. Y. Srinivasan <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1676635

As part of the effort to interact with Hyper-V in an instruction set
architecture independent way, use the new API to get the current
tick.

Signed-off-by: K. Y. Srinivasan <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
(cherry picked from commit 305f7549c9298247723c255baddb7a54b4e63050)
Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/hv_util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index bcd06306f3e8..07a783b52784 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 <asm/mshyperv.h>
 
 #include "hyperv_vmbus.h"
 
@@ -199,7 +200,7 @@ static void hv_set_host_time(struct work_struct *work)
  */
  u64 current_tick;
 
- rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+ hv_get_current_tick(current_tick);
  newtime += (current_tick - wrk->ref_time);
  }
  host_tns = (newtime - WLTIMEDELTA) * 100;
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 3/8] hv_util: switch to using timespec64

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
From: Vitaly Kuznetsov <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1676635

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

Signed-off-by: Vitaly Kuznetsov <[hidden email]>
Acked-by: John Stultz <[hidden email]>
Acked-by: Thomas Gleixner <[hidden email]>
Signed-off-by: K. Y. Srinivasan <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
(cherry picked from commit 17244623a4c0f68d3f02c9c74d9b6ae259425826)
Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 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 07a783b52784..ca928b13022a 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -185,7 +185,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);
 
@@ -204,9 +204,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.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 4/8] hv_utils: implement Hyper-V PTP source

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
From: Vitaly Kuznetsov <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1676635

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 dpoll -2 offset 0

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 <[hidden email]>
Signed-off-by: K. Y. Srinivasan <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
(cherry picked from commit 3716a49a81ba19dda7202633a68b28564ba95eb5)
Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/hv_util.c | 179 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 152 insertions(+), 27 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index ca928b13022a..fc4fd1142737 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -27,6 +27,8 @@
 #include <linux/sysctl.h>
 #include <linux/reboot.h>
 #include <linux/hyperv.h>
+#include <linux/clockchips.h>
+#include <linux/ptp_clock_kernel.h>
 #include <asm/mshyperv.h>
 
 #include "hyperv_vmbus.h"
@@ -182,29 +184,15 @@ struct adj_time_work {
 
 static void hv_set_host_time(struct work_struct *work)
 {
- struct adj_time_work *wrk;
- s64 host_tns;
- u64 newtime;
+ struct adj_time_work *wrk;
  struct timespec64 host_ts;
+ u64 reftime, 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;
-
- hv_get_current_tick(current_tick);
- newtime += (current_tick - wrk->ref_time);
- }
- host_tns = (newtime - WLTIMEDELTA) * 100;
- host_ts = ns_to_timespec64(host_tns);
+ reftime = hyperv_cs->read(hyperv_cs);
+ newtime = wrk->host_time + (reftime - wrk->ref_time);
+ host_ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
 
  do_settimeofday64(&host_ts);
 }
@@ -223,22 +211,60 @@ 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;
+ struct system_time_snapshot snap;
+ 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 arre always
  * delivered on the same CPU.
  */
- if (work_pending(&wrk.work))
- return;
-
- wrk.host_time = hosttime;
- wrk.ref_time = reftime;
- wrk.flags = flags;
- if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) != 0) {
+ 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;
+ ktime_get_snapshot(&host_ts.snap);
+
+ /*
+ * 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);
  }
 }
 
@@ -468,14 +494,113 @@ 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)
+{
+ 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);
+
+ 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,
+};
+
+static struct ptp_clock *hv_ptp_clock;
+
 static int hv_timesync_init(struct hv_util_service *srv)
 {
+ /* TimeSync requires Hyper-V clocksource. */
+ if (!hyperv_cs)
+ return -ENODEV;
+
  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.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 5/8] Drivers: hv: util: Fix a typo

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
From: K. Y. Srinivasan <[hidden email]>

Fix a typo.

Signed-off-by: K. Y. Srinivasan <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
(cherry picked from commit bb6a4db92f8345a210b369b791e6920253b10437)
Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/hv_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index fc4fd1142737..45a4f92ecb39 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -231,7 +231,7 @@ static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
 
  /*
  * This check is safe since we are executing in the
- * interrupt context and time synch messages arre always
+ * interrupt context and time synch messages are always
  * delivered on the same CPU.
  */
  if (adj_flags & ICTIMESYNCFLAG_SYNC) {
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 6/8] Drivers: hv: util: don't forget to init host_ts.lock

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
From: Dexuan Cui <[hidden email]>

Without the patch, I always get a "BUG: spinlock bad magic" warning.

Fixes: 3716a49a81ba ("hv_utils: implement Hyper-V PTP source")

Signed-off-by: Dexuan Cui <[hidden email]>
Cc: Vitaly Kuznetsov <[hidden email]>
Cc: "K. Y. Srinivasan" <[hidden email]>
Cc: Haiyang Zhang <[hidden email]>
Cc: Stephen Hemminger <[hidden email]>
Signed-off-by: K. Y. Srinivasan <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
(cherry picked from commit 5a16dfc855127906fcd2935fb039bc8989313915)
Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/hv_util.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 45a4f92ecb39..b2cc46e3dd86 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -580,6 +580,8 @@ static int hv_timesync_init(struct hv_util_service *srv)
  if (!hyperv_cs)
  return -ENODEV;
 
+ spin_lock_init(&host_ts.lock);
+
  INIT_WORK(&wrk.work, hv_set_host_time);
 
  /*
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 7/8] hv_utils: drop .getcrosststamp() support from PTP driver

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
From: Vitaly Kuznetsov <[hidden email]>

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 <[hidden email]>
Signed-off-by: K. Y. Srinivasan <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
(cherry picked from linux-next commit 4f9bac039a64f6306b613a0d90e6b7e75d7ab0c4)
Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/hv_util.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index b2cc46e3dd86..0c18c63f6166 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -220,7 +220,6 @@ static struct adj_time_work  wrk;
 static struct {
  u64 host_time;
  u64 ref_time;
- struct system_time_snapshot snap;
  spinlock_t lock;
 } host_ts;
 
@@ -253,7 +252,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
@@ -528,46 +526,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,
 };
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[xenial][PATCH v2 8/8] hv_utils: fix TimeSync work on pre-TimeSync-v4 hosts

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
From: Vitaly Kuznetsov <[hidden email]>

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 <[hidden email]>
Signed-off-by: K. Y. Srinivasan <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
(cherry picked from linux-next commit 1d10602d306cb7f70545b5e1166efc9409e7d384)
Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/hv_util.c | 128 ++++++++++++++++++++++-----------------------------
 1 file changed, 54 insertions(+), 74 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 0c18c63f6166..8887802f83cf 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -174,27 +174,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);
 }
 
 /*
@@ -210,60 +222,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);
 }
 
 /*
@@ -310,8 +297,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);
  }
  }
 
@@ -514,14 +501,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;
 }
@@ -546,7 +526,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
@@ -567,7 +547,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)
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [xenial][PATCH v2 1/8] UBUNTU: SAUCE: hv: make clocksource available for PTP device supporting

Kleber Souza
In reply to this post by Marcelo Henrique Cerri


On 06/30/17 21:21, Marcelo Henrique Cerri wrote:

> BugLink: http://bugs.launchpad.net/bugs/1676635
>
> Make the MSR and TSC clock sources available during the hv_init() and
> export them via hyperv_cs in a similar way as the following upstream
> commit does:
>
> commit dee863b571b0 ("hv: export current Hyper-V clocksource")
>
> Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 23 -----------------------
>  drivers/hv/hv.c                | 36 +++++++++++++++++++++++++++++++++++-
>  drivers/hv/hyperv_vmbus.h      |  3 +++
>  3 files changed, 38 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 668df428f8ad..8ffd894d4b8e 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -134,26 +134,6 @@ static uint32_t  __init ms_hyperv_platform(void)
>   return 0;
>  }
>  
> -static cycle_t read_hv_clock(struct clocksource *arg)
> -{
> - cycle_t current_tick;
> - /*
> - * Read the partition counter to get the current tick count. This count
> - * is set to 0 when the partition is created and is incremented in
> - * 100 nanosecond units.
> - */
> - rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> - return current_tick;
> -}
> -
> -static struct clocksource hyperv_cs = {
> - .name = "hyperv_clocksource",
> - .rating = 400, /* use this when running on Hyperv*/
> - .read = read_hv_clock,
> - .mask = CLOCKSOURCE_MASK(64),
> - .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> -};
> -
>  static unsigned char hv_get_nmi_reason(void)
>  {
>   return 0;
> @@ -209,9 +189,6 @@ static void __init ms_hyperv_init_platform(void)
>       "hv_nmi_unknown");
>  #endif
>  
> - if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
> - clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
> -
>  #ifdef CONFIG_X86_IO_APIC
>   no_timer_check = 1;
>  #endif
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 176ee7d4c243..92fb168a6633 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -191,6 +191,28 @@ static struct clocksource hyperv_cs_tsc = {
>  };
>  #endif
>  
> +static cycle_t read_hv_clock_msr(struct clocksource *arg)
> +{
> + cycle_t current_tick;
> + /*
> + * Read the partition counter to get the current tick count. This count
> + * is set to 0 when the partition is created and is incremented in
> + * 100 nanosecond units.
> + */
> + rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> + return current_tick;
> +}
> +
> +static struct clocksource hyperv_cs_msr = {
> + .name = "hyperv_clocksource_msr",
> + .rating = 400, /* use this when running on Hyperv*/
> + .read = read_hv_clock_msr,
> + .mask = CLOCKSOURCE_MASK(64),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> +};
> +
> +struct clocksource *hyperv_cs;
> +EXPORT_SYMBOL_GPL(hyperv_cs);
>  
>  /*
>   * hv_init - Main initialization routine.
> @@ -254,9 +276,11 @@ int hv_init(void)
>  
>   va_tsc = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
>   if (!va_tsc)
> - goto cleanup;
> + goto register_msr_cs;
>   hv_context.tsc_page = va_tsc;
>  
> + hyperv_cs = &hyperv_cs_tsc;
> +
>   rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
>  
>   tsc_msr.enable = 1;
> @@ -266,6 +290,16 @@ int hv_init(void)
>   clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
>   }
>  #endif
> + /*
> + * For 32 bit guests just use the MSR based mechanism for reading
> + * the partition counter.
> + */
> +
> +register_msr_cs:
> + hyperv_cs = &hyperv_cs_msr;
> + if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
> + clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100);
> +
>   return 0;

As in commit dee863b571b0 ("hv: export current Hyper-V clocksource")
upstream, the label could be placed inside the ifdef to void the
following warning on archs other than X86_64:

/tmp/kernel-kleber-3902cd9-oxu2/build/drivers/hv/hv.c: In function
'hv_init':
/tmp/kernel-kleber-3902cd9-oxu2/build/drivers/hv/hv.c:298:1: warning:
label 'register_msr_cs' defined but not used [-Wunused-label]
 register_msr_cs:
 ^

>  
>  cleanup:
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 8d7f865c1133..f75729b5aed6 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -29,6 +29,7 @@
>  #include <asm/sync_bitops.h>
>  #include <linux/atomic.h>
>  #include <linux/hyperv.h>
> +#include <linux/clocksource.h>
>  
>  /*
>   * Timeout for services such as KVP and fcopy.
> @@ -719,4 +720,6 @@ enum hvutil_device_state {
>   HVUTIL_DEVICE_DYING,     /* driver unload is in progress */
>  };
>  
> +extern struct clocksource *hyperv_cs;
> +
>  #endif /* _HYPERV_VMBUS_H */
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Acked/cmt] [xenial][PATCH v2 0/8] [Hyper-V] Implement Hyper-V PTP Source

Andy Whitcroft-3
In reply to this post by Marcelo Henrique Cerri
On Fri, Jun 30, 2017 at 04:21:26PM -0300, Marcelo Henrique Cerri wrote:

> BugLink: http://bugs.launchpad.net/bugs/1676635
>
> Adding a few more patches that fix additional issues introduced by the
> original patch ("hv_utils: implement Hyper-V PTP source").
>
> v1:
>
> The requested commit requires more than 50 additional commits in order
> to be applied cleanly. To avoid that, I cherry-picked just the strict
> necessary commits and adapted to the Xenial kernel the upstream change
> that makes the Hyper-V clock souce available via hyperv_cs.
>
> Dexuan Cui (1):
>   Drivers: hv: util: don't forget to init host_ts.lock
>
> K. Y. Srinivasan (2):
>   Drivers: hv: util: Use hv_get_current_tick() to get current tick
>   Drivers: hv: util: Fix a typo
>
> Marcelo Henrique Cerri (1):
>   UBUNTU: SAUCE: hv: make clocksource available for PTP device
>     supporting
>
> Vitaly Kuznetsov (4):
>   hv_util: switch to using timespec64
>   hv_utils: implement Hyper-V PTP source
>   hv_utils: drop .getcrosststamp() support from PTP driver
>   hv_utils: fix TimeSync work on pre-TimeSync-v4 hosts
>
>  arch/x86/kernel/cpu/mshyperv.c |  23 ------
>  drivers/hv/hv.c                |  36 ++++++++-
>  drivers/hv/hv_util.c           | 168 +++++++++++++++++++++++++++++------------
>  drivers/hv/hyperv_vmbus.h      |   3 +
>  4 files changed, 158 insertions(+), 72 deletions(-)

Well thankfully they are all self-contains to HV specific code.  The
stack seems fairly small and benigh considering.  I feel this is going
to put a burden on userspace to start using NTP.  Right?  I assume
someone is sorting that out in userspace.

Anyhow.  Overall it looks fine.  Assuming you do somethign sensible eith
Klebers query:

Acked-by: Andy Whitcroft <[hidden email]>

-apw

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Acked/cmt] [xenial][PATCH v2 0/8] [Hyper-V] Implement Hyper-V PTP Source

Joshua R. Poulson
Userspace has to pick a PTP source, but we've generally recommended
using NTP and had it on by default in cloud images for years.

Thanks, --jrp

On Mon, Jul 3, 2017 at 5:18 AM, Andy Whitcroft <[hidden email]> wrote:

> On Fri, Jun 30, 2017 at 04:21:26PM -0300, Marcelo Henrique Cerri wrote:
>> BugLink: http://bugs.launchpad.net/bugs/1676635
>>
>> Adding a few more patches that fix additional issues introduced by the
>> original patch ("hv_utils: implement Hyper-V PTP source").
>>
>> v1:
>>
>> The requested commit requires more than 50 additional commits in order
>> to be applied cleanly. To avoid that, I cherry-picked just the strict
>> necessary commits and adapted to the Xenial kernel the upstream change
>> that makes the Hyper-V clock souce available via hyperv_cs.
>>
>> Dexuan Cui (1):
>>   Drivers: hv: util: don't forget to init host_ts.lock
>>
>> K. Y. Srinivasan (2):
>>   Drivers: hv: util: Use hv_get_current_tick() to get current tick
>>   Drivers: hv: util: Fix a typo
>>
>> Marcelo Henrique Cerri (1):
>>   UBUNTU: SAUCE: hv: make clocksource available for PTP device
>>     supporting
>>
>> Vitaly Kuznetsov (4):
>>   hv_util: switch to using timespec64
>>   hv_utils: implement Hyper-V PTP source
>>   hv_utils: drop .getcrosststamp() support from PTP driver
>>   hv_utils: fix TimeSync work on pre-TimeSync-v4 hosts
>>
>>  arch/x86/kernel/cpu/mshyperv.c |  23 ------
>>  drivers/hv/hv.c                |  36 ++++++++-
>>  drivers/hv/hv_util.c           | 168 +++++++++++++++++++++++++++++------------
>>  drivers/hv/hyperv_vmbus.h      |   3 +
>>  4 files changed, 158 insertions(+), 72 deletions(-)
>
> Well thankfully they are all self-contains to HV specific code.  The
> stack seems fairly small and benigh considering.  I feel this is going
> to put a burden on userspace to start using NTP.  Right?  I assume
> someone is sorting that out in userspace.
>
> Anyhow.  Overall it looks fine.  Assuming you do somethign sensible eith
> Klebers query:
>
> Acked-by: Andy Whitcroft <[hidden email]>
>
> -apw
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [xenial][PATCH v2 1/8] UBUNTU: SAUCE: hv: make clocksource available for PTP device supporting

Marcelo Henrique Cerri
In reply to this post by Kleber Souza
Good catch. I will prepare a v3 with that fixed.

--
Regards,
Marcelo

On Mon, Jul 03, 2017 at 12:29:47PM +0200, Kleber Souza wrote:

>
>
> On 06/30/17 21:21, Marcelo Henrique Cerri wrote:
> > BugLink: http://bugs.launchpad.net/bugs/1676635
> >
> > Make the MSR and TSC clock sources available during the hv_init() and
> > export them via hyperv_cs in a similar way as the following upstream
> > commit does:
> >
> > commit dee863b571b0 ("hv: export current Hyper-V clocksource")
> >
> > Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
> > ---
> >  arch/x86/kernel/cpu/mshyperv.c | 23 -----------------------
> >  drivers/hv/hv.c                | 36 +++++++++++++++++++++++++++++++++++-
> >  drivers/hv/hyperv_vmbus.h      |  3 +++
> >  3 files changed, 38 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > index 668df428f8ad..8ffd894d4b8e 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -134,26 +134,6 @@ static uint32_t  __init ms_hyperv_platform(void)
> >   return 0;
> >  }
> >  
> > -static cycle_t read_hv_clock(struct clocksource *arg)
> > -{
> > - cycle_t current_tick;
> > - /*
> > - * Read the partition counter to get the current tick count. This count
> > - * is set to 0 when the partition is created and is incremented in
> > - * 100 nanosecond units.
> > - */
> > - rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> > - return current_tick;
> > -}
> > -
> > -static struct clocksource hyperv_cs = {
> > - .name = "hyperv_clocksource",
> > - .rating = 400, /* use this when running on Hyperv*/
> > - .read = read_hv_clock,
> > - .mask = CLOCKSOURCE_MASK(64),
> > - .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> > -};
> > -
> >  static unsigned char hv_get_nmi_reason(void)
> >  {
> >   return 0;
> > @@ -209,9 +189,6 @@ static void __init ms_hyperv_init_platform(void)
> >       "hv_nmi_unknown");
> >  #endif
> >  
> > - if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
> > - clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
> > -
> >  #ifdef CONFIG_X86_IO_APIC
> >   no_timer_check = 1;
> >  #endif
> > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > index 176ee7d4c243..92fb168a6633 100644
> > --- a/drivers/hv/hv.c
> > +++ b/drivers/hv/hv.c
> > @@ -191,6 +191,28 @@ static struct clocksource hyperv_cs_tsc = {
> >  };
> >  #endif
> >  
> > +static cycle_t read_hv_clock_msr(struct clocksource *arg)
> > +{
> > + cycle_t current_tick;
> > + /*
> > + * Read the partition counter to get the current tick count. This count
> > + * is set to 0 when the partition is created and is incremented in
> > + * 100 nanosecond units.
> > + */
> > + rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> > + return current_tick;
> > +}
> > +
> > +static struct clocksource hyperv_cs_msr = {
> > + .name = "hyperv_clocksource_msr",
> > + .rating = 400, /* use this when running on Hyperv*/
> > + .read = read_hv_clock_msr,
> > + .mask = CLOCKSOURCE_MASK(64),
> > + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> > +};
> > +
> > +struct clocksource *hyperv_cs;
> > +EXPORT_SYMBOL_GPL(hyperv_cs);
> >  
> >  /*
> >   * hv_init - Main initialization routine.
> > @@ -254,9 +276,11 @@ int hv_init(void)
> >  
> >   va_tsc = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
> >   if (!va_tsc)
> > - goto cleanup;
> > + goto register_msr_cs;
> >   hv_context.tsc_page = va_tsc;
> >  
> > + hyperv_cs = &hyperv_cs_tsc;
> > +
> >   rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
> >  
> >   tsc_msr.enable = 1;
> > @@ -266,6 +290,16 @@ int hv_init(void)
> >   clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
> >   }
> >  #endif
> > + /*
> > + * For 32 bit guests just use the MSR based mechanism for reading
> > + * the partition counter.
> > + */
> > +
> > +register_msr_cs:
> > + hyperv_cs = &hyperv_cs_msr;
> > + if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
> > + clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100);
> > +
> >   return 0;
>
> As in commit dee863b571b0 ("hv: export current Hyper-V clocksource")
> upstream, the label could be placed inside the ifdef to void the
> following warning on archs other than X86_64:
>
> /tmp/kernel-kleber-3902cd9-oxu2/build/drivers/hv/hv.c: In function
> 'hv_init':
> /tmp/kernel-kleber-3902cd9-oxu2/build/drivers/hv/hv.c:298:1: warning:
> label 'register_msr_cs' defined but not used [-Wunused-label]
>  register_msr_cs:
>  ^
>
> >  
> >  cleanup:
> > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> > index 8d7f865c1133..f75729b5aed6 100644
> > --- a/drivers/hv/hyperv_vmbus.h
> > +++ b/drivers/hv/hyperv_vmbus.h
> > @@ -29,6 +29,7 @@
> >  #include <asm/sync_bitops.h>
> >  #include <linux/atomic.h>
> >  #include <linux/hyperv.h>
> > +#include <linux/clocksource.h>
> >  
> >  /*
> >   * Timeout for services such as KVP and fcopy.
> > @@ -719,4 +720,6 @@ enum hvutil_device_state {
> >   HVUTIL_DEVICE_DYING,     /* driver unload is in progress */
> >  };
> >  
> > +extern struct clocksource *hyperv_cs;
> > +
> >  #endif /* _HYPERV_VMBUS_H */
> >
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Acked/cmt] [xenial][PATCH v2 0/8] [Hyper-V] Implement Hyper-V PTP Source

Marcelo Henrique Cerri
In reply to this post by Andy Whitcroft-3
On Mon, Jul 03, 2017 at 01:18:31PM +0100, Andy Whitcroft wrote:
> Well thankfully they are all self-contains to HV specific code.  The
> stack seems fairly small and benigh considering.  I feel this is going
> to put a burden on userspace to start using NTP.  Right?  I assume
> someone is sorting that out in userspace.
>

On userspace side we still have some systemd issues to solve. I will
make sure somebody is taking care of that.

> Anyhow.  Overall it looks fine.  Assuming you do somethign sensible eith
> Klebers query:
>
> Acked-by: Andy Whitcroft <[hidden email]>
>
> -apw

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [xenial][PATCH v2 0/8] [Hyper-V] Implement Hyper-V PTP Source

brad.figg
In reply to this post by Marcelo Henrique Cerri
Loading...