improve timer handling

+ Previous code assumed a timer would only trigger when requested, but
  didn't explicitly disable timers during init. Newer versions of qemu
  (>= 10?) seem to have some kind of timer set, unsure if that's via
  OpenSBI or what exactly but some slower tests were failing because of
  it.

3 files changed, 47 insertions, 2 deletions

diff --git a/arch/riscv64/kernel/timer.c b/arch/riscv64/kernel/timer.c
index c7752f8..e9e509d 100644
--- a/arch/riscv64/kernel/timer.c
+++ b/arch/riscv64/kernel/timer.c
@@ -24,6 +24,11 @@ void set_timer(ticks_t ticks)
 	sbi_set_timer(ticks);
 }
 
+void clear_timer()
+{
+	sbi_set_timer(-1);
+}
+
 /**
  * Get current ticks.
  *
diff --git a/include/arch/timer.h b/include/arch/timer.h
index 8b832c9..8f235e6 100644
--- a/include/arch/timer.h
+++ b/include/arch/timer.h
@@ -30,6 +30,12 @@ ticks_t stat_timer(const void *fdt);
 
 /**
  * Set up timer interrupt for absolute ticks.
+ * At least currently assumes there is only one global timer, the kernel will
+ * ensure that the timer is scheduled to handle events in order.
+ *
+ * When setting a new timer, the current timer (if one is set) will
+ * not trigger an interrupt. For instance, if called from the interrupt handler
+ * of the existing interrupt, it won't re-trigger.
  *
  * @param ticks Time point for timer to trigger.
  * \todo Should maybe be stat_t?
@@ -37,6 +43,13 @@ ticks_t stat_timer(const void *fdt);
 void set_timer(ticks_t ticks);
 
 /**
+ * Stop the timer interrupts from happening altogether. Mainly called during
+ * init to avoid spurious interrupts and when all userspace timers have been
+ * handled.
+ */
+void clear_timer();
+
+/**
  * Get current ticks.
  *
  * @return Current tick count.
diff --git a/src/timer.c b/src/timer.c
index 5583cd3..5b3ac86 100644
--- a/src/timer.c
+++ b/src/timer.c
@@ -73,6 +73,7 @@ static struct sp_root *__cpu_timers()
 
 void init_timer(const void *fdt)
 {
+	clear_timer();
 	ticks_per_sec = stat_timer(fdt);
 	info("ticks_per_sec: %" PRIu64 "\n", ticks_per_sec);
 	info("current ticks: %" PRIu64 "\n", current_ticks());
@@ -151,14 +152,23 @@ id_t new_rel_timer(id_t tid, ticks_t ticks)
 
 id_t new_abs_timer(id_t tid, ticks_t ticks)
 {
+	/* if there are no active timers or this timer is before the next active
+	 * timer, start the hardware timer */
+	struct timer *t = newest_timer();
+	if (!t || ticks < t->ticks)
+		set_timer(ticks);
+
 	id_t id = __new_timer(tid, ticks);
-	set_timer(ticks);
 	return id;
 }
 
 struct timer *newest_timer()
 {
-	struct sp_node *t = sp_first(sp_root(__cpu_timers()));
+	struct sp_node *r = sp_root(__cpu_timers());
+	if (!r)
+		return NULL;
+
+	struct sp_node *t = sp_first(r);
 	return &timer_container(t)->timer;
 }
 
@@ -202,9 +212,25 @@ void handle_timer()
 	/** @todo should this also disable irqs? */
 	bkl_lock();
 	struct timer *t = newest_timer();
+	if (!t) {
+		info("spurious timer\n");
+		clear_timer();
+		bkl_unlock();
+		return;
+	}
+
 	id_t tid = t->tid;
 	remove_timer(t);
 
+
+	/* maybe schedule next timer event, maybe disable timer so we don't
+	 * start infinitely retrigger the timer interrupt */
+	t = newest_timer();
+	if (t)
+		set_timer(t->ticks);
+	else
+		clear_timer();
+
 	struct tcb *r = get_tcb(tid);
 	if (!r || orphan(r)) {
 		info("tcb %llu dead at timer\n",
@@ -213,6 +239,7 @@ void handle_timer()
 		return;
 	}
 
+	/** @todo check that timer didn't trigger before scheduled time? */
 	notify(r, NOTIFY_TIMER);
 	bkl_unlock();
 }