riscv64: integrate minimal flattened devicetree reader and switch timer to time CSR

Punch list §25 item 0.3 NOT complete.

- Added `starkernel/fdt.h` and `fdt.c` for minimal read-only devicetree parsing: sufficient for boot-time lookups such as `timebase-frequency`.
- Bootloader now captures the devicetree blob (DTB) from `EFI_DTB_TABLE_GUID` into `BootInfo::dtb`.
- RISC-V timer subsystem now uses the `time` CSR as the primary timestamp source, abandoning the hardcoded `cycle` frequency assumption.
- Timer rate is read from `timebase-frequency` in the DTB when accessible; otherwise, a fallback value is used with a RELATIVE trust level.
- Integrated the SBI TIME extension for one-shot timer deadlines, ensuring re-arming occurs after each tick to avoid missing heartbeats.

Verified: riscv64 builds clean, boots to the ok> prompt with no regression; `riscv64/timer.c` reports accurate frequencies on QEMU's default firmware.
Signed-off-by: Robert Allan James <robert.allan.james@gmail.com>

src/starkernel/arch/riscv64/apic.c

@@ -12,10 +12,127 @@
 
 #include "apic.h"
 #include "uefi.h"
+#include "console.h"
+#include "timer.h"
 #include <stdint.h>
 
 static uint64_t s_timer_period_tsc = 0;
 
+/* ---------------------------------------------------------------------------
+ * SBI (Supervisor Binary Interface)
+ *
+ * RISC-V S-mode cannot program the timer directly: the CLINT's mtimecmp is an
+ * M-mode register. The timer is armed by asking the SEE (OpenSBI, running in
+ * M-mode beneath EDK2) via ECALL.
+ *
+ * Calling convention, SBI v0.2+ (SBI spec §3): a7 = EID, a6 = FID,
+ * a0.. = arguments; returns a0 = error, a1 = value.
+ * ------------------------------------------------------------------------- */
+
+#define SBI_EXT_BASE            0x10UL
+#define SBI_BASE_FID_PROBE_EXT  3UL
+
+#define SBI_EXT_TIME            0x54494D45UL   /* "TIME" */
+#define SBI_TIME_FID_SET_TIMER  0UL
+
+#define SBI_SUCCESS             0L
+
+/* sie.STIE — supervisor timer interrupt enable (Privileged Spec §4.1.3) */
+#define SIE_STIE                (1UL << 5)
+
+typedef struct
+{
+    long error;
+    long value;
+} sbiret_t;
+
+/** @brief Issue an SBI ECALL with one argument. */
+static sbiret_t sbi_call1(unsigned long eid, unsigned long fid,
+                          unsigned long arg0)
+{
+    register unsigned long r_a0 __asm__("a0") = arg0;
+    register unsigned long r_a1 __asm__("a1") = 0;
+    register unsigned long r_a6 __asm__("a6") = fid;
+    register unsigned long r_a7 __asm__("a7") = eid;
+    sbiret_t ret;
+
+    __asm__ volatile (
+    "ecall"
+    : "+r"(r_a0), "+r"(r_a1)
+    : "r"(r_a6), "r"(r_a7)
+    : "memory");
+
+    ret.error = (long)r_a0;
+    ret.value = (long)r_a1;
+    return ret;
+}
+
+/**
+ * @brief Read the RISC-V @c time CSR.
+ *
+ * Deadlines handed to @c sbi_set_timer() are absolute values on this counter.
+ * @c arch/riscv64/timer.c keeps its own copy of this accessor; duplicating
+ * four instructions is preferable to widening @c timer.h with an
+ * architecture-specific accessor that only these two files can use.
+ */
+static inline uint64_t rdtime(void)
+{
+    uint64_t val;
+    __asm__ volatile (
+    "rdtime %0" : "=r"(val));
+    return val;
+}
+
+/* Set once in apic_timer_start(): 1 when the TIME extension probed present,
+ * 0 when the timer could not be armed at all. */
+static int s_sbi_time_ok = 0;
+/* Absolute `time` value of the next expected interrupt. Advanced by period
+ * rather than recomputed from "now" so that a late tick does not push the
+ * whole schedule out; see riscv64_timer_rearm(). */
+static uint64_t s_next_deadline = 0;
+
+/**
+ * @brief Arm the SBI timer for @p deadline.
+ * @return 1 on success, 0 if the SEE rejected the call.
+ */
+static int sbi_set_timer(uint64_t deadline)
+{
+    sbiret_t r = sbi_call1(SBI_EXT_TIME, SBI_TIME_FID_SET_TIMER,
+                           (unsigned long)deadline);
+    return r.error == SBI_SUCCESS;
+}
+
+/**
+ * @brief Re-arm the one-shot SBI timer and account the tick.
+ *
+ * **The SBI timer is one-shot by nature.** Servicing a timer interrupt without
+ * programming the next deadline leaves the heartbeat stopped permanently, with
+ * no error anywhere — the single most likely silent failure of this driver.
+ * Every path out of a timer interrupt must reach this function.
+ *
+ * Called from @c riscv64_interrupt_handler() in @c interrupts.c on
+ * @c scause cause 5.
+ */
+void riscv64_timer_rearm(void)
+{
+    uint64_t now;
+
+    if (!s_sbi_time_ok) return;
+
+    s_next_deadline += s_timer_period_tsc;
+
+    /* If servicing ran long enough that the next deadline is already behind
+     * us, resynchronise rather than burn through a backlog of instant
+     * interrupts. */
+    now = rdtime();
+    if (s_next_deadline <= now)
+    {
+        s_next_deadline = now + s_timer_period_tsc;
+    }
+
+    sbi_set_timer(s_next_deadline);
+}
+
 /*
  * @brief Initialise the interrupt controller (RISC-V PLIC stub).
  *
@@ -70,20 +187,62 @@ int apic_timer_init(uint64_t tsc_hz, uint32_t tick_hz)
 }
 
 /**
- * @brief Start periodic timer delivery (RISC-V stub).
+ * @brief Start timer delivery via the SBI TIME extension.
  *
- * On x86-64 this unmasks the APIC timer. On RISC-V a periodic timer would
- * be armed via CLINT or SBI here; the driver is deferred. No-op stub.
+ * Probes for the TIME extension first. If the SEE does not provide it the
+ * timer is **not** armed and the condition is reported loudly rather than
+ * papered over with the legacy EID 0x00 call: a heartbeat that silently never
+ * ticks is far worse to diagnose than one that says why at boot.
+ *
+ * On success: computes the first absolute deadline, arms it, and sets
+ * @c sie.STIE. Global delivery is gated separately by @c sstatus.SIE, which
+ * @c arch_enable_interrupts() sets.
  */
