[Qemu-devel] [PATCH 0/2] memory: Convert skip_dump to ram_device and avoid memcpy

[Qemu-devel] [PATCH 0/2] memory: Convert skip_dump to ram_device and avoid memcpy

[Alex Williamson]

As based on previous RFC:

https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg05183.html

TL;DR, this adds tracing, converts skip_dump to ram_device (named
after rom_device), adds full access widths, identifies ram_device
regions based on ops pointer.

Paolo had suggested converting "skip_dump" to "device_memory", but
"ram_device" seemed like a better fit, feel free to disagree.  It
also didn't feel right to set ram_device via a separate function,
because then we need to worry about all the ways in which a
MemoryRegion might be manipulated to make sure we only apply this
attribute to the kind we want.  Much easier to have a separate
constructor that does this, we're not flipping this on and off like
romd.

Also, I had trouble documenting why I only implemented dword access
as in the RFC, it just seemed like procrastination, so let's just
support full qword.  In fact, why even care about alignment, we
really want to perform the access as prescribed by the guest driver
and performed by the guest processor.  If the driver does an invalid
width or alignment, then so should we.  At least that's my theory.
Of course this means that xp in the monitor once again can't dump
RTL MMIO in more than 4-byte chunks, but that seems like an
operator issue.  Dumping memory via xp is just a tool and if we
apply the tool to do accesses that the device is not capable of,
that's not a problem with the tool.

This fixes what I believe to be the problem Thorsten has
identified with RTL assignment and hopefully he can add a
Tested-by once confirmed.  Thanks,

Alex

---

Alex Williamson (2):
      memory: Replace skip_dump flag with "ram_device"
      memory: Don't use memcpy for ram_device regions


 hw/vfio/common.c      |    9 ++----
 hw/vfio/spapr.c       |    2 +
 include/exec/memory.h |   46 +++++++++++++++++++----------
 memory.c              |   79 +++++++++++++++++++++++++++++++++++++++++++++++--
 memory_mapping.c      |    2 +
 trace-events          |    2 +
 6 files changed, 114 insertions(+), 26 deletions(-)

[Qemu-devel] [PATCH 1/2] memory: Replace skip_dump flag with "ram_device"

[Alex Williamson]

Setting skip_dump on a MemoryRegion allows us to modify one specific
code path, but the restriction we're trying to address encompasses
more than that.  If we have a RAM MemoryRegion backed by a physical
device, it not only restricts our ability to dump that region, but
also affects how we should manipulate it.  Here we recognize that
MemoryRegions do not change to sometimes allow dumps and other times
not, so we replace setting the skip_dump flag with a new initializer
so that we know exactly the type of region to which we're applying
this behavior.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/common.c      |    9 ++++-----
 hw/vfio/spapr.c       |    2 +-
 include/exec/memory.h |   41 ++++++++++++++++++++++++++++-------------
 memory.c              |   13 +++++++++----
 memory_mapping.c      |    2 +-
 5 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 9505fb3..c764cb3 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -724,12 +724,11 @@ int vfio_region_mmap(VFIORegion *region)
 
         name = g_strdup_printf("%s mmaps[%d]",
                                memory_region_name(region->mem), i);
-        memory_region_init_ram_ptr(&region->mmaps[i].mem,
-                                   memory_region_owner(region->mem),
-                                   name, region->mmaps[i].size,
-                                   region->mmaps[i].mmap);
+        memory_region_init_ram_device_ptr(&region->mmaps[i].mem,
+                                          memory_region_owner(region->mem),
+                                          name, region->mmaps[i].size,
+                                          region->mmaps[i].mmap);
         g_free(name);
-        memory_region_set_skip_dump(&region->mmaps[i].mem);
         memory_region_add_subregion(region->mem, region->mmaps[i].offset,
                                     &region->mmaps[i].mem);
 
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 7443d34..4409bcc 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -25,7 +25,7 @@ static bool vfio_prereg_listener_skipped_section(MemoryRegionSection *section)
     }
 
     return !memory_region_is_ram(section->mr) ||
