[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v4 03/42] xen/riscv: introduce csr_volatile_read64()



On RV32 a 64-bit CSR is accessed as two halves, <csr> and <csr>H. Reading
the halves with two separate instructions isn't always safe: for a CSR
which hardware increments, a carry from the low half into the high one can
happen in between, so a plain pair of reads can produce a value the CSR
never held. Therefore the high half is re-read and the sequence retried if
it changed in the meantime.

Only CSRs which change under Xen's feet need that, hence the name. The
majority can be read with a plain pair of csr_read()s, so no helper for
those is introduced here as there would be no user for it.

Use it for CSR_TIME, which is exactly such a counter, and widen cycles_t to
uint64_t. Otherwise get_cycles() would still truncate the time counter to
32 bits on RV32.

No functional change on RV64.

Fixes: a541ddadec0a ("xen/riscv: introduce time.h")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes in v4:
 - Take the CSR name without the CSR_ prefix, so that the macro parameter
   is used as an operand of ## only (MISRA C Rule 20.12).
 - Drop the (void) of the ...H constant on RV64.
 - Initialise hi_ in its declaration.
---
Changes in v3:
 - Rename csr_read64() to csr_volatile_read64(): only CSRs which change
   under Xen's feet need the retry, the majority can be read with a plain
   split access, which isn't introduced here as it would have no user.
 - Carry the high half read at the end of an iteration over to the next
   one, instead of reading it twice per retry.
 - Update the commit message.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/include/asm/csr.h  | 22 ++++++++++++++++++++++
 xen/arch/riscv/include/asm/time.h |  4 ++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
index 3cf8e1857ce2..6df595e810ef 100644
--- a/xen/arch/riscv/include/asm/csr.h
+++ b/xen/arch/riscv/include/asm/csr.h
@@ -39,8 +39,30 @@
     csr_write(CSR_ ## name, v_);            \
     csr_write(CSR_ ## name ## H, v_ >> 32); \
 })
+
+/*
+ * The two halves are read by separate instructions, so a CSR which hardware
+ * increments can carry from the low half into the high one in between,
+ * yielding a value the CSR never held. Re-read the high half and retry the
+ * sequence if it changed.
+ *
+ * Only needed for CSRs which change under Xen's feet.
+ */
+#define csr_volatile_read64(name)                            \
+({                                                           \
+    uint32_t lo_, old_, hi_ = csr_read(CSR_ ## name ## H);   \
+                                                             \
+    do {                                                     \
+        old_ = hi_;                                          \
+        lo_ = csr_read(CSR_ ## name);                        \
+    } while ( (hi_ = csr_read(CSR_ ## name ## H)) != old_ ); \
+                                                             \
+    ((uint64_t)hi_ << 32) | lo_;                             \
+})
 #else
 #define csr_write64(name, val) csr_write(CSR_ ## name, val)
+
+#define csr_volatile_read64(name) csr_read(CSR_ ## name)
 #endif
 
 #define csr_swap(csr, val)                                      \
diff --git a/xen/arch/riscv/include/asm/time.h 
b/xen/arch/riscv/include/asm/time.h
index 0e5b96a8631c..02b9a03ed90d 100644
--- a/xen/arch/riscv/include/asm/time.h
+++ b/xen/arch/riscv/include/asm/time.h
@@ -18,11 +18,11 @@ static inline void force_update_vcpu_system_time(struct 
vcpu *v)
     BUG_ON("unimplemented");
 }
 
-typedef unsigned long cycles_t;
+typedef uint64_t cycles_t;
 
 static inline cycles_t get_cycles(void)
 {
-    return csr_read(CSR_TIME);
+    return csr_volatile_read64(TIME);
 }
 
 void preinit_xen_time(void);
-- 
2.56.1




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.