-void apic_timer_start(void) { }
+void apic_timer_start(void)
+{
+    sbiret_t probe;
+
+    probe = sbi_call1(SBI_EXT_BASE, SBI_BASE_FID_PROBE_EXT, SBI_EXT_TIME);
+    if (probe.error != SBI_SUCCESS || probe.value == 0)
+    {
+        console_println("SBI: TIME extension ABSENT - timer NOT armed, "
+            "heartbeat will not tick");
+        s_sbi_time_ok = 0;
+        return;
+    }
+
+    s_sbi_time_ok = 1;
+    s_next_deadline = rdtime() + s_timer_period_tsc;
+
+    if (!sbi_set_timer(s_next_deadline))
+    {
+        console_println("SBI: set_timer REJECTED - timer NOT armed");
+        s_sbi_time_ok = 0;
+        return;
+    }
+
+    __asm__ volatile (
+    "csrs sie, %0"
+    ::
+    "r"(SIE_STIE) : "memory");
+    console_println("SBI: timer armed (TIME extension)");
+}
 
 /**
- * @brief Stop periodic timer delivery (RISC-V stub).
+ * @brief Stop timer delivery by masking @c sie.STIE.
  *
- * On x86-64 this masks the APIC timer. On RISC-V a periodic timer would
- * be disarmed via CLINT or SBI here; the driver is deferred. No-op stub.
+ * The SBI timer cannot be cancelled outright — masking the enable bit is the
+ * supported way to stop delivery. Any deadline already programmed simply goes
+ * unserviced.
  */
-void apic_timer_stop(void)  { }
+void apic_timer_stop(void)
+{
+    __asm__ volatile (
+    "csrc sie, %0"
+    ::
+    "r"(SIE_STIE) : "memory");
+    s_sbi_time_ok = 0;
+}
 
 /**
  * @brief Return the expected cycle-counter ticks per heartbeat period.

src/starkernel/arch/riscv64/interrupts.c

@@ -17,6 +17,12 @@ volatile const char *g_sk_fault_word = (void *)0;
 
 extern void riscv64_install_vectors(void);
 
+/* Defined in arch/riscv64/apic.c. Declared here rather than in the shared
+ * starkernel/apic.h because re-arming is specific to the one-shot SBI timer
+ * and has no meaning for the amd64 periodic APIC timer. Same extern-in-place
+ * convention as riscv64_install_vectors above. */
+extern void riscv64_timer_rearm(void);
+
 /* scause cause codes for supervisor-mode interrupts (RISC-V Privileged Spec
  * §4.1.9, Table "Supervisor cause register values"). Only the timer is used;
  * software (1) and external (9) interrupts are not enabled. */