-            memory_region_is_skip_dump(section->mr);
+            memory_region_is_ram_device(section->mr);
 }
 
 static void *vfio_prereg_gpa_to_vaddr(MemoryRegionSection *section, hwaddr gpa)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 79ccaab..a75b8c3 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -209,7 +209,7 @@ struct MemoryRegion {
     void (*destructor)(MemoryRegion *mr);
     uint64_t align;
     bool terminates;
-    bool skip_dump;
+    bool ram_device;
     bool enabled;
     bool warning_printed; /* For reservations */
     uint8_t vga_logging_count;
@@ -449,6 +449,30 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
                                 void *ptr);
 
 /**
+ * memory_region_init_ram_device_ptr:  Initialize RAM device memory region from
+ *                                     a user-provided pointer.
+ *
+ * A RAM device represents a mapping to a physical device, such as to a PCI
+ * MMIO BAR of an vfio-pci assigned device.  The memory region may be mapped
+ * into the VM address space and access to the region will modify memory
+ * directly.  However, the memory region should not be included in a memory
+ * dump (device may not be enabled/mapped at the time of the dump), and
+ * operations incompatible with manipulating MMIO should be avoided.  Replaces
+ * skip_dump flag.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: size of the region.
+ * @ptr: memory to be mapped; must contain at least @size bytes.
+ */
+void memory_region_init_ram_device_ptr(MemoryRegion *mr,
+                                       struct Object *owner,
+                                       const char *name,
+                                       uint64_t size,
+                                       void *ptr);
+
+/**
  * memory_region_init_alias: Initialize a memory region that aliases all or a
  *                           part of another memory region.
  *
@@ -574,22 +598,13 @@ static inline bool memory_region_is_ram(MemoryRegion *mr)
 }
 
 /**
- * memory_region_is_skip_dump: check whether a memory region should not be
- *                             dumped
- *
- * Returns %true is a memory region should not be dumped(e.g. VFIO BAR MMAP).
+ * memory_region_is_ram_device: check whether a memory region is a ram device
  *
- * @mr: the memory region being queried
- */
-bool memory_region_is_skip_dump(MemoryRegion *mr);
-
-/**
- * memory_region_set_skip_dump: Set skip_dump flag, dump will ignore this memory
- *                              region
+ * Returns %true is a memory region is a device backed ram region
  *
  * @mr: the memory region being queried
  */
-void memory_region_set_skip_dump(MemoryRegion *mr);
+bool memory_region_is_ram_device(MemoryRegion *mr);
 
 /**
  * memory_region_is_romd: check whether a memory region is in ROMD mode
diff --git a/memory.c b/memory.c
index edbc701..7ffcff1 100644
--- a/memory.c
+++ b/memory.c
@@ -1355,9 +1355,14 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
     mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
 }
 
-void memory_region_set_skip_dump(MemoryRegion *mr)
+void memory_region_init_ram_device_ptr(MemoryRegion *mr,
+                                       Object *owner,
+                                       const char *name,
+                                       uint64_t size,
+                                       void *ptr)
 {
-    mr->skip_dump = true;
+    memory_region_init_ram_ptr(mr, owner, name, size, ptr);
+    mr->ram_device = true;
 }
 
 void memory_region_init_alias(MemoryRegion *mr,
@@ -1491,9 +1496,9 @@ const char *memory_region_name(const MemoryRegion *mr)
     return mr->name;
 }
 
-bool memory_region_is_skip_dump(MemoryRegion *mr)
+bool memory_region_is_ram_device(MemoryRegion *mr)
 {
-    return mr->skip_dump;
+    return mr->ram_device;
 }
 
 uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
diff --git a/memory_mapping.c b/memory_mapping.c
index e3e0d95..6a39d71 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -206,7 +206,7 @@ static void guest_phys_blocks_region_add(MemoryListener *listener,
 
     /* we only care about RAM */
     if (!memory_region_is_ram(section->mr) ||
-        memory_region_is_skip_dump(section->mr)) {
+        memory_region_is_ram_device(section->mr)) {
         return;
     }
 

[Qemu-devel] [PATCH 2/2] memory: Don't use memcpy for ram_device regions

[Alex Williamson]

