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

[PATCH v3 03/39] 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 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  | 27 +++++++++++++++++++++++++++
 xen/arch/riscv/include/asm/time.h |  4 ++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
index 888d6a2a86d6..a5cb24c863dd 100644
--- a/xen/arch/riscv/include/asm/csr.h
+++ b/xen/arch/riscv/include/asm/csr.h
@@ -39,12 +39,39 @@
     csr_write(csr, v_);             \
     csr_write(csr ## 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(csr)                    \
+({                                                  \
+    uint32_t hi_, lo_, old_;                        \
+                                                    \
+    hi_ = csr_read(csr ## H);                       \
+    do {                                            \
+        old_ = hi_;                                 \
+        lo_ = csr_read(csr);                        \
+    } while ( (hi_ = csr_read(csr ## H)) != old_ ); \
+                                                    \
+    ((uint64_t)hi_ << 32) | lo_;                    \
+})
 #else
 #define csr_write64(csr, val)       \
 ({                                  \
     csr_write(csr, val);            \
     (void)csr ## H;                 \
 })
+
+#define csr_volatile_read64(csr)    \
+({                                  \
+    (void)csr ## H;                 \
+    csr_read(csr);                  \
+})
 #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 4d68900151a7..a7c9736ae719 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(CSR_TIME);
 }
 
 void preinit_xen_time(void);
-- 
2.55.0




 


Rackspace

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