All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/3] system/memory: Make ram device region directly accessible
@ 2026-07-28  3:17 Gavin Shan
  2026-07-28  3:17 ` [PATCH v5 1/3] system/memory: Use memmove() for directly accessible regions Gavin Shan
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Gavin Shan @ 2026-07-28  3:17 UTC (permalink / raw)
  To: qemu-arm
  Cc: qemu-devel, peterx, mst, philmd, peter.maydell, richard.henderson,
	alex, berrange, philmd, david, clg, pbonzini, phrdina, jugraham,
	liugang24219, dinghui, shan.gavin

All ram device regions was turned to be indirectly accessible by commit
4a2e242bbb ("memory: Don't use memcpy for ram_device regions"). This leads
to a frozen guest where a NVidia GH100 GPU is passed from host. The memory
in its PCI BAR#4 can be allocated as DMA target buffer. qemu has to take
DMA bounce buffer in address_space_map() to cover the DMA request. However,
the bounce buffer size is 4096 bytes only and it's exhaused very quickly
when the guest has significant disk activities on compiling 'cuda-samples'.
The full log and problem description can be found from PATCH[2/3]'s commit
log.

Fix the issue handled in commit 4a2e242bbb by replacing memmove() with newly
added qemu_ram_move() where the aligned and small-sized accesses are handled
by qatomics, and fall back to memmove() otherwise, for the directly accessible
regions. With this, we can revert 4a2e242bbb to make ram device region directly
accessible again and bypass the bounce buffer in address_space_map() where the
guest hang happens.

PATCH[1] replaces memcpy() with memomve() for directly accessible regions
PATCH[2] uses qemu_ram_move() for directly accessible regions
PATCH[3] makes ram device region directly accessible again

Changelog
=========
v4 -> v5:
  * https://lore.kernel.org/qemu-arm/20260727032649.145381-1-gshan@redhat.com/
  * Improved commit log of PATCH[2/3] and comments for
    qemu_ram_move()                                   (Peterm/Peterx)
v3 -> v4:
  * https://lore.kernel.org/qemu-arm/20260616052552.389021-1-gshan@redhat.com/
  * New PATCH[1/3] to replace memcpy() with memmove() (Peterx)
  * Unified qemu_ram_move() for all architectures     (Peterx/Michael)
v2 -> v3:
  * https://lore.kernel.org/qemu-arm/20260615100200.266968-1-gshan@redhat.com/
  * Documentation for qemu_ram_{copy, move}           (Peterm/Michael)
  * Support qemu_ram_move() for overlapped src/dest   (Richard)
  * Use {memcpy, memmove} if step is 16-bytes or more (Michael)
  * Code improvements                                 (Richard/Michael)
v1 -> v2:
  * https://lore.kernel.org/qemu-arm/20260612110307.1264798-1-gshan@redhat.com/
  * Rename address_space_{memcpy, memmove}() to qemu_ram_{copy, move}()
    and move them to physmem.c and memory.h   (Philippe)
  * Use memcpy() and memmove() in qemu_ram_{copy, move}() for the variable
    length case                               (Miachel)
  * Handle unaligned access in qemu_ram_{copy, move}() for all archs
    except i386 and x86_64                    (Richard/Michael)
RFCv1 -> v1:
  * https://lists.nongnu.org/archive/html/qemu-arm/2026-06/msg00307.html
  * Reworked solution based on suggestions from Peter Xu, Peter Maydell
    and Michael S. Tsirkin

Gavin Shan (3):
  system/memory: Use memmove() for directly accessible regions
  system/memory: Use qemu_ram_move() for directly accessible regions
  system/memory: Make ram device region directly accessible

 hw/remote/vfio-user-obj.c |  4 ++--
 include/system/memory.h   | 46 ++++++++++++++++++++++++++++++---------
 system/memory.c           | 41 +---------------------------------
 system/physmem.c          | 43 ++++++++++++++++++++++++++++++++++--
 system/trace-events       |  2 --
 5 files changed, 80 insertions(+), 56 deletions(-)

-- 
2.55.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 1/3] system/memory: Use memmove() for directly accessible regions
  2026-07-28  3:17 [PATCH v5 0/3] system/memory: Make ram device region directly accessible Gavin Shan
@ 2026-07-28  3:17 ` Gavin Shan
  2026-07-28 16:07   ` Philippe Mathieu-Daudé
  2026-07-28  3:17 ` [PATCH v5 2/3] system/memory: Use qemu_ram_move() " Gavin Shan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Gavin Shan @ 2026-07-28  3:17 UTC (permalink / raw)
  To: qemu-arm
  Cc: qemu-devel, peterx, mst, philmd, peter.maydell, richard.henderson,
	alex, berrange, philmd, david, clg, pbonzini, phrdina, jugraham,
	liugang24219, dinghui, shan.gavin