With a vfio assigned device we lay down a base MemoryRegion registered
as an IO region, giving us read & write accessors.  If the region
supports mmap, we lay down a higher priority sub-region MemoryRegion
on top of the base layer initialized as a RAM device pointer to the
mmap.  Finally, if we have any quirks for the device (ie. address
ranges that need additional virtualization support), we put another IO
sub-region on top of the mmap MemoryRegion.  When this is flattened,
we now potentially have sub-page mmap MemoryRegions exposed which
cannot be directly mapped through KVM.

This is as expected, but a subtle detail of this is that we end up
with two different access mechanisms through QEMU.  If we disable the
mmap MemoryRegion, we make use of the IO MemoryRegion and service
accesses using pread and pwrite to the vfio device file descriptor.
If the mmap MemoryRegion is enabled and results in one of these
sub-page gaps, QEMU handles the access as RAM, using memcpy to the
mmap.  Using either pread/pwrite or the mmap directly should be
correct, but using memcpy causes us problems.  I expect that not only
does memcpy not necessarily honor the original width and alignment in
performing a copy, but it potentially also uses processor instructions
not intended for MMIO spaces.  It turns out that this has been a
problem for Realtek NIC assignment, which has such a quirk that
creates a sub-page mmap MemoryRegion access.

To resolve this, we disable memory_access_is_direct() for ram_device
regions since QEMU assumes that it can use memcpy for those regions.
Instead we access through MemoryRegionOps, which replaces the memcpy
with simple de-references of standard sizes to the host memory.  This
also allows us to eliminate the ram_device bool from the MemoryRegion
structure since we can simply test the ops pointer.

With this patch we attempt to provide unrestricted access to the RAM
device, allowing byte through qword access as well as unaligned
access.  The assumption here is that accesses initiated by the VM are
driven by a device specific driver, which knows the device
capabilities.  If unaligned accesses are not supported by the device,
we don't want them to work in a VM by performing multiple aligned
accesses to compose the unaligned access.  A down-side of this
philosophy is that the xp command from the monitor attempts to use
the largest available access weidth, unaware of the underlying
device.  Using memcpy had this same restriction, but at least now an
operator can dump individual registers, even if blocks of device
memory may result in access widths beyond the capabilities of a
given device (RTL NICs only support up to dword).