@@ -30,11 +36,11 @@ extern void riscv64_install_vectors(void);
  * Unlike @c riscv64_exception_handler() this **returns** — the trap entry
  * restores the caller-saved register set and issues @c SRET.
  *
- * Supervisor timer (cause 5) is routed to @c heartbeat_tick(). No timer is
- * armed yet: arming via the SBI TIME extension, enabling @c sie.STIE, and the
- * mandatory per-tick re-arm are punch-list item 0.3. Until then this path is
- * unreachable, which is why item 0.2 accepts on "boots with no regression"
- * rather than on having observed an interrupt.
+ * Supervisor timer (cause 5) re-arms the one-shot SBI timer and then accounts
+ * the tick. **Re-arm comes first**: the SBI timer fires once per programmed
+ * deadline, so any return path that skips the re-arm stops the heartbeat
+ * permanently and silently. Ordering it ahead of @c heartbeat_tick() means a
+ * fault in the bookkeeping cannot also cost the next tick.
  *
  * Any other cause is ignored rather than fatal: an unexpected-but-harmless
  * asynchronous interrupt should not take the kernel down, and none are
@@ -47,6 +53,7 @@ void riscv64_interrupt_handler(uint64_t scause)
     uint64_t cause = scause & ~SCAUSE_INTERRUPT_BIT;
 
     if (cause == SCAUSE_S_TIMER) {
+        riscv64_timer_rearm();
         heartbeat_tick();
     }
 }

src/starkernel/arch/riscv64/timer.c

@@ -5,45 +5,92 @@
  */
 
 /**
- * timer.c (riscv64) - Timer using rdcycle counter.
+ * timer.c (riscv64) - Timer using the `time` CSR.
  *
- * RISC-V provides rdcycle (CPU cycle counter) and rdtime (wall-clock timer).
- * We use rdcycle as the primary timestamp source; frequency is estimated
- * at 1 GHz (QEMU virt default) and updated if firmware provides a hint.
+ * RISC-V provides `cycle` (per-hart clock cycles) and `time` (fixed-frequency
+ * wall clock). The primary timestamp source is `time`, read via rdtime, and
+ * its rate comes from the devicetree property timebase-frequency.
+ *
+ * This was `cycle` at an assumed 1 GHz until punch-list item 0.3. Two things
+ * forced the change: the SBI TIME extension arms deadlines against `time`, so
+ * mixing the two counters would compare unrelated clocks; and `cycle` has no
+ * discoverable frequency, so every heartbeat variance and TIME-TRUST figure
+ * riscv64 produced before this was measured against a wrong expected interval
+ * (FABRIC.md §16.2). Figures from before and after are not comparable.
  */
 
 #include "timer.h"
 #include "console.h"
 #include "q48_16.h"
 #include "uefi.h"
+#include "starkernel/fdt.h"
 #include <stdint.h>
 
 /**
- * @brief Read the RISC-V CPU cycle counter (@c rdcycle).
+ * @brief Read the RISC-V wall-clock counter (@c rdtime, CSR @c time 0xC01).
  *
- * Issues the @c RDCYCLE pseudo-instruction (a @c CSRRS on @c cycle,
- * CSR 0xC00) to read the 64-bit hardware cycle counter. On RISC-V the
- * cycle counter is a per-hart monotonically incrementing register whose
- * frequency equals the hart's clock rate — the architectural equivalent
- * of the x86-64 TSC.
+ * @c time is the memory-mapped real-time counter mandated by the privileged
+ * spec: fixed frequency, common to all harts, and — unlike @c cycle — its
+ * rate is discoverable, published by firmware as the devicetree property
+ * @c timebase-frequency.
  *
- * Unlike @c CNTPCT_EL0 on AArch64, @c cycle is not architecturally
- * synchronised across harts; this is acceptable for the single-hart
- * LithosAnanke build. The frequency is not provided by hardware CSR
- * (unlike AArch64's @c CNTFRQ_EL0); @c timer_init() assumes 1 GHz
- * for QEMU @c virt-machine compatibility.
+ * This replaces the earlier @c rdcycle() source. Two reasons, and the first
+ * is not optional:
  *
- * @return Current 64-bit cycle count; wraps at UINT64_MAX (about 584 years
- *         at 1 GHz — not a practical concern).
+ * - **The SBI TIME extension is defined against @c time.** @c sbi_set_timer()
+ *   takes an absolute value on this counter, so arming a timer from a
+ *   @c cycle reading would compare two unrelated clocks.
+ * - **@c cycle's frequency is not discoverable**, which is why the previous
+ *   implementation hardcoded an assumed 1 GHz. Every heartbeat variance and
+ *   TIME-TRUST figure riscv64 has produced was therefore computed against a
+ *   wrong expected interval.
+ *
+ * @return Current 64-bit @c time value.
  */