Similar to what's done in commit 4a73aee88140 ("softmmu: Use memmove in
flatview_write_continue"), there are more sites where the overlapping
source and destination buffer are allowed for the directly accessible
regions. Use memmove() in those sites, listed as below.

  hw/remote/vfio-user-obj.c::vfu_object_mr_rw
  include/system/memory.h::address_space_read
  system/physmem.c::flatview_read_continue_step

Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 hw/remote/vfio-user-obj.c | 4 ++--
 include/system/memory.h   | 2 +-
 system/physmem.c          | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index 87fa7b6572..ea50270628 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -375,9 +375,9 @@ static int vfu_object_mr_rw(MemoryRegion *mr, uint8_t *buf, hwaddr offset,
         ram_ptr = memory_region_get_ram_ptr(mr);
 
         if (is_write) {
-            memcpy((ram_ptr + offset), buf, size);
+            memmove((ram_ptr + offset), buf, size);
         } else {
-            memcpy(buf, (ram_ptr + offset), size);
+            memmove(buf, (ram_ptr + offset), size);
         }
 
         return 0;
diff --git a/include/system/memory.h b/include/system/memory.h
index 2192fc9bdc..336d4e84a6 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -2741,7 +2741,7 @@ MemTxResult address_space_read(const AddressSpace *as, hwaddr addr,
             mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
             if (len == l && memory_access_is_direct(mr, false, attrs)) {
                 ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
-                memcpy(buf, ptr, len);
+                memmove(buf, ptr, len);
             } else {
                 result = flatview_read_continue(fv, addr, attrs, buf, len,
                                                 addr1, l, mr);
diff --git a/system/physmem.c b/system/physmem.c
index c21ea92915..2c42e365cb 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3363,7 +3363,7 @@ static MemTxResult flatview_read_continue_step(MemTxAttrs attrs, uint8_t *buf,
         uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l,
                                                false, false);
 
-        memcpy(buf, ram_ptr, *l);
+        memmove(buf, ram_ptr, *l);
 
         return MEMTX_OK;
     }
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 2/3] system/memory: Use qemu_ram_move() for directly accessible regions
  2026-07-28  3:17 [PATCH v5 0/3] system/memory: Make ram device region directly accessible Gavin Shan
  2026-07-28  3:17 ` [PATCH v5 1/3] system/memory: Use memmove() for directly accessible regions Gavin Shan
@ 2026-07-28  3:17 ` Gavin Shan
  2026-07-29 22:35   ` Michael S. Tsirkin
  2026-07-28  3:17 ` [PATCH v5 3/3] system/memory: Make ram device region directly accessible Gavin Shan
  2026-07-28 15:19 ` [PATCH v5 0/3] " Peter Xu
  3 siblings, 1 reply; 12+ messages in thread
From: Gavin Shan @ 2026-07-28  3:17 UTC (permalink / raw)
  To: qemu-arm
  Cc: qemu-devel, peterx, mst, philmd, peter.maydell, richard.henderson,
	alex, berrange, philmd, david, clg, pbonzini, phrdina, jugraham,
	liugang24219, dinghui, shan.gavin

All ram device regions were turned to be indirectly accessible by commit
4a2e242bbb ("memory: Don't use memcpy for ram_device regions"). This leads
to guest hang on attempt to build 'cuda-samples' as reported by Julia. The
guest is started by the following command lines, with GH100 GPU card passed
from the host.

   host$ lspci | grep GH100
   0009:01:00.0 3D controller: NVIDIA Corporation GH100 [GH200 120GB / 480GB] (rev a1)
   host$ /home/sandbox/gavin/qemu.main/build/qemu-system-aarch64            \
         -machine virt,gic-version=host,ras=on,highmem-mmio-size=4T         \
         -accel kvm -cpu host -smp cpus=48 -m size=8G                       \
         -drive file=/home/gavin/sandbox/images/disk.qcow2,if=none,id=d0    \
         -device virtio-blk-pci,id=vb0,bus=pcie.0,drive=d0,num-queues=4     \
         -device vfio-pci-nohotplug,host=0009:01:00.0,bus=pcie.1.0
           :
   guest$ cd cuda-samples/build
   guest$ make -j 20 clean
   guest$ make -j 20
           :
   [ 54%] Linking CUDA executable graphMemoryNodes
   [ 54%] Built target graphMemoryNodes
   <no more output afterwards, guest becomes frozen here>

   guest$ qemu-system-aarch64: virtio: bogus descriptor or out of resources
   [  555.814025] virtio_blk virtio0: [vda] new size: 268435456 512-byte logical blocks (137 GB/128 GiB)

When the GPU's driver (NVidia open driver) is loaded on guest bootup,
the memory blocks residing in the PCI BAR#4 of the GH100 GPU card can
be presented to the guest through memory hot-add. The page cache can
then be allocated from the hot added memory blocks when cuda-samples
is being built. Afterwards, the page cache is sent to QEMU's virtio-blk
device as part of the DMA request, the bounce buffer has to be used to
accomodate the request as the corresponding memory region (MemoryRegion)
is an indirectly accessible ram device region in qemu. However, the max
bounce bufer size is only 4096 bytes by default and that is exhausted
quickly, leading to a reset on the virtio-blk device and frozen guest
eventually.

  QEMU
  ====
  virtio_blk_handle_output
    virtio_blk_handle_vq
      virtio_blk_get_request
        virtqueue_pop
          virtqueue_split_pop
            virtqueue_map_desc
              address_space_map
                memory_access_is_direct         # Return false
                  memory_region_supports_direct_access

  (qemu) info mtree
  memory-region: pci_bridge_pci
    0000000000000000-ffffffffffffffff (prio 0, container): pci_bridge_pci
      0000042000000000-0000043fffffffff (prio 1, i/o): 0009:01:00.0 base BAR 4
        0000042000000000-0000043fffffffff (prio 0, i/o): 0009:01:00.0 BAR 4
          0000042000000000-000004379fffffff (prio 0, ramd): 0009:01:00.0 BAR 4 mmaps[0]

This adds qemu_ram_move() where the aligned and small-sized accesses are
handled by qatomics, and fall back to memmove() otherwise. The memove()
for the directly accessible regions is replaced by qemu_ram_move() so that
the issue covered by commit 4a2e242bbb (MMIO access instructions were
optimized to SSE instructions) is fixed. This makes 'ram_device_mem_ops'
redundant, paving the way to revert that commit to make the ram device
region directly accessible again in the next patch.

Besides, this also fixes the issue of the unexpected frozen reception on
e1000 NIC in the scenario of DPDK due to the wrong Rx queue full indication
caused by the following memcpy(), which is turned to 3 consective 'strb'
instructions to the same location by glibc-2.24+ for aarch64. With this
applied, the syntax of one-byte store is strictly ensured by a one-byte
qatomic set.

  QEMU
  ====
  e1000_receive_iov
    pci_dma_write
      pci_dma_rw
        dma_memory_rw
          dma_memory_rw_relaxed
            address_space_rw
              address_space_write
                flatview_write
                  flatview_write_continue
                    flatview_write_continue_step
                      memcpy    # 3 consective 'strb' instructions

Reported-by: Julia Graham <jugraham@redhat.com>
Reported-by: Liu Gang <liugang24219@sangfor.com.cn>
Reported-by: Ding Hui <dinghui@sangfor.com.cn>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Suggested-by: Peter Xu <peterx@redhat.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
v5: Improved commit log and comments to qemu_ram_move()
---
 hw/remote/vfio-user-obj.c |  4 ++--
 include/system/memory.h   | 35 ++++++++++++++++++++++++++++++-
 system/physmem.c          | 43 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index ea50270628..a0498d218f 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -375,9 +375,9 @@ static int vfu_object_mr_rw(MemoryRegion *mr, uint8_t *buf, hwaddr offset,
         ram_ptr = memory_region_get_ram_ptr(mr);
 
         if (is_write) {
-            memmove((ram_ptr + offset), buf, size);
+            qemu_ram_move((ram_ptr + offset), buf, size);
         } else {
-            memmove(buf, (ram_ptr + offset), size);
+            qemu_ram_move(buf, (ram_ptr + offset), size);
         }
 
         return 0;
diff --git a/include/system/memory.h b/include/system/memory.h
index 336d4e84a6..2965d922de 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -2668,6 +2668,39 @@ void address_space_register_map_client(AddressSpace *as, QEMUBH *bh);
 void address_space_unregister_map_client(AddressSpace *as, QEMUBH *bh);
 
 /* Internal functions, part of the implementation of address_space_read.  */
+
+/**
+ * qemu_ram_move: move data from or to ramblock
+ *
+ * @dst: destination where the data is moved to
+ * @src: source where the data is moved from
+ * @n: length of data to be moved
+ *
+ * Move @n bytes from @src to @dst, the memory areas may overlap. This
+ * provides the same semantics as memmove(), plus an additional stronger
+ * guarantee: if @n is 1, 2 or 4 or 8 bytes, and @src and @dst are both
+ * naturally aligned for that access size, and the memory areas do not
+ * overlap, then both the load and the store will be done as a single
+ * atomic access (with the semantics of qatomic_read() and qatomic_set()).
+ *
+ * This is the underlying function that we use to implement accesses by
+ * a guest vCPU or a device DMA operation to a ram block. The atomic
+ * guarantee is needed for two major cases: (A) When the ram block is
+ * backed by a PCI BAR passed through from a host device (and so it might
+ * be hardware registers that must be accessed exactly once at the right
+ * width); (B) When an emulated device updates a data structure shared in
+ * guest memory with guest software (e.g. a network device's set of tx and
+ * rx descriptor blocks), if a write to memory is accidentally performed
+ * multiple times then it can break the guest code when it busy polls the
+ * guest memory.
+ *
+ * We don't attempt to perform the exact access when it would be unaligned
+ * because this can't be done on all host architectures. Although this is
+ * strictly speaking not doing what would happen on real hardware, we don't
+ * think there are going to be situations where that matters in practice.
+ */
+void qemu_ram_move(void *dst, const void *src, size_t n);
+
 MemTxResult address_space_read_full(const AddressSpace *as, hwaddr addr,
                                     MemTxAttrs attrs, void *buf, hwaddr len);
 MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
@@ -2741,7 +2774,7 @@ MemTxResult address_space_read(const AddressSpace *as, hwaddr addr,
             mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
             if (len == l && memory_access_is_direct(mr, false, attrs)) {
                 ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
-                memmove(buf, ptr, len);
+                qemu_ram_move(buf, ptr, len);
             } else {
                 result = flatview_read_continue(fv, addr, attrs, buf, len,
                                                 addr1, l, mr);
diff --git a/system/physmem.c b/system/physmem.c
index 2c42e365cb..fbe7df2391 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3158,6 +3158,45 @@ void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size)
     invalidate_and_set_dirty(mr, addr, size);
 }
 
+void qemu_ram_move(void *dst, const void *src, size_t n)
+{
+    uintptr_t test, len;
+
+    if (src == dst || n == 0) {
+        return;
+    }
+
+    /*
+     * Maximal length of aligned access that are determined by @src,
+     * @dst and @n
+     */
+    test = (uintptr_t)src | (uintptr_t)dst | n;
+    len = test & -test;
+
+    /* Overlapping buffers, unaligned or oversized access */
+    if (n > 8 || len != n) {
+        memmove(dst, src, n);
+        return;
+    }
+
+    switch (len) {
+    case 1:
+        qatomic_set((uint8_t *)dst, qatomic_read((uint8_t *)src));
+        break;
+    case 2:
+        qatomic_set((uint16_t *)dst, qatomic_read((uint16_t *)src));
+        break;
+    case 4:
+        qatomic_set((uint32_t *)dst, qatomic_read((uint32_t *)src));
+        break;
+    case 8:
+        qatomic_set((uint64_t *)dst, qatomic_read((uint64_t *)src));
+        break;
+    default:
+        g_assert_not_reached();
+    }
+}
+
 int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
 {
     unsigned access_size_max = mr->ops->valid.max_access_size;
@@ -3270,7 +3309,7 @@ static MemTxResult flatview_write_continue_step(MemTxAttrs attrs,
         uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l,
                                                false, true);
 
-        memmove(ram_ptr, buf, *l);
+        qemu_ram_move(ram_ptr, buf, *l);
         invalidate_and_set_dirty(mr, mr_addr, *l);
 
         return MEMTX_OK;
@@ -3363,7 +3402,7 @@ static MemTxResult flatview_read_continue_step(MemTxAttrs attrs, uint8_t *buf,
         uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l,
                                                false, false);
 
-        memmove(buf, ram_ptr, *l);
+        qemu_ram_move(buf, ram_ptr, *l);
 
         return MEMTX_OK;
     }
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 3/3] system/memory: Make ram device region directly accessible
  2026-07-28  3:17 [PATCH v5 0/3] system/memory: Make ram device region directly accessible Gavin Shan
  2026-07-28  3:17 ` [PATCH v5 1/3] system/memory: Use memmove() for directly accessible regions Gavin Shan
  2026-07-28  3:17 ` [PATCH v5 2/3] system/memory: Use qemu_ram_move() " Gavin Shan
@ 2026-07-28  3:17 ` Gavin Shan
  2026-07-28 15:19 ` [PATCH v5 0/3] " Peter Xu
  3 siblings, 0 replies; 12+ messages in thread
From: Gavin Shan @ 2026-07-28  3:17 UTC (permalink / raw)
  To: qemu-arm
  Cc: qemu-devel, peterx, mst, philmd, peter.maydell, richard.henderson,
	alex, berrange, philmd, david, clg, pbonzini, phrdina, jugraham,
	liugang24219, dinghui, shan.gavin

This basically reverts 4a2e242bbb30 ("memory: Don't use memcpy for
ram_device regions") to make ram device region directly accessible
again. With this, the bounce buffer is bypassed in address_space_map()
when a ram device region is involved, potentially avoid to overrun
the (small) bounce buffer.

Reported-by: Julia Graham <jugraham@redhat.com>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Suggested-by: Peter Xu <peterx@redhat.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/system/memory.h | 11 ++---------
 system/memory.c         | 41 +----------------------------------------
 system/trace-events     |  2 --
 3 files changed, 3 insertions(+), 51 deletions(-)

diff --git a/include/system/memory.h b/include/system/memory.h
index 2965d922de..dbc17741ac 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -2718,15 +2718,8 @@ static inline bool memory_region_supports_direct_access(const MemoryRegion *mr)
     if (memory_region_is_romd(mr)) {
         return true;
     }
-    if (!memory_region_is_ram(mr)) {
-        return false;
-    }
-    /*
-     * RAM DEVICE regions can be accessed directly using memcpy, but it might
-     * be MMIO and access using mempy can be wrong (e.g., using instructions not
-     * intended for MMIO access). So we treat this as IO.
-     */
-    return !memory_region_is_ram_device(mr);
+
+    return memory_region_is_ram(mr);
 }
 
 static inline bool memory_access_is_direct(const MemoryRegion *mr,
diff --git a/system/memory.c b/system/memory.c
index 5fc36708ec..da710bbade 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1364,43 +1364,6 @@ const MemoryRegionOps unassigned_mem_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static uint64_t memory_region_ram_device_read(void *opaque,
-                                              hwaddr addr, unsigned size)
-{
-    MemoryRegion *mr = opaque;
-    uint64_t data = ldn_he_p(mr->ram_block->host + addr, size);
-
-    trace_memory_region_ram_device_read(get_cpu_index(), mr, addr, data, size);
-
-    return data;
-}
-
-static void memory_region_ram_device_write(void *opaque, hwaddr addr,
-                                           uint64_t data, unsigned size)
-{
-    MemoryRegion *mr = opaque;
-
-    trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
-
-    stn_he_p(mr->ram_block->host + addr, size, data);
-}
-
-static const MemoryRegionOps ram_device_mem_ops = {
-    .read = memory_region_ram_device_read,
-    .write = memory_region_ram_device_write,
-    .endianness = HOST_BIG_ENDIAN ? DEVICE_BIG_ENDIAN : DEVICE_LITTLE_ENDIAN,
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 8,
-        .unaligned = true,
-    },
-    .impl = {
-        .min_access_size = 1,
-        .max_access_size = 8,
-        .unaligned = true,
-    },
-};
-
 bool memory_region_access_valid(MemoryRegion *mr,
                                 hwaddr addr,
                                 unsigned size,
@@ -1692,10 +1655,8 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, Object *owner,
                                        const char *name, uint64_t size,
                                        void *ptr)
 {
-    memory_region_init_io(mr, owner, &ram_device_mem_ops, mr, name, size);
-    mr->ram = true;
+    memory_region_init_ram_ptr(mr, owner, name, size, ptr);
     mr->ram_device = true;
-    memory_region_set_ram_ptr(mr, size, ptr);
 }
 
 void memory_region_init_alias(MemoryRegion *mr, Object *owner,
diff --git a/system/trace-events b/system/trace-events
index 51b4a4679a..d483b31419 100644
--- a/system/trace-events
+++ b/system/trace-events
@@ -20,8 +20,6 @@ memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, u
 memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size, const char *name) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u name '%s'"
 memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
-memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
-memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_sync_dirty(const char *mr, const char *listener, int global) "mr '%s' listener '%s' synced (global=%d)"
 flatview_new(void *view, void *root) "%p (root %p)"
 flatview_destroy(void *view, void *root) "%p (root %p)"
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/3] system/memory: Make ram device region directly accessible
  2026-07-28  3:17 [PATCH v5 0/3] system/memory: Make ram device region directly accessible Gavin Shan
                   ` (2 preceding siblings ...)
  2026-07-28  3:17 ` [PATCH v5 3/3] system/memory: Make ram device region directly accessible Gavin Shan
@ 2026-07-28 15:19 ` Peter Xu
  3 siblings, 0 replies; 12+ messages in thread
From: Peter Xu @ 2026-07-28 15:19 UTC (permalink / raw)
  To: Gavin Shan
  Cc: qemu-arm, qemu-devel, mst, philmd, peter.maydell,
	richard.henderson, alex, berrange, philmd, david, clg, pbonzini,
	phrdina, jugraham, liugang24219, dinghui, shan.gavin

On Tue, Jul 28, 2026 at 01:17:28PM +1000, Gavin Shan wrote:
> All ram device regions was turned to be indirectly accessible by commit
> 4a2e242bbb ("memory: Don't use memcpy for ram_device regions"). This leads
> to a frozen guest where a NVidia GH100 GPU is passed from host. The memory
> in its PCI BAR#4 can be allocated as DMA target buffer. qemu has to take
> DMA bounce buffer in address_space_map() to cover the DMA request. However,
> the bounce buffer size is 4096 bytes only and it's exhaused very quickly
> when the guest has significant disk activities on compiling 'cuda-samples'.
> The full log and problem description can be found from PATCH[2/3]'s commit
> log.
> 
> Fix the issue handled in commit 4a2e242bbb by replacing memmove() with newly
> added qemu_ram_move() where the aligned and small-sized accesses are handled
> by qatomics, and fall back to memmove() otherwise, for the directly accessible
> regions. With this, we can revert 4a2e242bbb to make ram device region directly
> accessible again and bypass the bounce buffer in address_space_map() where the
> guest hang happens.
> 
> PATCH[1] replaces memcpy() with memomve() for directly accessible regions
> PATCH[2] uses qemu_ram_move() for directly accessible regions
> PATCH[3] makes ram device region directly accessible again

Queued for 11.2, with fixups suggested by PeterM in v4 discussions:

https://lore.kernel.org/qemu-devel/CAFEAcA-y+vNK2u-rSq+SVjNZrhO2=BsUysEw2PtCdCZ=qYx2bw@mail.gmail.com/

Fixup:

diff --git a/include/system/memory.h b/include/system/memory.h
index d5fca96cea..16bf04ef07 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -2677,9 +2677,9 @@ void address_space_unregister_map_client(AddressSpace *as, QEMUBH *bh);
  * Move @n bytes from @src to @dst, the memory areas may overlap. This
  * provides the same semantics as memmove(), plus an additional stronger
  * guarantee: if @n is 1, 2 or 4 or 8 bytes, and @src and @dst are both
- * naturally aligned for that access size, and the memory areas do not
- * overlap, then both the load and the store will be done as a single
- * atomic access (with the semantics of qatomic_read() and qatomic_set()).
+ * naturally aligned for that access size, then both the load and the store
+ * will be done as a single atomic access (with the semantics of
+ * qatomic_read() and qatomic_set()).
  *
  * This is the underlying function that we use to implement accesses by
  * a guest vCPU or a device DMA operation to a ram block. The atomic
diff --git a/system/physmem.c b/system/physmem.c
index fbe7df2391..2f37cbeb07 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3162,13 +3162,18 @@ void qemu_ram_move(void *dst, const void *src, size_t n)
 {
     uintptr_t test, len;
 
-    if (src == dst || n == 0) {
+    if (n == 0) {
         return;
     }
 
     /*
-     * Maximal length of aligned access that are determined by @src,
-     * @dst and @n
+     * Calculate "the lowest set bit" over @src, @dst and @n, result put
+     * into @len (which guarantees a power-of-two).  With that and the
+     * later check (len!=n), it makes sure that we will only do the atomic
+     * ops when:
+     *
+     * (1) @n is a power-of-two
+     * (2) @src and @dst addresses are both aligned to @n
      */
     test = (uintptr_t)src | (uintptr_t)dst | n;
     len = test & -test;

-- 
Peter Xu



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 1/3] system/memory: Use memmove() for directly accessible regions
  2026-07-28  3:17 ` [PATCH v5 1/3] system/memory: Use memmove() for directly accessible regions Gavin Shan
@ 2026-07-28 16:07   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-07-28 16:07 UTC (permalink / raw)
  To: Gavin Shan, qemu-arm
  Cc: qemu-devel, peterx, mst, peter.maydell, richard.henderson, alex,
	berrange, philmd, david, clg, pbonzini, phrdina, jugraham,
	liugang24219, dinghui, shan.gavin

On 28/7/26 05:17, Gavin Shan wrote:
> Similar to what's done in commit 4a73aee88140 ("softmmu: Use memmove in
> flatview_write_continue"), there are more sites where the overlapping
> source and destination buffer are allowed for the directly accessible
> regions. Use memmove() in those sites, listed as below.
> 
>    hw/remote/vfio-user-obj.c::vfu_object_mr_rw
>    include/system/memory.h::address_space_read
>    system/physmem.c::flatview_read_continue_step
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
>   hw/remote/vfio-user-obj.c | 4 ++--
>   include/system/memory.h   | 2 +-
>   system/physmem.c          | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] system/memory: Use qemu_ram_move() for directly accessible regions
  2026-07-28  3:17 ` [PATCH v5 2/3] system/memory: Use qemu_ram_move() " Gavin Shan
@ 2026-07-29 22:35   ` Michael S. Tsirkin
  2026-07-30  6:26     ` Gavin Shan
  2026-07-30  9:24     ` Peter Maydell
  0 siblings, 2 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2026-07-29 22:35 UTC (permalink / raw)
  To: Gavin Shan
  Cc: qemu-arm, qemu-devel, peterx, philmd, peter.maydell,
	richard.henderson, alex, berrange, philmd, david, clg, pbonzini,
	phrdina, jugraham, liugang24219, dinghui, shan.gavin

On Tue, Jul 28, 2026 at 01:17:30PM +1000, Gavin Shan wrote:
> All ram device regions were turned to be indirectly accessible by commit
> 4a2e242bbb ("memory: Don't use memcpy for ram_device regions"). This leads
> to guest hang on attempt to build 'cuda-samples' as reported by Julia. The
> guest is started by the following command lines, with GH100 GPU card passed
> from the host.
> 
>    host$ lspci | grep GH100
>    0009:01:00.0 3D controller: NVIDIA Corporation GH100 [GH200 120GB / 480GB] (rev a1)
>    host$ /home/sandbox/gavin/qemu.main/build/qemu-system-aarch64            \
>          -machine virt,gic-version=host,ras=on,highmem-mmio-size=4T         \
>          -accel kvm -cpu host -smp cpus=48 -m size=8G                       \
>          -drive file=/home/gavin/sandbox/images/disk.qcow2,if=none,id=d0    \
>          -device virtio-blk-pci,id=vb0,bus=pcie.0,drive=d0,num-queues=4     \
>          -device vfio-pci-nohotplug,host=0009:01:00.0,bus=pcie.1.0
>            :
>    guest$ cd cuda-samples/build
>    guest$ make -j 20 clean
>    guest$ make -j 20
>            :
>    [ 54%] Linking CUDA executable graphMemoryNodes
>    [ 54%] Built target graphMemoryNodes
>    <no more output afterwards, guest becomes frozen here>
> 
>    guest$ qemu-system-aarch64: virtio: bogus descriptor or out of resources
>    [  555.814025] virtio_blk virtio0: [vda] new size: 268435456 512-byte logical blocks (137 GB/128 GiB)
> 
> When the GPU's driver (NVidia open driver) is loaded on guest bootup,
> the memory blocks residing in the PCI BAR#4 of the GH100 GPU card can
> be presented to the guest through memory hot-add. The page cache can
> then be allocated from the hot added memory blocks when cuda-samples
> is being built. Afterwards, the page cache is sent to QEMU's virtio-blk
> device as part of the DMA request, the bounce buffer has to be used to
> accomodate the request as the corresponding memory region (MemoryRegion)
> is an indirectly accessible ram device region in qemu. However, the max
> bounce bufer size is only 4096 bytes by default and that is exhausted
> quickly, leading to a reset on the virtio-blk device and frozen guest
> eventually.
> 
>   QEMU
>   ====
>   virtio_blk_handle_output
>     virtio_blk_handle_vq
>       virtio_blk_get_request
>         virtqueue_pop
>           virtqueue_split_pop
>             virtqueue_map_desc
>               address_space_map
>                 memory_access_is_direct         # Return false
>                   memory_region_supports_direct_access
> 
>   (qemu) info mtree
>   memory-region: pci_bridge_pci
>     0000000000000000-ffffffffffffffff (prio 0, container): pci_bridge_pci
>       0000042000000000-0000043fffffffff (prio 1, i/o): 0009:01:00.0 base BAR 4
>         0000042000000000-0000043fffffffff (prio 0, i/o): 0009:01:00.0 BAR 4
>           0000042000000000-000004379fffffff (prio 0, ramd): 0009:01:00.0 BAR 4 mmaps[0]
> 
> This adds qemu_ram_move() where the aligned and small-sized accesses are
> handled by qatomics, and fall back to memmove() otherwise. The memove()
> for the directly accessible regions is replaced by qemu_ram_move() so that
> the issue covered by commit 4a2e242bbb (MMIO access instructions were
> optimized to SSE instructions) is fixed. This makes 'ram_device_mem_ops'
> redundant, paving the way to revert that commit to make the ram device
> region directly accessible again in the next patch.
> 
> Besides, this also fixes the issue of the unexpected frozen reception on
> e1000 NIC in the scenario of DPDK due to the wrong Rx queue full indication
> caused by the following memcpy(), which is turned to 3 consective 'strb'
> instructions to the same location by glibc-2.24+ for aarch64. With this
> applied, the syntax of one-byte store is strictly ensured by a one-byte
> qatomic set.
> 
>   QEMU
>   ====
>   e1000_receive_iov
>     pci_dma_write
>       pci_dma_rw
>         dma_memory_rw
>           dma_memory_rw_relaxed
>             address_space_rw
>               address_space_write
>                 flatview_write
>                   flatview_write_continue
>                     flatview_write_continue_step
>                       memcpy    # 3 consective 'strb' instructions
> 
> Reported-by: Julia Graham <jugraham@redhat.com>
> Reported-by: Liu Gang <liugang24219@sangfor.com.cn>
> Reported-by: Ding Hui <dinghui@sangfor.com.cn>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> v5: Improved commit log and comments to qemu_ram_move()
> ---
>  hw/remote/vfio-user-obj.c |  4 ++--
>  include/system/memory.h   | 35 ++++++++++++++++++++++++++++++-
>  system/physmem.c          | 43 +++++++++++++++++++++++++++++++++++++--
>  3 files changed, 77 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
> index ea50270628..a0498d218f 100644
> --- a/hw/remote/vfio-user-obj.c
> +++ b/hw/remote/vfio-user-obj.c
> @@ -375,9 +375,9 @@ static int vfu_object_mr_rw(MemoryRegion *mr, uint8_t *buf, hwaddr offset,
>          ram_ptr = memory_region_get_ram_ptr(mr);
>  
>          if (is_write) {
> -            memmove((ram_ptr + offset), buf, size);
> +            qemu_ram_move((ram_ptr + offset), buf, size);
>          } else {
> -            memmove(buf, (ram_ptr + offset), size);
> +            qemu_ram_move(buf, (ram_ptr + offset), size);
>          }
>  
>          return 0;
> diff --git a/include/system/memory.h b/include/system/memory.h
> index 336d4e84a6..2965d922de 100644
> --- a/include/system/memory.h
> +++ b/include/system/memory.h
> @@ -2668,6 +2668,39 @@ void address_space_register_map_client(AddressSpace *as, QEMUBH *bh);
>  void address_space_unregister_map_client(AddressSpace *as, QEMUBH *bh);
>  
>  /* Internal functions, part of the implementation of address_space_read.  */
> +
> +/**
> + * qemu_ram_move: move data from or to ramblock
> + *
> + * @dst: destination where the data is moved to
> + * @src: source where the data is moved from
> + * @n: length of data to be moved
> + *
> + * Move @n bytes from @src to @dst, the memory areas may overlap. This
> + * provides the same semantics as memmove(), plus an additional stronger
> + * guarantee: if @n is 1, 2 or 4 or 8 bytes, and @src and @dst are both
> + * naturally aligned for that access size, and the memory areas do not
> + * overlap, then both the load and the store will be done as a single
> + * atomic access (with the semantics of qatomic_read() and qatomic_set()).
> + *
> + * This is the underlying function that we use to implement accesses by
> + * a guest vCPU or a device DMA operation to a ram block. The atomic
> + * guarantee is needed for two major cases: (A) When the ram block is
> + * backed by a PCI BAR passed through from a host device (and so it might
> + * be hardware registers that must be accessed exactly once at the right
> + * width); (B) When an emulated device updates a data structure shared in
> + * guest memory with guest software (e.g. a network device's set of tx and
> + * rx descriptor blocks), if a write to memory is accidentally performed
> + * multiple times then it can break the guest code when it busy polls the
> + * guest memory.
> + *
> + * We don't attempt to perform the exact access when it would be unaligned
> + * because this can't be done on all host architectures. Although this is
> + * strictly speaking not doing what would happen on real hardware, we don't
> + * think there are going to be situations where that matters in practice.
> + */
> +void qemu_ram_move(void *dst, const void *src, size_t n);
> +
>  MemTxResult address_space_read_full(const AddressSpace *as, hwaddr addr,
>                                      MemTxAttrs attrs, void *buf, hwaddr len);
>  MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
> @@ -2741,7 +2774,7 @@ MemTxResult address_space_read(const AddressSpace *as, hwaddr addr,
>              mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
>              if (len == l && memory_access_is_direct(mr, false, attrs)) {
>                  ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
> -                memmove(buf, ptr, len);
> +                qemu_ram_move(buf, ptr, len);
>              } else {
>                  result = flatview_read_continue(fv, addr, attrs, buf, len,
>                                                  addr1, l, mr);
> diff --git a/system/physmem.c b/system/physmem.c
> index 2c42e365cb..fbe7df2391 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -3158,6 +3158,45 @@ void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size)
>      invalidate_and_set_dirty(mr, addr, size);
>  }
>  
> +void qemu_ram_move(void *dst, const void *src, size_t n)
> +{
> +    uintptr_t test, len;
> +
> +    if (src == dst || n == 0) {
> +        return;
> +    }
> +
> +    /*
> +     * Maximal length of aligned access that are determined by @src,
> +     * @dst and @n
> +     */
> +    test = (uintptr_t)src | (uintptr_t)dst | n;
> +    len = test & -test;
> +
> +    /* Overlapping buffers, unaligned or oversized access */
> +    if (n > 8 || len != n) {
> +        memmove(dst, src, n);
> +        return;
> +    }


This part I don't get exactly. It's fine on most arches I think.
x86, power...

But here is aarch64 for example:
https://codebrowser.dev/glibc/glibc/sysdeps/aarch64/memcpy.S.html

it says 
   It uses unaligned accesses and branchless sequences to keep the code small,
   simple and improve performance.

so if the point is to get rid of unaligned accesses to BARs, are you sure
it achieves this?




> +
> +    switch (len) {
> +    case 1:
> +        qatomic_set((uint8_t *)dst, qatomic_read((uint8_t *)src));
> +        break;
> +    case 2:
> +        qatomic_set((uint16_t *)dst, qatomic_read((uint16_t *)src));
> +        break;
> +    case 4:
> +        qatomic_set((uint32_t *)dst, qatomic_read((uint32_t *)src));
> +        break;
> +    case 8:
> +        qatomic_set((uint64_t *)dst, qatomic_read((uint64_t *)src));
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +}
> +
>  int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
>  {
>      unsigned access_size_max = mr->ops->valid.max_access_size;
> @@ -3270,7 +3309,7 @@ static MemTxResult flatview_write_continue_step(MemTxAttrs attrs,
>          uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l,
>                                                 false, true);
>  
> -        memmove(ram_ptr, buf, *l);
> +        qemu_ram_move(ram_ptr, buf, *l);
>          invalidate_and_set_dirty(mr, mr_addr, *l);
>  
>          return MEMTX_OK;
> @@ -3363,7 +3402,7 @@ static MemTxResult flatview_read_continue_step(MemTxAttrs attrs, uint8_t *buf,
>          uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l,
>                                                 false, false);
>  
> -        memmove(buf, ram_ptr, *l);
> +        qemu_ram_move(buf, ram_ptr, *l);
>  
>          return MEMTX_OK;
>      }
> -- 
> 2.55.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] system/memory: Use qemu_ram_move() for directly accessible regions
  2026-07-29 22:35   ` Michael S. Tsirkin
@ 2026-07-30  6:26     ` Gavin Shan
  2026-07-30  9:24     ` Peter Maydell
  1 sibling, 0 replies; 12+ messages in thread
From: Gavin Shan @ 2026-07-30  6:26 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-arm, qemu-devel, peterx, philmd, peter.maydell,
	richard.henderson, alex, berrange, philmd, david, clg, pbonzini,
	phrdina, jugraham, liugang24219, dinghui, shan.gavin

On 7/30/26 8:35 AM, Michael S. Tsirkin wrote:
> On Tue, Jul 28, 2026 at 01:17:30PM +1000, Gavin Shan wrote:
>> All ram device regions were turned to be indirectly accessible by commit
>> 4a2e242bbb ("memory: Don't use memcpy for ram_device regions"). This leads
>> to guest hang on attempt to build 'cuda-samples' as reported by Julia. The
>> guest is started by the following command lines, with GH100 GPU card passed
>> from the host.
>>
>>     host$ lspci | grep GH100
>>     0009:01:00.0 3D controller: NVIDIA Corporation GH100 [GH200 120GB / 480GB] (rev a1)
>>     host$ /home/sandbox/gavin/qemu.main/build/qemu-system-aarch64            \
>>           -machine virt,gic-version=host,ras=on,highmem-mmio-size=4T         \
>>           -accel kvm -cpu host -smp cpus=48 -m size=8G                       \
>>           -drive file=/home/gavin/sandbox/images/disk.qcow2,if=none,id=d0    \
>>           -device virtio-blk-pci,id=vb0,bus=pcie.0,drive=d0,num-queues=4     \
>>           -device vfio-pci-nohotplug,host=0009:01:00.0,bus=pcie.1.0
>>             :
>>     guest$ cd cuda-samples/build
>>     guest$ make -j 20 clean
>>     guest$ make -j 20
>>             :
>>     [ 54%] Linking CUDA executable graphMemoryNodes
>>     [ 54%] Built target graphMemoryNodes
>>     <no more output afterwards, guest becomes frozen here>
>>
>>     guest$ qemu-system-aarch64: virtio: bogus descriptor or out of resources
>>     [  555.814025] virtio_blk virtio0: [vda] new size: 268435456 512-byte logical blocks (137 GB/128 GiB)
>>
>> When the GPU's driver (NVidia open driver) is loaded on guest bootup,
>> the memory blocks residing in the PCI BAR#4 of the GH100 GPU card can
>> be presented to the guest through memory hot-add. The page cache can
>> then be allocated from the hot added memory blocks when cuda-samples
>> is being built. Afterwards, the page cache is sent to QEMU's virtio-blk
>> device as part of the DMA request, the bounce buffer has to be used to
>> accomodate the request as the corresponding memory region (MemoryRegion)
>> is an indirectly accessible ram device region in qemu. However, the max
>> bounce bufer size is only 4096 bytes by default and that is exhausted
>> quickly, leading to a reset on the virtio-blk device and frozen guest
>> eventually.
>>
>>    QEMU
>>    ====
>>    virtio_blk_handle_output
>>      virtio_blk_handle_vq
>>        virtio_blk_get_request
>>          virtqueue_pop
>>            virtqueue_split_pop
>>              virtqueue_map_desc
>>                address_space_map
>>                  memory_access_is_direct         # Return false
>>                    memory_region_supports_direct_access
>>
>>    (qemu) info mtree
>>    memory-region: pci_bridge_pci
>>      0000000000000000-ffffffffffffffff (prio 0, container): pci_bridge_pci
>>        0000042000000000-0000043fffffffff (prio 1, i/o): 0009:01:00.0 base BAR 4
>>          0000042000000000-0000043fffffffff (prio 0, i/o): 0009:01:00.0 BAR 4
>>            0000042000000000-000004379fffffff (prio 0, ramd): 0009:01:00.0 BAR 4 mmaps[0]
>>
>> This adds qemu_ram_move() where the aligned and small-sized accesses are
>> handled by qatomics, and fall back to memmove() otherwise. The memove()
>> for the directly accessible regions is replaced by qemu_ram_move() so that
>> the issue covered by commit 4a2e242bbb (MMIO access instructions were
>> optimized to SSE instructions) is fixed. This makes 'ram_device_mem_ops'
>> redundant, paving the way to revert that commit to make the ram device
>> region directly accessible again in the next patch.
>>
>> Besides, this also fixes the issue of the unexpected frozen reception on
>> e1000 NIC in the scenario of DPDK due to the wrong Rx queue full indication
>> caused by the following memcpy(), which is turned to 3 consective 'strb'
>> instructions to the same location by glibc-2.24+ for aarch64. With this
>> applied, the syntax of one-byte store is strictly ensured by a one-byte
>> qatomic set.
>>
>>    QEMU
>>    ====
>>    e1000_receive_iov
>>      pci_dma_write
>>        pci_dma_rw
>>          dma_memory_rw
>>            dma_memory_rw_relaxed
>>              address_space_rw
>>                address_space_write
>>                  flatview_write
>>                    flatview_write_continue
>>                      flatview_write_continue_step
>>                        memcpy    # 3 consective 'strb' instructions
>>
>> Reported-by: Julia Graham <jugraham@redhat.com>
>> Reported-by: Liu Gang <liugang24219@sangfor.com.cn>
>> Reported-by: Ding Hui <dinghui@sangfor.com.cn>
>> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
>> Suggested-by: Peter Xu <peterx@redhat.com>
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> v5: Improved commit log and comments to qemu_ram_move()
>> ---
>>   hw/remote/vfio-user-obj.c |  4 ++--
>>   include/system/memory.h   | 35 ++++++++++++++++++++++++++++++-
>>   system/physmem.c          | 43 +++++++++++++++++++++++++++++++++++++--
>>   3 files changed, 77 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
>> index ea50270628..a0498d218f 100644
>> --- a/hw/remote/vfio-user-obj.c
>> +++ b/hw/remote/vfio-user-obj.c
>> @@ -375,9 +375,9 @@ static int vfu_object_mr_rw(MemoryRegion *mr, uint8_t *buf, hwaddr offset,
>>           ram_ptr = memory_region_get_ram_ptr(mr);
>>   
>>           if (is_write) {
>> -            memmove((ram_ptr + offset), buf, size);
>> +            qemu_ram_move((ram_ptr + offset), buf, size);
>>           } else {
>> -            memmove(buf, (ram_ptr + offset), size);
>> +            qemu_ram_move(buf, (ram_ptr + offset), size);
>>           }
>>   
>>           return 0;
>> diff --git a/include/system/memory.h b/include/system/memory.h
>> index 336d4e84a6..2965d922de 100644
>> --- a/include/system/memory.h
>> +++ b/include/system/memory.h
>> @@ -2668,6 +2668,39 @@ void address_space_register_map_client(AddressSpace *as, QEMUBH *bh);
>>   void address_space_unregister_map_client(AddressSpace *as, QEMUBH *bh);
>>   
>>   /* Internal functions, part of the implementation of address_space_read.  */
>> +
>> +/**
>> + * qemu_ram_move: move data from or to ramblock
>> + *
>> + * @dst: destination where the data is moved to
>> + * @src: source where the data is moved from
>> + * @n: length of data to be moved
>> + *
>> + * Move @n bytes from @src to @dst, the memory areas may overlap. This
>> + * provides the same semantics as memmove(), plus an additional stronger
>> + * guarantee: if @n is 1, 2 or 4 or 8 bytes, and @src and @dst are both
>> + * naturally aligned for that access size, and the memory areas do not
>> + * overlap, then both the load and the store will be done as a single
>> + * atomic access (with the semantics of qatomic_read() and qatomic_set()).
>> + *
>> + * This is the underlying function that we use to implement accesses by
>> + * a guest vCPU or a device DMA operation to a ram block. The atomic
>> + * guarantee is needed for two major cases: (A) When the ram block is
>> + * backed by a PCI BAR passed through from a host device (and so it might
>> + * be hardware registers that must be accessed exactly once at the right
>> + * width); (B) When an emulated device updates a data structure shared in
>> + * guest memory with guest software (e.g. a network device's set of tx and
>> + * rx descriptor blocks), if a write to memory is accidentally performed
>> + * multiple times then it can break the guest code when it busy polls the
>> + * guest memory.
>> + *
>> + * We don't attempt to perform the exact access when it would be unaligned
>> + * because this can't be done on all host architectures. Although this is
>> + * strictly speaking not doing what would happen on real hardware, we don't
>> + * think there are going to be situations where that matters in practice.
>> + */
>> +void qemu_ram_move(void *dst, const void *src, size_t n);
>> +
>>   MemTxResult address_space_read_full(const AddressSpace *as, hwaddr addr,
>>                                       MemTxAttrs attrs, void *buf, hwaddr len);
>>   MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
>> @@ -2741,7 +2774,7 @@ MemTxResult address_space_read(const AddressSpace *as, hwaddr addr,
>>               mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
>>               if (len == l && memory_access_is_direct(mr, false, attrs)) {
>>                   ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
>> -                memmove(buf, ptr, len);
>> +                qemu_ram_move(buf, ptr, len);
>>               } else {
>>                   result = flatview_read_continue(fv, addr, attrs, buf, len,
>>                                                   addr1, l, mr);
>> diff --git a/system/physmem.c b/system/physmem.c
>> index 2c42e365cb..fbe7df2391 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -3158,6 +3158,45 @@ void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size)
>>       invalidate_and_set_dirty(mr, addr, size);
>>   }
>>   
>> +void qemu_ram_move(void *dst, const void *src, size_t n)
>> +{
>> +    uintptr_t test, len;
>> +
>> +    if (src == dst || n == 0) {
>> +        return;
>> +    }
>> +
>> +    /*
>> +     * Maximal length of aligned access that are determined by @src,
>> +     * @dst and @n
>> +     */
>> +    test = (uintptr_t)src | (uintptr_t)dst | n;
>> +    len = test & -test;
>> +
>> +    /* Overlapping buffers, unaligned or oversized access */
>> +    if (n > 8 || len != n) {
>> +        memmove(dst, src, n);
>> +        return;
>> +    }
> 
> 
> This part I don't get exactly. It's fine on most arches I think.
> x86, power...
> 
> But here is aarch64 for example:
> https://codebrowser.dev/glibc/glibc/sysdeps/aarch64/memcpy.S.html
> 
> it says
>     It uses unaligned accesses and branchless sequences to keep the code small,
>     simple and improve performance.
> 
> so if the point is to get rid of unaligned accesses to BARs, are you sure
> it achieves this?
> 

No, we don't intend to avoid the unaligned accesses to BARs if they're
existing, even on aarch64. The behavior is consistent to that introduced
by 'ram_device_mem_ops'. When 'ram_device_mem_ops' is in place of upstream
'master' branch, the unaligned access is put through, without splitting
it to aligned accesses because 'ram_device_mem_ops::{valid, impl}.unaligned'
have been set to true.

> 
>> +
>> +    switch (len) {
>> +    case 1:
>> +        qatomic_set((uint8_t *)dst, qatomic_read((uint8_t *)src));
>> +        break;
>> +    case 2:
>> +        qatomic_set((uint16_t *)dst, qatomic_read((uint16_t *)src));
>> +        break;
>> +    case 4:
>> +        qatomic_set((uint32_t *)dst, qatomic_read((uint32_t *)src));
>> +        break;
>> +    case 8:
>> +        qatomic_set((uint64_t *)dst, qatomic_read((uint64_t *)src));
>> +        break;
>> +    default:
>> +        g_assert_not_reached();
>> +    }
>> +}
>> +
>>   int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
>>   {
>>       unsigned access_size_max = mr->ops->valid.max_access_size;
>> @@ -3270,7 +3309,7 @@ static MemTxResult flatview_write_continue_step(MemTxAttrs attrs,
>>           uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l,
>>                                                  false, true);
>>   
>> -        memmove(ram_ptr, buf, *l);
>> +        qemu_ram_move(ram_ptr, buf, *l);
>>           invalidate_and_set_dirty(mr, mr_addr, *l);
>>   
>>           return MEMTX_OK;
>> @@ -3363,7 +3402,7 @@ static MemTxResult flatview_read_continue_step(MemTxAttrs attrs, uint8_t *buf,
>>           uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l,
>>                                                  false, false);
>>   
>> -        memmove(buf, ram_ptr, *l);
>> +        qemu_ram_move(buf, ram_ptr, *l);
>>   
>>           return MEMTX_OK;
>>       }

Thanks,
Gavin




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] system/memory: Use qemu_ram_move() for directly accessible regions
  2026-07-29 22:35   ` Michael S. Tsirkin
  2026-07-30  6:26     ` Gavin Shan
@ 2026-07-30  9:24     ` Peter Maydell
  2026-07-30 10:27       ` Michael S. Tsirkin
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2026-07-30  9:24 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Gavin Shan, qemu-arm, qemu-devel, peterx, philmd,
	richard.henderson, alex, berrange, philmd, david, clg, pbonzini,
	phrdina, jugraham, liugang24219, dinghui, shan.gavin

On Wed, 29 Jul 2026 at 23:35, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Jul 28, 2026 at 01:17:30PM +1000, Gavin Shan wrote:
> > +void qemu_ram_move(void *dst, const void *src, size_t n)
> > +{
> > +    uintptr_t test, len;
> > +
> > +    if (src == dst || n == 0) {
> > +        return;
> > +    }
> > +
> > +    /*
> > +     * Maximal length of aligned access that are determined by @src,
> > +     * @dst and @n
> > +     */
> > +    test = (uintptr_t)src | (uintptr_t)dst | n;
> > +    len = test & -test;
> > +
> > +    /* Overlapping buffers, unaligned or oversized access */
> > +    if (n > 8 || len != n) {
> > +        memmove(dst, src, n);
> > +        return;
> > +    }
>
>
> This part I don't get exactly. It's fine on most arches I think.
> x86, power...
>
> But here is aarch64 for example:
> https://codebrowser.dev/glibc/glibc/sysdeps/aarch64/memcpy.S.html
>
> it says
>    It uses unaligned accesses and branchless sequences to keep the code small,
>    simple and improve performance.
>
> so if the point is to get rid of unaligned accesses to BARs, are you sure
> it achieves this?

The idea is rather just to ignore the unaligned-access-to-not-RAM
case as "only a theoretical issue" and default it to memmove().
If anybody reports an actual problem we can think about it then :-)

-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] system/memory: Use qemu_ram_move() for directly accessible regions
  2026-07-30  9:24     ` Peter Maydell
@ 2026-07-30 10:27       ` Michael S. Tsirkin
  2026-07-30 10:34         ` Peter Maydell
  0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2026-07-30 10:27 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Gavin Shan, qemu-arm, qemu-devel, peterx, philmd,
	richard.henderson, alex, berrange, philmd, david, clg, pbonzini,
	phrdina, jugraham, liugang24219, dinghui, shan.gavin

On Thu, Jul 30, 2026 at 10:24:32AM +0100, Peter Maydell wrote:
> On Wed, 29 Jul 2026 at 23:35, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Jul 28, 2026 at 01:17:30PM +1000, Gavin Shan wrote:
> > > +void qemu_ram_move(void *dst, const void *src, size_t n)
> > > +{
> > > +    uintptr_t test, len;
> > > +
> > > +    if (src == dst || n == 0) {
> > > +        return;
> > > +    }
> > > +
> > > +    /*
> > > +     * Maximal length of aligned access that are determined by @src,
> > > +     * @dst and @n
> > > +     */
> > > +    test = (uintptr_t)src | (uintptr_t)dst | n;
> > > +    len = test & -test;
> > > +
> > > +    /* Overlapping buffers, unaligned or oversized access */
> > > +    if (n > 8 || len != n) {
> > > +        memmove(dst, src, n);
> > > +        return;
> > > +    }
> >
> >
> > This part I don't get exactly. It's fine on most arches I think.
> > x86, power...
> >
> > But here is aarch64 for example:
> > https://codebrowser.dev/glibc/glibc/sysdeps/aarch64/memcpy.S.html
> >
> > it says
> >    It uses unaligned accesses and branchless sequences to keep the code small,
> >    simple and improve performance.
> >
> > so if the point is to get rid of unaligned accesses to BARs, are you sure
> > it achieves this?
> 
> The idea is rather just to ignore the unaligned-access-to-not-RAM
> case as "only a theoretical issue" and default it to memmove().
> If anybody reports an actual problem we can think about it then :-)
> 
> -- PMM

Well given the only reported case of virtio DMA into vfio memory we have is
GH100 and that one is mapped MT_NORMAL, I'm fine with that.

I feel extending vfio to report the mapping type to userspace might
not be a bad idea down the road, though. "Directly accessible"
is a very rough approximation to the rich variety of mappings
available across architectures.

-- 
MST



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] system/memory: Use qemu_ram_move() for directly accessible regions
  2026-07-30 10:27       ` Michael S. Tsirkin
@ 2026-07-30 10:34         ` Peter Maydell
  2026-07-30 10:50           ` Cédric Le Goater
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2026-07-30 10:34 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Gavin Shan, qemu-arm, qemu-devel, peterx, philmd,
	richard.henderson, alex, berrange, philmd, david, clg, pbonzini,
	phrdina, jugraham, liugang24219, dinghui, shan.gavin

On Thu, 30 Jul 2026 at 11:27, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Jul 30, 2026 at 10:24:32AM +0100, Peter Maydell wrote:
> > The idea is rather just to ignore the unaligned-access-to-not-RAM
> > case as "only a theoretical issue" and default it to memmove().
> > If anybody reports an actual problem we can think about it then :-)

