[PATCH v2] virtio: Add aligned ld/st accessors for vring

The generic ld/st*_p() pointer helpers lower to __builtin_memcpy,
which on RISC-V will be expanded to multiple byte-access instructions
rather than a single aligned access by the compiler because it
cannot prove alignment at the call site.

Each cached 16-bit access of a vring field therefore performs several
distinct byte ld/st, which is a memory-tearing hazard for fields that
the guest may access concurrently — most notably avail->idx, where we
find the guest can write a new value between the individual byte loads
and produce a torn read that never existed in memory, as seen in logs
like:
  "Guest moved used index from 49417 to 49919"
Here, 49919 (binary 1100 0010-1111 1111) is incorrectly assembled from
the lower byte of 49663 (1100 0001-1111 1111) and the upper byte of
49664 (1100 0010-0000 0000).

Add a parallel set of _aligned cached accessors so the fast (RAM) path
emits a single aligned load instruction, eliminating the tearing window.

Callers MUST ensure @addr is naturally aligned to the access size before
invoking the _aligned helpers; the virtio vring layout guarantees this
for avail->idx and other naturally-aligned fields.

This patch fixes the memory-tearing hazard while also improves performance.

Signed-off-by: BillXiang <[email protected]>
---
 hw/virtio/virtio.c                            |  8 +-
 include/qemu/bswap.h                          | 20 +++++
 include/system/memory_cached.h                | 16 ++++
 .../system/memory_ldst_cached_aligned.h.inc   | 77 +++++++++++++++++++
 4 files changed, 117 insertions(+), 4 deletions(-)
 create mode 100644 include/system/memory_ldst_cached_aligned.h.inc

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index daa5607..f796bdd 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -223,9 +223,9 @@ static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
                                                hwaddr pa)
 {
     if (virtio_vdev_is_big_endian(vdev)) {
-        return lduw_be_phys_cached(cache, pa);
+        return lduw_be_phys_cached_aligned(cache, pa);
     }
-    return lduw_le_phys_cached(cache, pa);
+    return lduw_le_phys_cached_aligned(cache, pa);
 }
 
 static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
@@ -233,9 +233,9 @@ static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
                                           hwaddr pa, uint16_t value)
 {
     if (virtio_vdev_is_big_endian(vdev)) {
-        stw_be_phys_cached(cache, pa, value);
+        stw_be_phys_cached_aligned(cache, pa, value);
     } else {
-        stw_le_phys_cached(cache, pa, value);
+        stw_le_phys_cached_aligned(cache, pa, value);
     }
 }
 
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 387d65c..be9913c 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -301,6 +301,11 @@ static inline int lduw_le_p(const void *ptr)
     return (uint16_t)le_bswap(lduw_he_p(ptr), 16);
 }
 
+static inline int lduw_le_p_aligned(const void *ptr)
+{
+    return le16_to_cpu(*(uint16_t *)ptr);
+}
+
 static inline int ldsw_le_p(const void *ptr)
 {
     return (int16_t)le_bswap(lduw_he_p(ptr), 16);
@@ -321,6 +326,11 @@ static inline void stw_le_p(void *ptr, uint16_t v)
     stw_he_p(ptr, le_bswap(v, 16));
 }
 
+static inline void stw_le_p_aligned(void *ptr, uint16_t v)
+{
+    *(uint16_t *)ptr = cpu_to_le16(v);
+}
+
 static inline void st24_le_p(void *ptr, uint32_t v)
 {
     st24_he_p(ptr, le_bswap24(v));
@@ -341,6 +351,11 @@ static inline int lduw_be_p(const void *ptr)
     return (uint16_t)be_bswap(lduw_he_p(ptr), 16);
 }
 
+static inline int lduw_be_p_aligned(const void *ptr)
+{
+    return be16_to_cpu(*(uint16_t *)ptr);
+}
+
 static inline int ldsw_be_p(const void *ptr)
 {
     return (int16_t)be_bswap(lduw_he_p(ptr), 16);
@@ -361,6 +376,11 @@ static inline void stw_be_p(void *ptr, uint16_t v)
     stw_he_p(ptr, be_bswap(v, 16));
 }
 