-static inline uint64_t rdcycle(void)
+static inline uint64_t rdtime(void)
 {
     uint64_t val;
-    __asm__ volatile ("rdcycle %0" : "=r"(val));
+    __asm__ volatile (
+    "rdtime %0" : "=r"(val));
     return val;
 }
 
-static uint64_t s_counter_hz  = 1000000000ULL; /* assume 1 GHz */
+/* Fallback when firmware publishes no devicetree, or none carrying
+ * timebase-frequency. 10 MHz is the QEMU virt machine's value. Named rather
+ * than inlined so that a boot running on the fallback is greppable and
+ * obviously distinct from a discovered rate. */
+#define RISCV_TIMEBASE_HZ_FALLBACK 10000000ULL
+
+/**
+ * @brief Print @p val in decimal via @c console_putc().
+ *
+ * Same shape as the file-local helper in @c arch/amd64/timer.c — there is no
+ * shared decimal printer in @c console.h, and the freestanding build has no
+ * @c printf(). Prints "0" for zero.
+ *
+ * @param val Value to print.
+ */
+static void print_dec(uint64_t val)
+{
+    char buf[32];
+    int i = 0;
+    if (val == 0)
+    {
+        console_putc('0');
+        return;
+    }
+    while (val > 0 && i < (int)sizeof(buf))
+    {
+        buf[i++] = (char)('0' + (val % 10));
+        val /= 10;
+    }
+    while (i-- > 0)
+    {
+        console_putc(buf[i]);
+    }
+}
+
+static uint64_t s_counter_hz = RISCV_TIMEBASE_HZ_FALLBACK;
 static uint64_t s_ns_per_tick = 0;
 static uint64_t s_base_count  = 0;
 static uint64_t s_base_ns     = 0;
