Module Name:    src
Committed By:   riastradh
Date:           Sat Jun 30 14:59:38 UTC 2018

Modified Files:
        src/sys/arch/xen/xen: clock.c

Log Message:
Rearm the Xen timer on resume.

This just moves the timer-arming logic from xen_initclocks into
xen_resumeclocks so that it runs on resume too.

I hypothesize that this is necessary for Xen to resume.  Otherwise,
how could the one-shot timer possibly by rearmed?  On the other hand,
it is conceivable that something automatically rearms it.

This also reorders the initialization so that we establish a timer
interrupt handler first, and _then_ arm the timer.  If the order
matters, it is hard to imagine that the other way is correct:
conceivably, the interrupt could arrive before we've established the
handler, and then there's nothing to rearm it.

Whether this is _sufficient_ for Xen to resume, I don't know.
Symptoms recently reported in
<https://mail-index.netbsd.org/port-xen/2018/06/15/msg009207.html>
look different from how I would expect this to manifest, which is as
a system wedged because there's no no hardclock activity.

ok cherry


To generate a diff of this commit:
cvs rdiff -u -r1.69 -r1.70 src/sys/arch/xen/xen/clock.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/arch/xen/xen/clock.c
diff -u src/sys/arch/xen/xen/clock.c:1.69 src/sys/arch/xen/xen/clock.c:1.70
--- src/sys/arch/xen/xen/clock.c:1.69	Sat Jun 30 14:21:19 2018
+++ src/sys/arch/xen/xen/clock.c	Sat Jun 30 14:59:38 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: clock.c,v 1.69 2018/06/30 14:21:19 riastradh Exp $	*/
+/*	$NetBSD: clock.c,v 1.70 2018/06/30 14:59:38 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2017, 2018 The NetBSD Foundation, Inc.
@@ -36,7 +36,7 @@
 #endif
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: clock.c,v 1.69 2018/06/30 14:21:19 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: clock.c,v 1.70 2018/06/30 14:59:38 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -740,8 +740,8 @@ xen_suspendclocks(struct cpu_info *ci)
 /*
  * xen_resumeclocks(ci)
  *
- *	Start handling the Xen timer event on the CPU of ci.  Caller
- *	must be running on and bound to ci's CPU.
+ *	Start handling the Xen timer event on the CPU of ci.  Arm the
+ *	Xen timer.  Caller must be running on and bound to ci's CPU.
  *
  *	Actually, caller must have kpreemption disabled, because that's
  *	easier to assert at the moment.
@@ -751,6 +751,7 @@ xen_resumeclocks(struct cpu_info *ci)
 {
 	char intr_xname[INTRDEVNAMEBUF];
 	int evtch;
+	int error;
 
 	KASSERT(ci == curcpu());
 	KASSERT(kpreempt_disabled());
@@ -772,6 +773,21 @@ xen_resumeclocks(struct cpu_info *ci)
 
 	aprint_verbose("Xen %s: using event channel %d\n", intr_xname, evtch);
 
+	/* Disarm the periodic timer on Xen>=3.1 which is allegedly buggy.  */
+	if (XEN_MAJOR(xen_version) > 3 || XEN_MINOR(xen_version) > 0) {
+		error = HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer,
+		    ci->ci_cpuid, NULL);
+		KASSERT(error == 0);
+	}
+
+	/* Pretend the last hardclock happened right now.  */
+	ci->ci_xen_hardclock_systime_ns = xen_vcputime_systime_ns();
+
+	/* Arm the one-shot timer.  */
+	error = HYPERVISOR_set_timer_op(ci->ci_xen_hardclock_systime_ns +
+	    NS_PER_TICK);
+	KASSERT(error == 0);
+
 	/* We'd better not have switched CPUs.  */
 	KASSERT(ci == curcpu());
 }
@@ -848,7 +864,6 @@ void
 xen_initclocks(void)
 {
 	struct cpu_info *ci = curcpu();
-	int error;
 
 	/* If this is the primary CPU, do global initialization first.  */
 	if (ci == &cpu_info_primary) {
@@ -865,9 +880,6 @@ xen_initclocks(void)
 #endif
 	}
 
-	/* Pretend the last hardclock happened right now.  */
-	ci->ci_xen_hardclock_systime_ns = xen_vcputime_systime_ns();
-
 	/* Attach the event counters.  */
 	evcnt_attach_dynamic(&ci->ci_xen_cpu_tsc_backwards_evcnt,
 	    EVCNT_TYPE_INTR, NULL, device_xname(ci->ci_dev),
@@ -888,18 +900,6 @@ xen_initclocks(void)
 	    EVCNT_TYPE_INTR, NULL, device_xname(ci->ci_dev),
 	    "missed hardclock");
 
-	/* Disarm the periodic timer on Xen>=3.1 which is allegedly buggy.  */
-	if (XEN_MAJOR(xen_version) > 3 || XEN_MINOR(xen_version) > 0) {
-		error = HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer,
-		    ci->ci_cpuid, NULL);
-		KASSERT(error == 0);
-	}
-
-	/* Arm the timer.  */
-	error = HYPERVISOR_set_timer_op(ci->ci_xen_hardclock_systime_ns +
-	    NS_PER_TICK);
-	KASSERT(error == 0);
-
 	/* Fire up the clocks.  */
 	xen_resumeclocks(ci);
 }

Reply via email to