hw/virtio/virtio.c | 8 +- include/qemu/bswap.h | 41 ++++++++++ include/system/memory_cached.h | 16 ++++ .../system/memory_ldst_cached_aligned.h.inc | 76 +++++++++++++++++++ 4 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 include/system/memory_ldst_cached_aligned.h.inc
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 <xiangwencheng@lanxincomputing.com>
---
hw/virtio/virtio.c | 8 +-
include/qemu/bswap.h | 41 ++++++++++
include/system/memory_cached.h | 16 ++++
.../system/memory_ldst_cached_aligned.h.inc | 76 +++++++++++++++++++
4 files changed, 137 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..c23f5a2 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -255,6 +255,20 @@ static inline int lduw_he_p(const void *ptr)
return r;
}
+static inline int lduw_he_p_aligned(const void *ptr)
+{
+ uint16_t r;
+ __builtin_memcpy(&r, __builtin_assume_aligned(ptr, sizeof(r)), sizeof(r));
+ return r;
+}
+
static inline int ldsw_he_p(const void *ptr)
{
int16_t r;
@@ -267,6 +281,11 @@ static inline void stw_he_p(void *ptr, uint16_t v)
__builtin_memcpy(ptr, &v, sizeof(v));
}
+static inline void stw_he_p_aligned(void *ptr, uint16_t v)
+{
+ __builtin_memcpy(__builtin_assume_aligned(ptr, sizeof(v)), &v, sizeof(v));
+}
+
static inline void st24_he_p(void *ptr, uint32_t v)
{
__builtin_memcpy(ptr, &v, 3);
@@ -301,6 +320,12 @@ 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 (uint16_t)le_bswap(lduw_he_p_aligned(ptr), 16);
+}
+
static inline int ldsw_le_p(const void *ptr)
{
return (int16_t)le_bswap(lduw_he_p(ptr), 16);
@@ -321,6 +346,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)
+{
+ stw_he_p_aligned(ptr, le_bswap(v, 16));
+}
+
static inline void st24_le_p(void *ptr, uint32_t v)
{
st24_he_p(ptr, le_bswap24(v));
@@ -341,6 +371,12 @@ 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 (uint16_t)be_bswap(lduw_he_p_aligned(ptr), 16);
+}
+
static inline int ldsw_be_p(const void *ptr)
{
return (int16_t)be_bswap(lduw_he_p(ptr), 16);
@@ -361,6 +397,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)
+{
+ stw_he_p_aligned(ptr, be_bswap(v, 16));
+}
+
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..cfa7a26 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..c21c135
--- /dev/null
+++ b/include/system/memory_ldst_cached_aligned.h.inc
@@ -0,0 +1,76 @@
+/*
+ * 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
On 8/20/26 02:46, BillXiang wrote:
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index 387d65c..c23f5a2 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -255,6 +255,20 @@ static inline int lduw_he_p(const void *ptr)
> return r;
> }
>
> +static inline int lduw_he_p_aligned(const void *ptr)
> +{
> + uint16_t r;
> + __builtin_memcpy(&r, __builtin_assume_aligned(ptr, sizeof(r)), sizeof(r));
> + return r;
> +}
If you know it's aligned, then you don't need anything special: just a normal C memory
access. E.g.
*(uint16_t *)ptr
and then of course no need for a wrapper function.
> +static inline int lduw_le_p_aligned(const void *ptr)
> +{
> + return (uint16_t)le_bswap(lduw_he_p_aligned(ptr), 16);
> +}
And this: le16_to_cpu(*(uint16_t *)ptr)
r~
On 8/20/2026 10:59 PM, Richard Henderson wrote:
> On 8/20/26 02:46, BillXiang wrote:
>> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
>> index 387d65c..c23f5a2 100644
>> --- a/include/qemu/bswap.h
>> +++ b/include/qemu/bswap.h
>> @@ -255,6 +255,20 @@ static inline int lduw_he_p(const void *ptr)
>> return r;
>> }
>> +static inline int lduw_he_p_aligned(const void *ptr)
>> +{
>> + uint16_t r;
>> + __builtin_memcpy(&r, __builtin_assume_aligned(ptr, sizeof(r)),
>> sizeof(r));
>> + return r;
>> +}
>
> If you know it's aligned, then you don't need anything special: just a
> normal C memory access. E.g.
>
> *(uint16_t *)ptr
>
> and then of course no need for a wrapper function.
Noted, thanks! I’ll resend it with the changes.
>
>> +static inline int lduw_le_p_aligned(const void *ptr)
>> +{
>> + return (uint16_t)le_bswap(lduw_he_p_aligned(ptr), 16);
>> +}
>
> And this: le16_to_cpu(*(uint16_t *)ptr)
>
>
> r~
© 2016 - 2026 Red Hat, Inc.