[PATCH] lib: sbi_timer: Insert timer event before the later event

[Nickolai Zeldovich]

The per-HART event list is kept sorted by expiry time: the head is the
event programmed into the timer device by __sbi_timer_update_device(),
and sbi_timer_process() stops at the first unexpired entry.

__sbi_timer_event_start() finds the first event that expires after the
new one, but then inserts the new event with sbi_list_add(), i.e. after
that event instead of before it.  Starting an event at time 50 while an
event at time 1000 is pending yields the list [1000, 50]: the device is
programmed for 1000 and the earlier event fires 950 ticks late.

Today each HART only ever has its S-mode timer event on the list, so
the misordering needs a second timer event user to show up.  Once there
is one, on HARTs without Sstc an S-mode timer programmed while a later
M-mode event is pending is delivered only when that later event
expires.

Use sbi_list_add_tail() so the new event goes in front of next_ev.

Fixes: b7fa8a246cba ("lib: sbi_timer: Add support for timer events")
Signed-off-by: Nickolai Zeldovich <nickolai at csail.mit.edu>
Co-Authored-By: Claude Fable 5.1 <noreply at anthropic.com>
---
This bug was found in the process of formally verifying the correctness
of OpenSBI on top of the Sail RISC-V semantics using Lean.

__sbi_timer_event_start() walks event_list for the first tev with
next_event < tev->time_stamp and then calls
sbi_list_add(&ev->head, &next_ev->head), which links ev after next_ev
(__sbi_list_add(new, head, head->next)).  Every later consumer assumes
the list is sorted: __sbi_timer_update_device() programs the head's
time_stamp and sbi_timer_process() breaks at the first unexpired head.
With [A at 1000] pending, starting B at 50 gives [A at 1000, B at 50], so B fires
at 1000.  Inserting at the tail when no later event exists was already
correct.

In-tree each HART has only smode_ev on its list (and
sbi_timer_event_start() unlinks an event before re-inserting it), so
the list never has two entries today.  The proposed SBI watchdog
extension (Prajwal N, "[PATCH 3/4] lib: sbi: Add SBI watchdog
framework", 2026-10-02) adds two more per-HART events; on a HART
without Sstc, an S-mode timer set while the watchdog event is pending
would then be delayed until the watchdog event expires.

This is independent of Pengpeng Hou's pending "lib: sbi_timer: Restart
events from the restart list" (2026-08-31), which fixes the restart
loop in sbi_timer_process(); the two patches touch different hunks and
apply in either order.  Re-armed events go through the same insertion,
so they need both.

Testing:
- make PLATFORM=generic LLVM=1 PLATFORM_RISCV_XLEN=64: builds, no
  warnings (also with Pengpeng Hou's patch applied on top).
- PLATFORM_RISCV_XLEN=32 (rv32imac_zicsr_zifencei/ilp32): sbi_timer.o
  builds without warnings; the full rv32 build stops in
  lib/sbi/sbi_trap_ldst.c (-Wsometimes-uninitialized on 'val'), which
  fails the same way on unpatched master.
- Userspace harness: include/sbi/sbi_list.h plus copies of
  __sbi_timer_event_start(), __sbi_timer_event_stop() and
  sbi_timer_process() with stubs.  Starting 50 while 1000 is pending:
  master gives [1000, 50], patched gives [50, 1000].  A callback
  re-arming at 110 with an event pending at 1000: with only the
  restart-list fix the result is [1000, 110], with both [110, 1000].
- Not run on QEMU or hardware.  To see it there, start two M-mode timer
  events on one HART, the later one first, e.g. from a platform
  final_init on a non-Sstc CPU (qemu-system-riscv64 -M virt
  -cpu rv64,sstc=off): sbi_timer_event_start(&a, now + 10 * freq);
  sbi_timer_event_start(&b, now + freq); b's callback runs after 10 s
  on master and after 1 s with this patch.

 lib/sbi/sbi_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
index 78aaa32e..1f13a3b1 100644
--- a/lib/sbi/sbi_timer.c
+++ b/lib/sbi/sbi_timer.c
@@ -205,7 +205,7 @@ static void __sbi_timer_event_start(struct timer_state *tstate,
 	ev->hart_index = current_hartindex();
 	ev->time_stamp = next_event;
 	if (next_ev)
-		sbi_list_add(&ev->head, &next_ev->head);
+		sbi_list_add_tail(&ev->head, &next_ev->head);
 	else
 		sbi_list_add_tail(&ev->head, &tstate->event_list);
 }

base-commit: 6ad246a1494807ee91e36ffcf60e4c30bb309d45
-- 
2.53.0