| Message ID | 1446743385-12848-3-git-send-email-stefano.stabellini@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/11/15 17:09, Stefano Stabellini wrote: > If Linux is running as dom0, call XENPF_settime to update the system > time in Xen on pvclock_gtod notifications. Isn't this a cut-and-paste from x86? Can you make it common? David
On Thursday 05 November 2015 17:09:45 Stefano Stabellini wrote: > If Linux is running as dom0, call XENPF_settime to update the system > time in Xen on pvclock_gtod notifications. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > arch/arm/xen/enlighten.c | 52 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 51 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index b6aea9c..0176db0 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -28,6 +28,7 @@ > #include <linux/cpufreq.h> > #include <linux/cpu.h> > #include <linux/console.h> > +#include <linux/pvclock_gtod.h> > #include <linux/timekeeping.h> > #include <clocksource/arm_arch_timer.h> > > @@ -123,6 +124,50 @@ static void xen_read_wallclock(struct timespec *ts) > set_normalized_timespec(ts, now.tv_sec, now.tv_nsec); > } > > +static int xen_pvclock_gtod_notify(struct notifier_block *nb, > + unsigned long was_set, void *priv) > +{ > + /* Protected by the calling core code serialization */ > + static struct timespec next_sync; > + > + struct xen_platform_op op; > + struct timespec now; Again, use timespec64 > + now = __current_kernel_time(); We don't have __current_kernel_time64() yet, but it is trivial to add, just follow the example of current_kernel_time()/current_kernel_time64() and convert the existing __current_kernel_time() function into a static inline wrapper for the new __current_kernel_time64(). > + /* > + * We only take the expensive HV call when the clock was set > + * or when the 11 minutes RTC synchronization time elapsed. > + */ > + if (!was_set && timespec_compare(&now, &next_sync) < 0) > + return NOTIFY_OK; > + > + op.interface_version = XENPF_INTERFACE_VERSION; > + op.cmd = XENPF_settime; > + op.u.settime.secs = now.tv_sec; > + op.u.settime.nsecs = now.tv_nsec; > + op.u.settime.system_time = arch_timer_read_counter(); > + printk("GTOD: Setting to %ld.%ld at %lld\n", > + (long)op.u.settime.secs, > + (long)op.u.settime.nsecs, > + (long long)op.u.settime.system_time); > + (void)HYPERVISOR_dom0_op(&op); I guess we will also need a XENPF_settime64 interface, but at least we can get away with implementing only that one on ARM, while x86 will have to support both the 32-bit and 64-bit based variant. Arnd
On Thu, 5 Nov 2015, David Vrabel wrote: > On 05/11/15 17:09, Stefano Stabellini wrote: > > If Linux is running as dom0, call XENPF_settime to update the system > > time in Xen on pvclock_gtod notifications. > > Isn't this a cut-and-paste from x86? Can you make it common? Not exactly, for the way system_time is set.
On Thu, 5 Nov 2015, Arnd Bergmann wrote: > On Thursday 05 November 2015 17:09:45 Stefano Stabellini wrote: > > If Linux is running as dom0, call XENPF_settime to update the system > > time in Xen on pvclock_gtod notifications. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > --- > > arch/arm/xen/enlighten.c | 52 +++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 51 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index b6aea9c..0176db0 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -28,6 +28,7 @@ > > #include <linux/cpufreq.h> > > #include <linux/cpu.h> > > #include <linux/console.h> > > +#include <linux/pvclock_gtod.h> > > #include <linux/timekeeping.h> > > #include <clocksource/arm_arch_timer.h> > > > > @@ -123,6 +124,50 @@ static void xen_read_wallclock(struct timespec *ts) > > set_normalized_timespec(ts, now.tv_sec, now.tv_nsec); > > } > > > > +static int xen_pvclock_gtod_notify(struct notifier_block *nb, > > + unsigned long was_set, void *priv) > > +{ > > + /* Protected by the calling core code serialization */ > > + static struct timespec next_sync; > > + > > + struct xen_platform_op op; > > + struct timespec now; > > Again, use timespec64 OK > > + now = __current_kernel_time(); > > We don't have __current_kernel_time64() yet, but it is trivial > to add, just follow the example of > current_kernel_time()/current_kernel_time64() and convert the > existing __current_kernel_time() function into a static > inline wrapper for the new __current_kernel_time64(). All right. I guess something like: struct timespec64 __current_kernel_time64(void) { struct timekeeper *tk = &tk_core.timekeeper; return tk_xtime(tk); } > > + /* > > + * We only take the expensive HV call when the clock was set > > + * or when the 11 minutes RTC synchronization time elapsed. > > + */ > > + if (!was_set && timespec_compare(&now, &next_sync) < 0) > > + return NOTIFY_OK; > > + > > + op.interface_version = XENPF_INTERFACE_VERSION; > > + op.cmd = XENPF_settime; > > + op.u.settime.secs = now.tv_sec; > > + op.u.settime.nsecs = now.tv_nsec; > > + op.u.settime.system_time = arch_timer_read_counter(); > > + printk("GTOD: Setting to %ld.%ld at %lld\n", > > + (long)op.u.settime.secs, > > + (long)op.u.settime.nsecs, > > + (long long)op.u.settime.system_time); > > + (void)HYPERVISOR_dom0_op(&op); > > I guess we will also need a XENPF_settime64 interface, but at > least we can get away with implementing only that one on ARM, > while x86 will have to support both the 32-bit and 64-bit > based variant. We already have XENPF_settime64, I'll just use it instead.
On Monday 09 November 2015 14:10:22 Stefano Stabellini wrote: > On Thu, 5 Nov 2015, Arnd Bergmann wrote: > > On Thursday 05 November 2015 17:09:45 Stefano Stabellini wrote: > > > + now = __current_kernel_time(); > > > > We don't have __current_kernel_time64() yet, but it is trivial > > to add, just follow the example of > > current_kernel_time()/current_kernel_time64() and convert the > > existing __current_kernel_time() function into a static > > inline wrapper for the new __current_kernel_time64(). > > All right. I guess something like: > > struct timespec64 __current_kernel_time64(void) > { > struct timekeeper *tk = &tk_core.timekeeper; > > return tk_xtime(tk); > } Yes, exactly. Just to make sure that this is actually the correct interface that you want to call: __current_kernel_time{,64}() is the fastest interface we have to get an approximation of the current time, while ignoring all of the locking. Is is possible that you instead want ktime_get_real_ts64(), which gives you the time as precise as the kernel knows it, but uses locking? > > > + /* > > > + * We only take the expensive HV call when the clock was set > > > + * or when the 11 minutes RTC synchronization time elapsed. > > > + */ > > > + if (!was_set && timespec_compare(&now, &next_sync) < 0) > > > + return NOTIFY_OK; > > > + > > > + op.interface_version = XENPF_INTERFACE_VERSION; > > > + op.cmd = XENPF_settime; > > > + op.u.settime.secs = now.tv_sec; > > > + op.u.settime.nsecs = now.tv_nsec; > > > + op.u.settime.system_time = arch_timer_read_counter(); > > > + printk("GTOD: Setting to %ld.%ld at %lld\n", > > > + (long)op.u.settime.secs, > > > + (long)op.u.settime.nsecs, > > > + (long long)op.u.settime.system_time); > > > + (void)HYPERVISOR_dom0_op(&op); > > > > I guess we will also need a XENPF_settime64 interface, but at > > least we can get away with implementing only that one on ARM, > > while x86 will have to support both the 32-bit and 64-bit > > based variant. > > We already have XENPF_settime64, I'll just use it instead. Ok, great! Then we just need to find a volunteer who can do the same thing on x86, with the fallback to XENPF_settime that they need for older hosts. Arnd
On Mon, 9 Nov 2015, Arnd Bergmann wrote: > On Monday 09 November 2015 14:10:22 Stefano Stabellini wrote: > > On Thu, 5 Nov 2015, Arnd Bergmann wrote: > > > On Thursday 05 November 2015 17:09:45 Stefano Stabellini wrote: > > > > + now = __current_kernel_time(); > > > > > > We don't have __current_kernel_time64() yet, but it is trivial > > > to add, just follow the example of > > > current_kernel_time()/current_kernel_time64() and convert the > > > existing __current_kernel_time() function into a static > > > inline wrapper for the new __current_kernel_time64(). > > > > All right. I guess something like: > > > > struct timespec64 __current_kernel_time64(void) > > { > > struct timekeeper *tk = &tk_core.timekeeper; > > > > return tk_xtime(tk); > > } > > Yes, exactly. > > Just to make sure that this is actually the correct interface > that you want to call: > > __current_kernel_time{,64}() is the fastest interface we have > to get an approximation of the current time, while ignoring > all of the locking. > > Is is possible that you instead want ktime_get_real_ts64(), > which gives you the time as precise as the kernel knows it, > but uses locking? I am not 100% sure. Can I call ktime_get_real_ts64 from a pvclock_gtod notifier_call function?
On Mon, 9 Nov 2015, Stefano Stabellini wrote: > On Mon, 9 Nov 2015, Arnd Bergmann wrote: > > On Monday 09 November 2015 14:10:22 Stefano Stabellini wrote: > > > On Thu, 5 Nov 2015, Arnd Bergmann wrote: > > > > On Thursday 05 November 2015 17:09:45 Stefano Stabellini wrote: > > > > > + now = __current_kernel_time(); > > > > > > > > We don't have __current_kernel_time64() yet, but it is trivial > > > > to add, just follow the example of > > > > current_kernel_time()/current_kernel_time64() and convert the > > > > existing __current_kernel_time() function into a static > > > > inline wrapper for the new __current_kernel_time64(). > > > > > > All right. I guess something like: > > > > > > struct timespec64 __current_kernel_time64(void) > > > { > > > struct timekeeper *tk = &tk_core.timekeeper; > > > > > > return tk_xtime(tk); > > > } > > > > Yes, exactly. > > > > Just to make sure that this is actually the correct interface > > that you want to call: > > > > __current_kernel_time{,64}() is the fastest interface we have > > to get an approximation of the current time, while ignoring > > all of the locking. > > > > Is is possible that you instead want ktime_get_real_ts64(), > > which gives you the time as precise as the kernel knows it, > > but uses locking? > > I am not 100% sure. Can I call ktime_get_real_ts64 from a > pvclock_gtod notifier_call function? It doesn't look like it: we would be getting the tk_core seqcount twice.
On Monday 09 November 2015 17:42:50 Stefano Stabellini wrote: > On Mon, 9 Nov 2015, Stefano Stabellini wrote: > > On Mon, 9 Nov 2015, Arnd Bergmann wrote: > > > On Monday 09 November 2015 14:10:22 Stefano Stabellini wrote: > > > > > > Just to make sure that this is actually the correct interface > > > that you want to call: > > > > > > __current_kernel_time{,64}() is the fastest interface we have > > > to get an approximation of the current time, while ignoring > > > all of the locking. > > > > > > Is is possible that you instead want ktime_get_real_ts64(), > > > which gives you the time as precise as the kernel knows it, > > > but uses locking? > > > > I am not 100% sure. Can I call ktime_get_real_ts64 from a > > pvclock_gtod notifier_call function? > > It doesn't look like it: we would be getting the tk_core seqcount twice. Ok, I see. I'm still unsure what this is all good for, but at least this call appears to be safe. Arnd
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index b6aea9c..0176db0 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -28,6 +28,7 @@ #include <linux/cpufreq.h> #include <linux/cpu.h> #include <linux/console.h> +#include <linux/pvclock_gtod.h> #include <linux/timekeeping.h> #include <clocksource/arm_arch_timer.h> @@ -123,6 +124,50 @@ static void xen_read_wallclock(struct timespec *ts) set_normalized_timespec(ts, now.tv_sec, now.tv_nsec); } +static int xen_pvclock_gtod_notify(struct notifier_block *nb, + unsigned long was_set, void *priv) +{ + /* Protected by the calling core code serialization */ + static struct timespec next_sync; + + struct xen_platform_op op; + struct timespec now; + + now = __current_kernel_time(); + + /* + * We only take the expensive HV call when the clock was set + * or when the 11 minutes RTC synchronization time elapsed. + */ + if (!was_set && timespec_compare(&now, &next_sync) < 0) + return NOTIFY_OK; + + op.interface_version = XENPF_INTERFACE_VERSION; + op.cmd = XENPF_settime; + op.u.settime.secs = now.tv_sec; + op.u.settime.nsecs = now.tv_nsec; + op.u.settime.system_time = arch_timer_read_counter(); + printk("GTOD: Setting to %ld.%ld at %lld\n", + (long)op.u.settime.secs, + (long)op.u.settime.nsecs, + (long long)op.u.settime.system_time); + (void)HYPERVISOR_dom0_op(&op); + + /* + * Move the next drift compensation time 11 minutes + * ahead. That's emulating the sync_cmos_clock() update for + * the hardware RTC. + */ + next_sync = now; + next_sync.tv_sec += 11 * 60; + + return NOTIFY_OK; +} + +static struct notifier_block xen_pvclock_gtod_notifier = { + .notifier_call = xen_pvclock_gtod_notify, +}; + static void xen_percpu_init(void) { struct vcpu_register_vcpu_info info; @@ -321,7 +366,12 @@ static int __init xen_guest_init(void) pv_time_ops.steal_clock = xen_stolen_accounting; static_key_slow_inc(¶virt_steal_enabled); xen_read_wallclock(&ts); - do_settimeofday(&ts); + if (xen_initial_domain()) + pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier); + else { + xen_read_wallclock(&ts); + do_settimeofday(&ts); + } return 0; }