Re: [PATCH v2 2/2] System Peripherals: Linux 2022 Base spec and server extension


Mayuresh Chitale
 





On Tue, May 4, 2021 at 11:50 PM Jonathan Behrens <behrensj@...> wrote:
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.
Actually these are different requirements. From the software perspective the counter is required to have atleast 10 ns resolution. But the hardware clock that drives the counter is expected to run at a minimum frequency of 10 MHz. So the counter increment per clock tick could be greater than 1 depending on the clock frequency.  

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






Join tech-unixplatformspec@lists.riscv.org to automatically receive all group messages.