> Well given the only reported case of virtio DMA into vfio memory we have is
> GH100 and that one is mapped MT_NORMAL, I'm fine with that.

> I feel extending vfio to report the mapping type to userspace might
> not be a bad idea down the road, though. "Directly accessible"
> is a very rough approximation to the rich variety of mappings
> available across architectures.

Yes, this sounds like a good longer term idea. If userspace doesn't
have a way to know what the mapping behind something it's mmap()ed
is then it can't know what operations it can and can't safely do.

-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] system/memory: Use qemu_ram_move() for directly accessible regions
  2026-07-30 10:34         ` Peter Maydell
@ 2026-07-30 10:50           ` Cédric Le Goater
  0 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2026-07-30 10:50 UTC (permalink / raw)
  To: Peter Maydell, Michael S. Tsirkin
  Cc: Gavin Shan, qemu-arm, qemu-devel, peterx, philmd,
	richard.henderson, alex, berrange, philmd, david, pbonzini,
	phrdina, jugraham, liugang24219, dinghui, shan.gavin

On 7/30/26 12:34, Peter Maydell wrote:
> On Thu, 30 Jul 2026 at 11:27, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Thu, Jul 30, 2026 at 10:24:32AM +0100, Peter Maydell wrote:
>>> The idea is rather just to ignore the unaligned-access-to-not-RAM
>>> case as "only a theoretical issue" and default it to memmove().
>>> If anybody reports an actual problem we can think about it then :-)
> 
>> Well given the only reported case of virtio DMA into vfio memory we have is
>> GH100 and that one is mapped MT_NORMAL, I'm fine with that.
> 
>> I feel extending vfio to report the mapping type to userspace might
>> not be a bad idea down the road, though. "Directly accessible"
>> is a very rough approximation to the rich variety of mappings
>> available across architectures.
> 
> Yes, this sounds like a good longer term idea. If userspace doesn't
> have a way to know what the mapping behind something it's mmap()ed
> is then it can't know what operations it can and can't safely do.
> 
Gavin, didn't you have the idea of using a VFIO region caps for that ?

This is similar to what we did on POWER9 with :

    VFIO_REGION_INFO_CAP_NVLINK2_SSATGT

which exported GPU RAM to the userspace too.

Thanks,

C.



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-07-30 10:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-28  3:17 [PATCH v5 0/3] system/memory: Make ram device region directly accessible Gavin Shan
2026-07-28  3:17 ` [PATCH v5 1/3] system/memory: Use memmove() for directly accessible regions Gavin Shan
2026-07-28 16:07   ` Philippe Mathieu-Daudé
2026-07-28  3:17 ` [PATCH v5 2/3] system/memory: Use qemu_ram_move() " Gavin Shan
2026-07-29 22:35   ` Michael S. Tsirkin
2026-07-30  6:26     ` Gavin Shan
2026-07-30  9:24     ` Peter Maydell
2026-07-30 10:27       ` Michael S. Tsirkin
2026-07-30 10:34         ` Peter Maydell
2026-07-30 10:50           ` Cédric Le Goater
2026-07-28  3:17 ` [PATCH v5 3/3] system/memory: Make ram device region directly accessible Gavin Shan
2026-07-28 15:19 ` [PATCH v5 0/3] " Peter Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.