Subject: Updating Xen's time-of-day clock
To: None <port-xen@NetBSD.org>
From: Jed Davis <jdev@panix.com>
List: port-xen
Date: 05/04/2006 19:29:08
--=-=-=

Attached is a patch to have the NetBSD dom0 periodically reset Xen's
time-of-day clock; there's a sysctl to change the interval or disable
the functionality entirely.  More eyes/opinions are desired (also
about the sysctl name; "machdep.xen_timepush_ticks" is a little
unwieldy).

Currently, we set Xen's time only on settimeofday(2); so, if the time
is being disciplined by ntpd or by periodic runs of ntpdate that use
adjtime(2), then Xen's clock will continue to drift.  This is bad
because domU use that value to set their clocks on boot (and resume),
and Linux in particular defaults to tying its local clock to the
hypervisor's.

The Linux dom0 does this every 60 seconds (which, for usual values of
drift, will keep the step size under a tick); out of probably
excessive paranoia that that might line up with a cron job or
something and cause strange non-reproducible problems, I used a less
round number.

-- 
(let ((C call-with-current-continuation)) (apply (lambda (x y) (x y)) (map
((lambda (r) ((C C) (lambda (s) (r (lambda l (apply (s s) l))))))  (lambda
(f) (lambda (l) (if (null? l) C (lambda (k) (display (car l)) ((f (cdr l))
(C k)))))))    '((#\J #\d #\D #\v #\s) (#\e #\space #\a #\i #\newline)))))

--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=xen-timepush.diff
Content-Description: Periodically update Xen's time-of-day clock

Index: xen/clock.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/clock.c,v
retrieving revision 1.22
diff -u -p -r1.22 clock.c
--- xen/clock.c	28 Apr 2006 02:30:42 -0000	1.22
+++ xen/clock.c	4 May 2006 23:14:18 -0000
@@ -41,6 +41,7 @@ __KERNEL_RCSID(0, "$NetBSD: clock.c,v 1.
 #include <sys/time.h>
 #include <sys/kernel.h>
 #include <sys/device.h>
+#include <sys/sysctl.h>
 
 #include <machine/xen.h>
 #include <machine/hypervisor.h>
@@ -69,6 +70,12 @@ static int timeset;
 
 static uint64_t processed_system_time;
 
+#ifdef DOM0OPS
+/* If we're dom0, send our time to Xen every minute or so. */
+int xen_timepush_ticks = 0;
+static struct callout xen_timepush_co = CALLOUT_INITIALIZER;
+#endif
+
 #define NS_PER_TICK (1000000000ULL/hz)
 
 /*
@@ -171,15 +178,15 @@ fstime:
 	printf("WARNING: CHECK AND RESET THE DATE!\n");
 }
 
-void
-resettodr()
+static void
+resettodr_i(void)
 {
 #ifdef DOM0OPS
 	dom0_op_t op;
 	int s;
 #endif
 #ifdef DEBUG_CLOCK
-	struct clock_ymdhms dt;
+	struct timeval sent_delta;
 #endif
 
 	/*
@@ -190,10 +197,19 @@ resettodr()
 		return;
 
 #ifdef DEBUG_CLOCK
-	clock_secs_to_ymdhms(time.tv_sec - rtc_offset * 60, &dt);
-
-	printf("setclock: %d/%d/%d %02d:%02d:%02d\n", dt.dt_year,
-	    dt.dt_mon, dt.dt_day, dt.dt_hour, dt.dt_min, dt.dt_sec);
+        {
+		char pm;
+ 
+		if (timercmp(&time, &shadow_tv, >)) {
+			timersub(&time, &shadow_tv, &sent_delta);
+			pm = '+';
+		} else {
+			timersub(&shadow_tv, &time, &sent_delta);
+			pm = '-';
+		}
+		printf("resettodr: stepping Xen clock by %c%ld.%06ld\n",
+	    pm, sent_delta.tv_sec, sent_delta.tv_usec);
+	}
 #endif
 #ifdef DOM0OPS
 	if (xen_start_info.flags & SIF_PRIVILEGED) {
@@ -214,6 +230,22 @@ resettodr()
 #endif
 }
 
+/*
+ * When the clock is administratively set, in addition to resetting
+ * Xen's clock if possible, we should also allow xen_microtime to 
+ * step backwards without complaint.
+ */
+void
+resettodr()
+{
+	CPU_INFO_ITERATOR cii;
+	struct cpu_info *ci;
+
+	resettodr_i();
+	for (CPU_INFO_FOREACH(cii, ci))
+		timerclear(&ci->ci_cc.cc_time);
+}
+
 void
 startrtclock()
 {
@@ -301,11 +333,12 @@ xen_microtime(struct timeval *tv)
 		tv->tv_sec++;
 		tv->tv_usec -= 1000000;
 	}
-	/* Avoid backsteps, e.g. at the beginning of a negative adjustment. */
+	/* Avoid small backsteps, e.g. at the beginning of a negative adjustment. */
 	if (timerisset(&ci->ci_cc.cc_time) &&	
 	    timercmp(tv, &ci->ci_cc.cc_time, <)) {
-		struct timeval backstep; /* XXX hook resettodr() instead of doing this. */
+		struct timeval backstep;
 
+		/* XXXjld: not sure if this check can be safely removed now */
 		timersub(&ci->ci_cc.cc_time, tv, &backstep);
 		if (backstep.tv_sec == 0) { /* if it was < 1sec */
 			*tv = ci->ci_cc.cc_time;
@@ -323,11 +356,52 @@ xen_microtime(struct timeval *tv)
 	splx(s);
 }
 
+#ifdef DOM0OPS
+/* ARGSUSED */
+static void
+xen_timepush(void *arg)
+{
+	struct callout *co = arg;
+
+	resettodr_i();
+	if (xen_timepush_ticks > 0)
+		callout_schedule(co, xen_timepush_ticks);
+}
+
+/* ARGSUSED */
+static int
+sysctl_xen_timepush(SYSCTLFN_ARGS)
+{
+	int error, new_ticks;
+	struct sysctlnode node;
+
+	new_ticks = xen_timepush_ticks;
+	node = *rnode;
+	node.sysctl_data = &new_ticks;
+	error = sysctl_lookup(SYSCTLFN_CALL(&node));
+	if (error || newp == NULL)
+		return error;
+
+	if (new_ticks < 0)
+		return EINVAL;
+	if (new_ticks != xen_timepush_ticks) {
+		xen_timepush_ticks = new_ticks;
+		if (new_ticks > 0)
+			callout_schedule(&xen_timepush_co, new_ticks);
+		else
+			callout_stop(&xen_timepush_co);
+	}
+
+	return 0;
+}
+#endif
+
 void
 xen_initclocks()
 {
-	int evtch = bind_virq_to_evtch(VIRQ_TIMER);
+	int evtch;
 
+	evtch = bind_virq_to_evtch(VIRQ_TIMER);
 	aprint_verbose("Xen clock: using event channel %d\n", evtch);
 
 	get_time_values_from_xen();
@@ -336,6 +410,19 @@ xen_initclocks()
 	event_set_handler(evtch, (int (*)(void *))xen_timer_handler,
 	    NULL, IPL_CLOCK, "clock");
 	hypervisor_enable_event(evtch);
+
+#ifdef DOM0OPS
+	xen_timepush_ticks = 53 * hz + 3; /* avoid exact # of min/sec */
+	if (xen_start_info.flags & SIF_PRIVILEGED) {
+		sysctl_createv(NULL, 0, NULL, NULL, CTLFLAG_READWRITE,
+		    CTLTYPE_INT, "xen_timepush_ticks", SYSCTL_DESCR("How often"
+		    " to update the hypervisor's time-of-day; 0 to disable"),
+		    sysctl_xen_timepush, 0, &xen_timepush_ticks, 0, 
+		    CTL_MACHDEP, CTL_CREATE, CTL_EOL);
+		callout_reset(&xen_timepush_co, xen_timepush_ticks,
+		    &xen_timepush, &xen_timepush_co);
+	}
+#endif
 }
 
 static int

--=-=-=--