+static inline void stw_be_p_aligned(void *ptr, uint16_t v)
+{
+    *(uint16_t *)ptr = cpu_to_be16(v);
+}
+
 static inline void st24_be_p(void *ptr, uint32_t v)
 {
     st24_he_p(ptr, be_bswap24(v));
diff --git a/include/system/memory_cached.h b/include/system/memory_cached.h
index 09d4682..6884f77 100644
--- a/include/system/memory_cached.h
+++ b/include/system/memory_cached.h
@@ -96,6 +96,22 @@ void address_space_stb_cached(const MemoryRegionCache *cache,
 #define ARG1_DECL    const MemoryRegionCache *cache
 #include "system/memory_ldst_phys.h.inc"
 
+/*
+ * Aligned counterparts of the cached load accessors.
+ *
+ * The fast path (direct RAM access) uses the ld*_p_aligned() pointer helpers,
+ * which assume the host pointer is naturally aligned to the access size and
+ * therefore let the compiler emit an aligned load instruction.
+ *
+ * Callers MUST ensure @addr is aligned to the access size before invoking
+ * these helpers; otherwise the behavior is undefined.
+ */
+#define ENDIANNESS   _le
+#include "system/memory_ldst_cached_aligned.h.inc"
+
+#define ENDIANNESS   _be
+#include "system/memory_ldst_cached_aligned.h.inc"
+
 /**
  * address_space_cache_init: prepare for repeated access to a physical
  *                           memory region
diff --git a/include/system/memory_ldst_cached_aligned.h.inc b/include/system/memory_ldst_cached_aligned.h.inc
new file mode 100644
index 0000000..62610d3
--- /dev/null
+++ b/include/system/memory_ldst_cached_aligned.h.inc
@@ -0,0 +1,77 @@
+/*
+ *  Aligned Memory access templates for MemoryRegionCache
+ *
+ *  Callers MUST ensure @addr is aligned to the access size before invoking
+ *  these helpers; otherwise the behavior is undefined.
+ *
+ *  Copyright (c) 2018 Red Hat, Inc.
+ *  Copyright (c) 2018 LanxinComputing, Ltd.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#define ADDRESS_SPACE_LD_CACHED_ALIGNED(size) \
+    glue(glue(address_space_ld, size), glue(ENDIANNESS, _cached_aligned))
+#define ADDRESS_SPACE_LD_CACHED_SLOW(size) \
+    glue(glue(address_space_ld, size), glue(ENDIANNESS, _cached_slow))
+#define LD_P_ALIGNED(size) \
+    glue(glue(ld, size), glue(ENDIANNESS, _p_aligned))
+#define LD_PHYS_CACHED_ALIGNED(size) \
+    glue(glue(ld, size), glue(ENDIANNESS, glue(_phys, _cached_aligned)))
+
+static inline uint16_t ADDRESS_SPACE_LD_CACHED_ALIGNED(uw)(MemoryRegionCache *cache,
+    hwaddr addr, MemTxAttrs attrs, MemTxResult *result)
+{
+    assert(addr < cache->len && 2 <= cache->len - addr);
+    fuzz_dma_read_cb(cache->xlat + addr, 2, cache->mrs.mr);
+    if (likely(cache->ptr)) {
+        return LD_P_ALIGNED(uw)(cache->ptr + addr);
+    } else {
+        return ADDRESS_SPACE_LD_CACHED_SLOW(uw)(cache, addr, attrs, result);
+    }
+}
+
+static inline uint16_t LD_PHYS_CACHED_ALIGNED(uw)(MemoryRegionCache *cache,
+                                                  hwaddr addr)
+{
+    return ADDRESS_SPACE_LD_CACHED_ALIGNED(uw)(cache, addr,
+                                              MEMTXATTRS_UNSPECIFIED, NULL);
+}
+
+#undef ADDRESS_SPACE_LD_CACHED_ALIGNED
+#undef ADDRESS_SPACE_LD_CACHED_SLOW
+#undef LD_P_ALIGNED
+#undef LD_PHYS_CACHED_ALIGNED
+
+#define ADDRESS_SPACE_ST_CACHED_ALIGNED(size) \
+    glue(glue(address_space_st, size), glue(ENDIANNESS, _cached_aligned))
+#define ADDRESS_SPACE_ST_CACHED_SLOW(size) \
+    glue(glue(address_space_st, size), glue(ENDIANNESS, _cached_slow))
+#define ST_P_ALIGNED(size) \
+    glue(glue(st, size), glue(ENDIANNESS, _p_aligned))
+#define ST_PHYS_CACHED_ALIGNED(size) \
+    glue(glue(st, size), glue(ENDIANNESS, glue(_phys, _cached_aligned)))
+
+static inline void ADDRESS_SPACE_ST_CACHED_ALIGNED(w)(const MemoryRegionCache *cache,
+    hwaddr addr, uint16_t val, MemTxAttrs attrs, MemTxResult *result)
+{
+    assert(addr < cache->len && 2 <= cache->len - addr);
+    if (likely(cache->ptr)) {
+        ST_P_ALIGNED(w)(cache->ptr + addr, val);
+    } else {
+        ADDRESS_SPACE_ST_CACHED_SLOW(w)(cache, addr, val, attrs, result);
+    }
+}
+
+static inline void ST_PHYS_CACHED_ALIGNED(w)(MemoryRegionCache *cache,
+    hwaddr addr, uint16_t val)
+{
+    ADDRESS_SPACE_ST_CACHED_ALIGNED(w)(cache, addr, val, MEMTXATTRS_UNSPECIFIED, NULL);
+}
+
+#undef ADDRESS_SPACE_ST_CACHED_ALIGNED
+#undef ADDRESS_SPACE_ST_CACHED_SLOW
+#undef ST_P_ALIGNED
+#undef ST_PHYS_CACHED_ALIGNED
+
+#undef ENDIANNESS
-- 
2.53.0