@@ -54,31 +101,44 @@ static TimeTrustState g_heartbeat;
 /*
  * @brief Initialise the RISC-V timer subsystem (M5 milestone).
  *
- * On RISC-V there is no architectural CSR that directly reports the
- * @c rdcycle frequency (unlike AArch64's @c CNTFRQ_EL0). The timer
- * subsystem therefore assumes 1,000,000,000 Hz (1 GHz), which matches
- * QEMU's @c virt machine default clock. Real hardware board support would
- * need to read the frequency from a device tree or firmware table and
- * update @c s_counter_hz before computing @c s_ns_per_tick.
+ * RISC-V has no CSR reporting the counter frequency (unlike AArch64's
+ * @c CNTFRQ_EL0), so the rate is read from the devicetree property
+ * @c timebase-frequency, located via the blob the firmware publishes under
+ * @c EFI_DTB_TABLE_GUID and carried in @c BootInfo::dtb.
  *
  * Steps performed:
- * 1. Snapshots @c rdcycle() into @c s_base_count as the ns origin.
- * 2. Computes @c s_ns_per_tick as @c (1e9 << 16) / @c s_counter_hz in
+ * 1. Reads @c timebase-frequency from @c boot_info->dtb when present;
+ *    otherwise keeps @c RISCV_TIMEBASE_HZ_FALLBACK and says so on the console.
+ * 2. Snapshots @c rdtime() into @c s_base_count as the ns origin.
+ * 3. Computes @c s_ns_per_tick as @c (1e9 << 16) / @c s_counter_hz in
  *    Q16.16 fixed-point to avoid floating-point in the freestanding build.
- * 3. Fills @c s_cal with the assumed frequency and sets
- *    @c TIMER_TRUST_ABSOLUTE (the @c rdcycle counter is invariant by
- *    specification once enabled, though its frequency is merely assumed
- *    rather than measured).
+ * 4. Fills @c s_cal, setting @c TIMER_TRUST_ABSOLUTE only when the rate came
+ *    from firmware. On the fallback the counter is still invariant, but its
+ *    scaling to real time is a guess — which is @c TIMER_TRUST_RELATIVE.
  *
- * @param boot_info Kernel @c BootInfo (device-tree / ACPI pointer); unused
- *                  at this milestone — clock frequency is hard-coded.
+ * @param boot_info Kernel @c BootInfo; @c ::dtb supplies the counter rate.
+ *                  NULL, or a NULL/invalid blob, selects the fallback.
  * @return 0 always.
  */
 int timer_init(BootInfo *boot_info)
 {
-    (void)boot_info;
+    uint32_t hz = 0;
+    int discovered = 0;
 
-    s_base_count  = rdcycle();
+    /* Discover the counter rate rather than assuming it. The devicetree is
+     * the only source: RISC-V has no CNTFRQ_EL0 equivalent. boot_info->dtb is
+     * NULL when firmware published no devicetree, in which case the named
+     * fallback stands and the banner says so. */
+    if (boot_info && fdt_valid(boot_info->dtb))
+    {
+        if (fdt_prop_u32(boot_info->dtb, "timebase-frequency", &hz) && hz != 0)
+        {
+            s_counter_hz = (uint64_t)hz;
+            discovered = 1;
+        }
+    }
+
+    s_base_count = rdtime();
     s_base_ns     = 0;
     s_ns_per_tick = (1000000000ULL << 16) / s_counter_hz;
 
@@ -87,14 +147,19 @@ int timer_init(BootInfo *boot_info)
     s_cal.pit_hz_mean = 0;
     s_cal.converged   = 1;
     s_cal.vm_mode     = 1;
-    s_cal.trust       = TIMER_TRUST_ABSOLUTE;
+    /* ABSOLUTE only when the rate came from firmware. On the fallback the
+     * counter is still monotonic and invariant, but its scaling to real time
+     * is a guess, which is exactly the RELATIVE case. */
+    s_cal.trust = discovered ? TIMER_TRUST_ABSOLUTE : TIMER_TRUST_RELATIVE;
 
-    console_println("Timer: RISC-V rdcycle timer initialised.");
+    console_puts("Timer: RISC-V time CSR @ ");
+    print_dec(s_counter_hz);
+    console_println(discovered ? " Hz (devicetree)" : " Hz (FALLBACK, no devicetree)");
     return 0;
 }
 
 /**
- * @brief Return the assumed cycle-counter frequency in Hz.
+ * @brief Return the `time` counter frequency in Hz.
  *
  * Returns @c s_counter_hz (initialised to 1,000,000,000 by the module).
  * Used by @c apic_timer_init() to compute the expected inter-tick period
@@ -128,7 +193,7 @@ uint64_t timer_tsc_hz(void)
  */
 uint64_t timer_now_ns(void)
 {
-    uint64_t delta = rdcycle() - s_base_count;
+    uint64_t delta = rdtime() - s_base_count;
     return s_base_ns + ((delta * s_ns_per_tick) >> 16);
 }
 
@@ -190,18 +255,24 @@ void heartbeat_init(uint64_t tsc_hz, uint64_t tick_hz)
 /**
  * @brief Record one heartbeat tick and update the inter-tick deviation window.
  *
- * Reads @c rdcycle() and, if @c last_tsc is non-zero, records the signed
+ * Reads @c rdtime() and, if @c last_tsc is non-zero, records the signed
  * deviation @c ((now - last_tsc) - expected_delta) into the circular
  * @c window.deltas[] buffer. Increments @c ticks and @c total_samples.
- * Sets @c trust = @c Q48_ONE unconditionally — the RISC-V cycle counter
+ * Sets @c trust = @c Q48_ONE unconditionally — the RISC-V @c time counter
  * is invariant and needs no statistical quality estimate.
  *
- * Called from the RISC-V timer ISR stub (or its no-op placeholder) at
- * each periodic heartbeat period.
+ * Must read the same counter the deadline was programmed against.
+ * @c expected_delta is derived from @c timebase-frequency and is therefore in
+ * @c time units; measuring the interval with @c cycle instead would difference
+ * two unrelated clocks and produce exactly the wrong-expected-interval defect
+ * that switching off @c rdcycle was meant to remove.
+ *
+ * Called from @c riscv64_interrupt_handler() on @c scause cause 5, after the
+ * timer has been re-armed.
  */
 void heartbeat_tick(void)
 {
-    uint64_t now = rdcycle();
+    uint64_t now = rdtime();
     if (g_heartbeat.last_tsc != 0) {
         int64_t delta = (int64_t)(now - g_heartbeat.last_tsc)
                       - (int64_t)g_heartbeat.expected_delta;