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

[PATCH] arm64: rewrite io r/w helper to avoid kvm crash.



For now, unikraft will crash on arm64 in kvm mode if it is compiled using
optimize mode. Because the compiler will choose post-index str intructions
to write device memory. Unluckily, arm64 virtualization extension only
provides syndrome information for a limited subset of load/store
instructions, that means we can't use load/store with writeback addressing
mode to access mmio device.
To avoid the compiler to do those optimization, io read/write helpers
should be rewitten using inline assembly with volatile constraint. After
this, unikraft can work fine on arm64.

Reviewed-by: Julien Grall <julien@xxxxxxx>
Signed-off-by: Jianyong Wu <jianyong.wu@xxxxxxx>
---
 plat/common/include/arm/arm64/cpu.h | 85 ++++++++++++++++++++---------
 1 file changed, 60 insertions(+), 25 deletions(-)

diff --git a/plat/common/include/arm/arm64/cpu.h 
b/plat/common/include/arm/arm64/cpu.h
index 93ad13b..fe981eb 100644
--- a/plat/common/include/arm/arm64/cpu.h
+++ b/plat/common/include/arm/arm64/cpu.h
@@ -41,31 +41,66 @@
 #include <uk/alloc.h>
 #include <uk/assert.h>
 
-/* Define macros to access IO registers */
-#define __IOREG_READ(bits) \
-static inline uint##bits##_t \
-       ioreg_read##bits(const volatile uint##bits##_t *addr) \
-               { return *addr; }
-
-#define __IOREG_WRITE(bits) \
-static inline void \
-       ioreg_write##bits(volatile uint##bits##_t *addr, \
-                                               uint##bits##_t value) \
-               { *addr = value; }
-
-
-#define __IOREG_READ_ALL() __IOREG_READ(8)  \
-                          __IOREG_READ(16) \
-                          __IOREG_READ(32) \
-                          __IOREG_READ(64) \
-
-#define __IOREG_WRITE_ALL()    __IOREG_WRITE(8)  \
-                          __IOREG_WRITE(16) \
-                          __IOREG_WRITE(32) \
-                          __IOREG_WRITE(64) \
-
-__IOREG_READ_ALL()
-__IOREG_WRITE_ALL()
+/*
+ * we should use inline assembly with volatile constraint to access mmio
+ * device memory to avoid compiler use load/store instructions of writeback
+ * addressing mode which will cause crash when running in hyper mode
+ * unless they will be decoded by hypervisor.
+ */
+static inline uint8_t ioreg_read8(const volatile uint8_t *address)
+{
+       uint8_t value;
+
+       asm volatile ("ldrb %w0, [%1]" : "=r"(value) : "r"(address));
+       return value;
+}
+
+static inline uint16_t ioreg_read16(const volatile uint16_t *address)
+{
+       uint16_t value;
+
+       asm volatile ("ldrh %w0, [%1]" : "=r"(value) : "r"(address));
+       return value;
+}
+
+static inline uint32_t ioreg_read32(const volatile uint32_t *address)
+{
+       uint32_t value;
+
+       asm volatile ("ldr %w0, [%1]" : "=r"(value) : "r"(address));
+       return value;
+}
+
+static inline uint64_t ioreg_read64(const volatile uint64_t *address)
+{
+       uint64_t value;
+
+       asm volatile ("ldr %0, [%1]" : "=r"(value) : "r"(address));
+       return value;
+}
+
+static inline void ioreg_write8(const volatile uint8_t *address, uint8_t value)
+{
+       asm volatile ("strb %w0, [%1]" : : "rZ"(value), "r"(address));
+}
+
+static inline void ioreg_write16(const volatile uint16_t *address,
+                                uint16_t value)
+{
+       asm volatile ("strh %w0, [%1]" : : "rZ"(value), "r"(address));
+}
+
+static inline void ioreg_write32(const volatile uint32_t *address,
+                                uint32_t value)
+{
+       asm volatile ("str %w0, [%1]" : : "rZ"(value), "r"(address));
+}
+
+static inline void ioreg_write64(const volatile uint64_t *address,
+                                uint64_t value)
+{
+       asm volatile ("str %0, [%1]" : : "rZ"(value), "r"(address));
+}
 
 static inline void _init_cpufeatures(void)
 {
-- 
2.17.1




 


Rackspace

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