Is the counter required to be at least 10ns resolution or exactly 10ns resolution? The document seems to be inconsistent on which is desired, but I personally don't see the point of having an "at least" requirement given that the update frequency is also specified.

On Tue, May 4, 2021 at 1:46 PM Mayuresh Chitale via lists.riscv.org <mchitale=ventanamicro.com@...> wrote:

Base spec:
- UART
- Clock and Timers

Server extension:
- Clock and Timers

Signed-off-by: Mayuresh Chitale <mchitale@...>
---
 riscv-platform-spec.adoc | 137 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 134 insertions(+), 3 deletions(-)

diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index 160c74a..ee21380 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -55,9 +55,40 @@ include::profiles.adoc[]

 ==== System Peripherals
 * UART/Serial Console
-* Clocks
-* Timers
-* Watchdog Timers
+
+In order to facilitate the bringup and debug of the low level initial platform
+software(firmware, bootloaders, kernel etc), platforms are required to
+implement a UART port which confirms to the following requirements:
+
+* The UART register addresses are required to be aligned to 4 byte boundaries.
+If the implemented register width is less than 4 bytes then the implmented
+bytes are required to be mapped starting at the smallest address.
+* The UART port implementation is required to be register-compatible with one
+of the following:
+** UART 16550 - _REQUIRED_
+** UART 8250 - _DEPRECATED_
+
+* Clock and Timers
+** Platforms are required to provide a 10ns resolution 64-bit counter with strictly
+monotonic updates.
+** The hardware clock that drives the counter is required to operate at a minimum
+frequency of 10MHz.
+** Platforms that use DT for hardware discovery are required to advertise the
+timebase to the operating systems via the `timebase-frequency` property of the
+"/cpus" node
+footnote:[https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/riscv/cpus.yaml].
+
+[sidebar]
+--
+[underline]*_Implementation Note_*
+
+For a counter with 10ns resolution the `timebase-frequency` value would be 100000000
+(100 MHz) which would also be the minimum possible value for `timebase-frequency`.
+From the software perspective a unit increment of the mtime value would correspond
+to a 10ns interval. However the hardware clock driving the counter could operate at a
+lower frequency, thereby incrementing the mtime value by more than one unit per
+clock tick.
+--

 ==== Boot Process
 * Firmware
@@ -246,6 +277,106 @@ implemented but it can return EFI_UNSUPPORTED.
 |===

 ==== System Peripherals
+* Clock and Timers
+** Platforms are required to implement the time CSR.
+** Platforms are required to implement the
+https://lists.riscv.org/g/tech-privileged/message/404[Sstc] extension.
+** Platforms are required to delegate the supervisor timer interrupt to 'S'
+mode. If the 'H' extension is implemented then the platforms are required to
+delegate the virtual supervisor timer interrupt to 'VS' mode.
+
+* Watchdog Timers
+** Platforms are required to implement a two stage watchdog timer - watchdog
+stage 1 (WS1) and watchdog stage 2(WS2).
+** The software is required to periodically update the WS1 compare value
+to a future moment.

How about "S-mode software may periodically update..." ? Software is also allowed to just leave w1timecmp set to 0xFFFFFFFFFFFFFFFF.

 

+** If the mtime increments past the WS1 compare value then the platform is
+required to raise an interrupt and route it to a RISC V hart and also
+activate the WS2.

This seems to me like it would be vastly more useful as an NMI than a normal interrupt.

 

+** If the mtime increments past the WS2 compare value then the platform is
+required to undergo a system reset. Specification of the mechanism to affect
+the system reset is out of the scope of this document. If the WS1 compare value
+was updated before the WS2 expiry then the platform is required to deassert the
+WS1 interrupt and also deactivate the WS2.
+
+[sidebar]
+--
+[underline]*_Implementation Note_*
+
+_The watchdog timers could be implemented using memory mapped machine mode
+registers similar to mtimecmp._

"could" -> "must"

If watchdog timers are mandatory, then a specific hardware-software interface also needs to be required.

+
+_W1timecmp: w1timecmp is a 64 bit memory mapped register that contains the
+compare value for WS1. WS1 shall expire whenever w1timecmp contains a value
+greater than mtime, treating the values as unsigned integers.
+W1timecmp register is periodically updated by software to a future moment
+to avoid WS1 expiry._
+
+_W2timedelta: w2timedelta is a 64 bit memory mapped register that together
+with w1timecmp provides the WS2 expiry value. WS2 shall expire whenever
+w1timecmp + w2timedelta evaluates to a value greater than mtime, treating
+the values as unsigned integers._
+
+The default value of w1timecmp and w2timecmp is 0xFFFFFFFFFFFFFFFF. Since
+the mtime value cannot exceed the default value of w1timecmp and w2timecmp
+the watchdog timers are disabled by default. The following sample code enables
+the WS1 and WS2 timers with intervals of 5mins and 10mins
+respectively.
+
+*_RISC-V 64_*
+```
+#define WDT_DELAY_SEC 300
+
+/* Base addresses and timebase are initialized elsewhere */
+extern void *mtime;
+extern void *w1timecmp;
+extern void *w2timecmp;
+extern unsigned long riscv_timebase;
+
+void riscv_wd_init(void)
+{
+    uint64_t now;
+    uint64_t ws1 = WDT_DELAY_SEC * riscv_timebase;
+    uint64_t ws2 = WDT_DELAY_SEC * riscv_timebase;
+
+    now = readq(mtime);
+    writeq(w2timecmp, now + ws1 + ws2);
+    writeq(w1timecmp, now + ws1);
+}
+```
+*_RISC-V 32_*
+```
+#define WDT_DELAY_SEC 300
+
+/* Base addresses and timebase are initialized elsewhere */
+extern void *mtime;
+extern void *w1timecmp;
+extern void *w2timecmp;
+extern unsigned long riscv_timebase;
+
+void riscv_wd_reg_write(void *addr, uint64_t value)
+{
+    /* As per sample code in section 3.2.1 of RISC-V privileged spec */
+    writel(addr, -1U);
+    writel(addr + 0x4, value >> 32);
+    writel(addr, value);
+}
+
+void riscv_wd_init(void)
+{
+    uint32_t now_l, now_h;
+    uint64_t now;
+    uint64_t ws1 = WDT_DELAY_SEC * riscv_timebase;
+    uint64_t ws2 = WDT_DELAY_SEC * riscv_timebase;
+
+    now_h = readl(mtime + 4);
+    now_l = readl(mtime);
+    now = ((uint64_t)now_h << 32) | now_l;
+    riscv_wd_reg_write(w2timecmp, now + ws1 + ws2);
+    riscv_wd_reg_write(w1timecmp, now + ws1);
+}
+```

These examples are missing any initialization of the w[1|2]timecmp pointers. That presumably comes from the device tree, but the exact layout needs to be specified somewhere.

 

+--
 * PCI-E

 ==== Secure Boot
--
2.17.1