Reported-by: Thorsten Kohfeldt <thorsten.kohfeldt@gmx.de>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 include/exec/memory.h |    7 +++--
 memory.c              |   70 ++++++++++++++++++++++++++++++++++++++++++++++++-
 trace-events          |    2 +
 3 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index a75b8c3..2d4a287 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -209,7 +209,6 @@ struct MemoryRegion {
     void (*destructor)(MemoryRegion *mr);
     uint64_t align;
     bool terminates;
-    bool ram_device;
     bool enabled;
     bool warning_printed; /* For reservations */
     uint8_t vga_logging_count;
@@ -1480,9 +1479,11 @@ void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
 static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
 {
     if (is_write) {
-        return memory_region_is_ram(mr) && !mr->readonly;
+        return memory_region_is_ram(mr) &&
+               !mr->readonly && !memory_region_is_ram_device(mr);
     } else {
-        return memory_region_is_ram(mr) || memory_region_is_romd(mr);
+        return (memory_region_is_ram(mr) && !memory_region_is_ram_device(mr)) ||
+               memory_region_is_romd(mr);
     }
 }
 
diff --git a/memory.c b/memory.c
index 7ffcff1..d07f785 100644
--- a/memory.c
+++ b/memory.c
@@ -1128,6 +1128,71 @@ 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 = (uint64_t)~0;
+
+    switch (size) {
+    case 1:
+        data = *(uint8_t *)(mr->ram_block->host + addr);
+        break;
+    case 2:
+        data = *(uint16_t *)(mr->ram_block->host + addr);
+        break;
+    case 4:
+        data = *(uint32_t *)(mr->ram_block->host + addr);
+        break;
+    case 8:
+        data = *(uint64_t *)(mr->ram_block->host + addr);
+        break;
+    }
+
+    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);
+
+    switch (size) {
+    case 1:
+        *(uint8_t *)(mr->ram_block->host + addr) = (uint8_t)data;
+        break;
+    case 2:
+        *(uint16_t *)(mr->ram_block->host + addr) = (uint16_t)data;
+        break;
+    case 4:
+        *(uint32_t *)(mr->ram_block->host + addr) = (uint32_t)data;
+        break;
+    case 8:
+        *(uint64_t *)(mr->ram_block->host + addr) = data;
+        break;
+    }
+}
+
+static const MemoryRegionOps ram_device_mem_ops = {
+    .read = memory_region_ram_device_read,
+    .write = memory_region_ram_device_write,
+    .endianness = DEVICE_NATIVE_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,
@@ -1362,7 +1427,8 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
                                        void *ptr)
 {
     memory_region_init_ram_ptr(mr, owner, name, size, ptr);
-    mr->ram_device = true;
+    mr->ops = &ram_device_mem_ops;
+    mr->opaque = mr;
 }
 
 void memory_region_init_alias(MemoryRegion *mr,
@@ -1498,7 +1564,7 @@ const char *memory_region_name(const MemoryRegion *mr)
 
 bool memory_region_is_ram_device(MemoryRegion *mr)
 {
-    return mr->ram_device;
+    return mr->ops == &ram_device_mem_ops;
 }
 
 uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
diff --git a/trace-events b/trace-events
index 8ecded5..f74e1d3 100644
--- a/trace-events
+++ b/trace-events
@@ -121,6 +121,8 @@ memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t va
 memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
 memory_region_tb_read(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr %#"PRIx64" value %#"PRIx64" size %u"
 memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr %#"PRIx64" value %#"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 %#"PRIx64" value %#"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 %#"PRIx64" value %#"PRIx64" size %u"
 
 ### Guest events, keep at bottom
 

Re: [Qemu-devel] [PATCH 2/2] memory: Don't use memcpy for ram_device regions

[Paolo Bonzini]

On 25/10/2016 01:00, Alex Williamson wrote:
> With a vfio assigned device we lay down a base MemoryRegion registered
> as an IO region, giving us read & write accessors.  If the region
> supports mmap, we lay down a higher priority sub-region MemoryRegion
> on top of the base layer initialized as a RAM device pointer to the
> mmap.  Finally, if we have any quirks for the device (ie. address
> ranges that need additional virtualization support), we put another IO
> sub-region on top of the mmap MemoryRegion.  When this is flattened,
> we now potentially have sub-page mmap MemoryRegions exposed which
> cannot be directly mapped through KVM.
> 
> This is as expected, but a subtle detail of this is that we end up
> with two different access mechanisms through QEMU.  If we disable the
> mmap MemoryRegion, we make use of the IO MemoryRegion and service
> accesses using pread and pwrite to the vfio device file descriptor.
> If the mmap MemoryRegion is enabled and results in one of these
> sub-page gaps, QEMU handles the access as RAM, using memcpy to the
> mmap.  Using either pread/pwrite or the mmap directly should be
> correct, but using memcpy causes us problems.  I expect that not only
> does memcpy not necessarily honor the original width and alignment in
> performing a copy, but it potentially also uses processor instructions
> not intended for MMIO spaces.  It turns out that this has been a
> problem for Realtek NIC assignment, which has such a quirk that
> creates a sub-page mmap MemoryRegion access.
> 
> To resolve this, we disable memory_access_is_direct() for ram_device
> regions since QEMU assumes that it can use memcpy for those regions.
> Instead we access through MemoryRegionOps, which replaces the memcpy
> with simple de-references of standard sizes to the host memory.  This
> also allows us to eliminate the ram_device bool from the MemoryRegion
> structure since we can simply test the ops pointer.
> 
> With this patch we attempt to provide unrestricted access to the RAM
> device, allowing byte through qword access as well as unaligned
> access.  The assumption here is that accesses initiated by the VM are
> driven by a device specific driver, which knows the device
> capabilities.  If unaligned accesses are not supported by the device,
> we don't want them to work in a VM by performing multiple aligned
> accesses to compose the unaligned access.  A down-side of this
> philosophy is that the xp command from the monitor attempts to use
> the largest available access weidth, unaware of the underlying
> device.  Using memcpy had this same restriction, but at least now an
> operator can dump individual registers, even if blocks of device
> memory may result in access widths beyond the capabilities of a
> given device (RTL NICs only support up to dword).
> 
> Reported-by: Thorsten Kohfeldt <thorsten.kohfeldt@gmx.de>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  include/exec/memory.h |    7 +++--
>  memory.c              |   70 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  trace-events          |    2 +
>  3 files changed, 74 insertions(+), 5 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index a75b8c3..2d4a287 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -209,7 +209,6 @@ struct MemoryRegion {
>      void (*destructor)(MemoryRegion *mr);
>      uint64_t align;
>      bool terminates;
> -    bool ram_device;
>      bool enabled;
>      bool warning_printed; /* For reservations */
>      uint8_t vga_logging_count;
> @@ -1480,9 +1479,11 @@ void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
>  static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
>  {
>      if (is_write) {
> -        return memory_region_is_ram(mr) && !mr->readonly;
> +        return memory_region_is_ram(mr) &&
> +               !mr->readonly && !memory_region_is_ram_device(mr);
>      } else {
> -        return memory_region_is_ram(mr) || memory_region_is_romd(mr);
> +        return (memory_region_is_ram(mr) && !memory_region_is_ram_device(mr)) ||
> +               memory_region_is_romd(mr);

Oops, I hadn't thought of this.  This is a bit slower, especially so if 
you have to compare the ops pointer.  It's probably best to keep the 
mr->ram_device flag, so that later we can place all five relevant flags 
(ram, readonly, ram_device, rom_device, romd_mode) into the same word 
and play games with bits, like the following:

	// write case
	mr->flags == MR_RAM

	// read cases
	(mr->flags & ~MR_READONLY) == MR_RAM ||
	(mr->flags & ~MR_READONLY) == (MR_ROM_DEVICE | MR_ROMD_MODE)

(the latter in turn can be optimized to a range check if the flags are
arranged cleverly).

The patch is okay with mr->ram_device left in place, feel free to merge 
it through your own VFIO tree.

Paolo

>      }
>  }
>  
> diff --git a/memory.c b/memory.c
> index 7ffcff1..d07f785 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1128,6 +1128,71 @@ 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 = (uint64_t)~0;
> +
> +    switch (size) {
> +    case 1:
> +        data = *(uint8_t *)(mr->ram_block->host + addr);
> +        break;
> +    case 2:
> +        data = *(uint16_t *)(mr->ram_block->host + addr);
> +        break;
> +    case 4:
> +        data = *(uint32_t *)(mr->ram_block->host + addr);
> +        break;
> +    case 8:
> +        data = *(uint64_t *)(mr->ram_block->host + addr);
> +        break;
> +    }
> +
> +    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);
> +
> +    switch (size) {
> +    case 1:
> +        *(uint8_t *)(mr->ram_block->host + addr) = (uint8_t)data;
> +        break;
> +    case 2:
> +        *(uint16_t *)(mr->ram_block->host + addr) = (uint16_t)data;
> +        break;
> +    case 4:
> +        *(uint32_t *)(mr->ram_block->host + addr) = (uint32_t)data;
> +        break;
> +    case 8:
> +        *(uint64_t *)(mr->ram_block->host + addr) = data;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps ram_device_mem_ops = {
> +    .read = memory_region_ram_device_read,
> +    .write = memory_region_ram_device_write,
> +    .endianness = DEVICE_NATIVE_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,
> @@ -1362,7 +1427,8 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
>                                         void *ptr)
>  {
>      memory_region_init_ram_ptr(mr, owner, name, size, ptr);
> -    mr->ram_device = true;
> +    mr->ops = &ram_device_mem_ops;
> +    mr->opaque = mr;
>  }
>  
>  void memory_region_init_alias(MemoryRegion *mr,
> @@ -1498,7 +1564,7 @@ const char *memory_region_name(const MemoryRegion *mr)
>  
>  bool memory_region_is_ram_device(MemoryRegion *mr)
>  {
> -    return mr->ram_device;
> +    return mr->ops == &ram_device_mem_ops;
>  }
>  
>  uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
> diff --git a/trace-events b/trace-events
> index 8ecded5..f74e1d3 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -121,6 +121,8 @@ memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t va
>  memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
>  memory_region_tb_read(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr %#"PRIx64" value %#"PRIx64" size %u"
>  memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr %#"PRIx64" value %#"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 %#"PRIx64" value %#"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 %#"PRIx64" value %#"PRIx64" size %u"
>  
>  ### Guest events, keep at bottom
>  
>