[PATCH v2 0/7] memory: Remove most _nomigrate variants

[PATCH v2 0/7] memory: Remove most _nomigrate variants

[BALATON Zoltan]

v2 of the memory region API clean ups. The _nomigrate variants are
rarely needed, some of them are already unused so remove them
standardising on memory_region_init_ram_flags_nomigrate() for the few
cases where this is really needed. The Sun machines are kept to use
global vmstate and added a few more clean up patches from my previous
series.

BALATON Zoltan (7):
  memory: Remove memory_region_init_rom_device_nomigrate()
  hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
  memory: Remove memory_region_init_rom_nomigrate()
  memory: Remove memory_region_init_ram_nomigrate()
  memory: Add internal memory_region_set_ops helper function
  memory: Factor out common ram region initialization
  memory: Add internal memory_region_register_ram function

 docs/devel/memory.rst                         |  10 +-
 hw/display/cg3.c                              |   6 +-
 hw/display/tcx.c                              |  11 +-
 hw/display/vga.c                              |   4 +-
 hw/sparc/sun4m.c                              |  15 +-
 hw/sparc64/sun4u.c                            |   9 +-
 hw/xtensa/xtfpga.c                            |   4 +-
 include/system/memory.h                       |  76 -----
 .../memory-region-housekeeping.cocci          |  42 +--
 system/memory.c                               | 271 ++++++------------
 10 files changed, 114 insertions(+), 334 deletions(-)

-- 
2.41.3

[PATCH v2 1/7] memory: Remove memory_region_init_rom_device_nomigrate()

[BALATON Zoltan]

This function is not used outside of memory_region_init_rom_device()
which is its only caller. Inline it there and remove it.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 docs/devel/memory.rst                         |  1 -
 include/system/memory.h                       | 27 ------------
 .../memory-region-housekeeping.cocci          |  8 ----
 system/memory.c                               | 41 ++++++-------------
 4 files changed, 13 insertions(+), 64 deletions(-)

diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
index f22146e56c..8558f70a42 100644
--- a/docs/devel/memory.rst
+++ b/docs/devel/memory.rst
@@ -114,7 +114,6 @@ the backing memory yourself, you can call the functions:
 
 - memory_region_init_ram_nomigrate()
 - memory_region_init_rom_nomigrate()
-- memory_region_init_rom_device_nomigrate()
 
 which only initialize the MemoryRegion and leave handling
 migration to the caller.
diff --git a/include/system/memory.h b/include/system/memory.h
index 8f8725ea2d..0562af3136 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -1614,33 +1614,6 @@ bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
                                       uint64_t size,
                                       Error **errp);
 
-/**
- * memory_region_init_rom_device_nomigrate:  Initialize a ROM memory region.
- *                                 Writes are handled via callbacks.
- *
- * Note that this function does not do anything to cause the data in the
- * RAM side of the memory region to be migrated; that is the responsibility
- * of the caller.
- *
- * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
- * @ops: callbacks for write access handling (must not be NULL).
- * @opaque: passed to the read and write callbacks of the @ops structure.
- * @name: Region name, becomes part of RAMBlock name used in migration stream
- *        must be unique within any device
- * @size: size of the region.
- * @errp: pointer to Error*, to store an error if it happens.
- *
- * Return: true on success, else false setting @errp with error.
- */
-bool memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
-                                             Object *owner,
-                                             const MemoryRegionOps *ops,
-                                             void *opaque,
-                                             const char *name,
-                                             uint64_t size,
-                                             Error **errp);
-
 /**
  * memory_region_init_iommu: Initialize a memory region of a custom type
  * that translates addresses
diff --git a/scripts/coccinelle/memory-region-housekeeping.cocci b/scripts/coccinelle/memory-region-housekeeping.cocci
index 29651ebde9..7f89e9712e 100644
--- a/scripts/coccinelle/memory-region-housekeeping.cocci
+++ b/scripts/coccinelle/memory-region-housekeeping.cocci
@@ -97,14 +97,6 @@ expression NAME;
 expression SIZE;
 expression ERRP;
 @@
--memory_region_init_rom_device_nomigrate(MR, NULL, OPS, OPAQUE, NAME, SIZE, ERRP);
-+memory_region_init_rom_device(MR, NULL, OPS, OPAQUE, NAME, SIZE, ERRP);
- ...
--vmstate_register_ram_global(MR);
-
-
-// Device is owner
-@@
 typedef DeviceState;
 identifier device_fn, dev, obj;
 expression E1, E2, E3, E4, E5;
diff --git a/system/memory.c b/system/memory.c
index 4bf00d82bc..a9032fb2cf 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1748,32 +1748,6 @@ bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
     return true;
 }
 
-bool memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
-                                             Object *owner,
-                                             const MemoryRegionOps *ops,
-                                             void *opaque,
-                                             const char *name,
-                                             uint64_t size,
-                                             Error **errp)
-{
-    Error *err = NULL;
-    assert(ops);
-    memory_region_init(mr, owner, name, size);
-    mr->ops = ops;
-    mr->opaque = opaque;
-    mr->terminates = true;
-    mr->rom_device = true;
-    mr->destructor = memory_region_destructor_ram;
-    mr->ram_block = qemu_ram_alloc(size, 0, mr, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
-        return false;
-    }
-    return true;
-}
-
 void memory_region_init_iommu(void *_iommu_mr,
                               size_t instance_size,
                               const char *mrtypename,
@@ -3802,9 +3776,20 @@ bool memory_region_init_rom_device(MemoryRegion *mr,
                                    Error **errp)
 {
     DeviceState *owner_dev;
+    Error *err = NULL;
 
-    if (!memory_region_init_rom_device_nomigrate(mr, owner, ops, opaque,
-                                                 name, size, errp)) {
+    assert(ops);
+    memory_region_init(mr, owner, name, size);
+    mr->ops = ops;
+    mr->opaque = opaque;
+    mr->terminates = true;
+    mr->rom_device = true;
+    mr->destructor = memory_region_destructor_ram;
+    mr->ram_block = qemu_ram_alloc(size, 0, mr, &err);
+    if (err) {
+        mr->size = int128_zero();
+        object_unparent(OBJECT(mr));
+        error_propagate(errp, err);
         return false;
     }
     /* This will assert if owner is neither NULL nor a DeviceState.
-- 
2.41.3

[PATCH v2 2/7] hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()

[BALATON Zoltan]

We could simply convert these to use memory_region_init_rom but that
would be a migration compatibility break so preserve current behaviour
for now.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/cg3.c | 6 ++++--
 hw/display/tcx.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index 568d6048a6..44966e7586 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -282,8 +282,10 @@ static void cg3_initfn(Object *obj)
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
     CG3State *s = CG3(obj);
 
-    memory_region_init_rom_nomigrate(&s->rom, obj, "cg3.prom",
-                                     FCODE_MAX_ROM_SIZE, &error_fatal);
+    memory_region_init_ram_flags_nomigrate(&s->rom, obj, "cg3.prom",
+                                           FCODE_MAX_ROM_SIZE, 0,
+                                           &error_fatal);
+    memory_region_set_readonly(&s->rom, true);
     sysbus_init_mmio(sbd, &s->rom);
 
     memory_region_init_io(&s->reg, obj, &cg3_reg_ops, s, "cg3.reg",
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 36cad82abd..87fe7216ba 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -756,8 +756,10 @@ static void tcx_initfn(Object *obj)
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
     TCXState *s = TCX(obj);
 
-    memory_region_init_rom_nomigrate(&s->rom, obj, "tcx.prom",
-                                     FCODE_MAX_ROM_SIZE, &error_fatal);
+    memory_region_init_ram_flags_nomigrate(&s->rom, obj, "tcx.prom",
+                                           FCODE_MAX_ROM_SIZE, 0,
+                                           &error_fatal);
+    memory_region_set_readonly(&s->rom, true);
     sysbus_init_mmio(sbd, &s->rom);
 
     /* 2/STIP : Stippler */
-- 
2.41.3

[PATCH v2 3/7] memory: Remove memory_region_init_rom_nomigrate()

[BALATON Zoltan]

All users were converted so no longer needed.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 docs/devel/memory.rst                         |  1 -
 include/system/memory.h                       | 26 -------------------
 .../memory-region-housekeeping.cocci          | 16 ------------
 system/memory.c                               | 19 +++-----------
 4 files changed, 3 insertions(+), 59 deletions(-)

diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
index 8558f70a42..0bb5acab21 100644
--- a/docs/devel/memory.rst
+++ b/docs/devel/memory.rst
@@ -113,7 +113,6 @@ have a special case where you need to manage the migration of
 the backing memory yourself, you can call the functions:
 
 - memory_region_init_ram_nomigrate()
-- memory_region_init_rom_nomigrate()
 
 which only initialize the MemoryRegion and leave handling
 migration to the caller.
diff --git a/include/system/memory.h b/include/system/memory.h
index 0562af3136..7117699b10 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -1588,32 +1588,6 @@ void memory_region_init_alias(MemoryRegion *mr,
                               hwaddr offset,
                               uint64_t size);
 
-/**
- * memory_region_init_rom_nomigrate: Initialize a ROM memory region.
- *
- * This has the same effect as calling memory_region_init_ram_nomigrate()
- * and then marking the resulting region read-only with
- * memory_region_set_readonly().
- *
- * Note that this function does not do anything to cause the data in the
- * RAM side of the memory region to be migrated; that is the responsibility
- * of the caller.
- *
- * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
- * @name: Region name, becomes part of RAMBlock name used in migration stream
- *        must be unique within any device
- * @size: size of the region.
- * @errp: pointer to Error*, to store an error if it happens.
- *
- * Return: true on success, else false setting @errp with error.
- */
-bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
-                                      Object *owner,
-                                      const char *name,
-                                      uint64_t size,
-                                      Error **errp);
-
 /**
  * memory_region_init_iommu: Initialize a memory region of a custom type
  * that translates addresses
diff --git a/scripts/coccinelle/memory-region-housekeeping.cocci b/scripts/coccinelle/memory-region-housekeeping.cocci
index 7f89e9712e..8ec9a06c18 100644
--- a/scripts/coccinelle/memory-region-housekeeping.cocci
+++ b/scripts/coccinelle/memory-region-housekeeping.cocci
@@ -16,17 +16,10 @@
 expression E1, E2, E3, E4, E5;
 symbol true;
 @@
-(
 - memory_region_init_ram(E1, E2, E3, E4, E5);
 + memory_region_init_rom(E1, E2, E3, E4, E5);
   ... WHEN != E1
 - memory_region_set_readonly(E1, true);
-|
-- memory_region_init_ram_nomigrate(E1, E2, E3, E4, E5);
-+ memory_region_init_rom_nomigrate(E1, E2, E3, E4, E5);
-  ... WHEN != E1
-- memory_region_set_readonly(E1, true);
-)
 
 
 @possible_memory_region_init_rom@
@@ -53,11 +46,7 @@ cocci.print_main("potential use of memory_region_init_rom*() in ", p)
 expression ROM, E1, E2, E3, E4;
 expression ALIAS, E5, E6, E7, E8;
 @@
-(
   memory_region_init_rom(ROM, E1, E2, E3, E4);
-|
-  memory_region_init_rom_nomigrate(ROM, E1, E2, E3, E4);
-)
   ...
    memory_region_init_alias(ALIAS, E5, E6, ROM, E7, E8);
 -  memory_region_set_readonly(ALIAS, true);
@@ -85,11 +74,6 @@ expression NAME;
 expression SIZE;
 expression ERRP;
 @@
--memory_region_init_rom_nomigrate(MR, NULL, NAME, SIZE, ERRP);
-+memory_region_init_rom(MR, NULL, NAME, SIZE, ERRP);
- ...
--vmstate_register_ram_global(MR);
-@@
 expression MR;
 expression OPS;
 expression OPAQUE;
diff --git a/system/memory.c b/system/memory.c
index a9032fb2cf..b2b00ba7b4 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1733,21 +1733,6 @@ void memory_region_init_alias(MemoryRegion *mr,
     mr->alias_offset = offset;
 }
 
-bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
-                                      Object *owner,
-                                      const char *name,
-                                      uint64_t size,
-                                      Error **errp)
-{
-    if (!memory_region_init_ram_flags_nomigrate(mr, owner, name,
-                                                size, 0, errp)) {
-         return false;
-    }
-    mr->readonly = true;
-
-    return true;
-}
-
 void memory_region_init_iommu(void *_iommu_mr,
                               size_t instance_size,
                               const char *mrtypename,
@@ -3752,9 +3737,11 @@ bool memory_region_init_rom(MemoryRegion *mr,
 {
     DeviceState *owner_dev;
 
-    if (!memory_region_init_rom_nomigrate(mr, owner, name, size, errp)) {
+    if (!memory_region_init_ram_flags_nomigrate(mr, owner, name,
+                                                size, 0, errp)) {
         return false;
     }
+    mr->readonly = true;
     /* This will assert if owner is neither NULL nor a DeviceState.
      * We only want the owner here for the purposes of defining a
      * unique name for migration. TODO: Ideally we should implement
-- 
2.41.3

[PATCH v2 4/7] memory: Remove memory_region_init_ram_nomigrate()

[BALATON Zoltan]

This is rarely needed and those cases can use
memory_region_init_ram_flags_nomigrate() instead which is now the only
_nomigrate variant left. (Some of these may better be converted to use
memory_region_init_ram() but I don't know so left them as they are.)

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 docs/devel/memory.rst                         |  8 +++----
 hw/display/tcx.c                              |  5 ++--
 hw/display/vga.c                              |  4 ++--
 hw/sparc/sun4m.c                              | 15 +++++++-----
 hw/sparc64/sun4u.c                            |  9 ++++----
 hw/xtensa/xtfpga.c                            |  4 ++--
 include/system/memory.h                       | 23 -------------------
 .../memory-region-housekeeping.cocci          | 18 +--------------
 system/memory.c                               | 13 ++---------
 9 files changed, 27 insertions(+), 72 deletions(-)

diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
index 0bb5acab21..9083b18f08 100644
--- a/docs/devel/memory.rst
+++ b/docs/devel/memory.rst
@@ -110,11 +110,9 @@ migrated:
 
 For most devices and boards this is the correct thing. If you
 have a special case where you need to manage the migration of
-the backing memory yourself, you can call the functions:
-
-- memory_region_init_ram_nomigrate()
-
-which only initialize the MemoryRegion and leave handling
+the backing memory yourself, you can call the function
+memory_region_init_ram_flags_nomigrate()
+which only initializes the MemoryRegion and leaves handling
 migration to the caller.
 
 The functions:
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 87fe7216ba..d6be297828 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -817,8 +817,9 @@ static void tcx_realizefn(DeviceState *dev, Error **errp)
     uint8_t *vram_base;
     char *fcode_filename;
 
-    memory_region_init_ram_nomigrate(&s->vram_mem, OBJECT(s), "tcx.vram",
-                           s->vram_size * (1 + 4 + 4), &error_fatal);
+    memory_region_init_ram_flags_nomigrate(&s->vram_mem, OBJECT(s), "tcx.vram",
+                                           s->vram_size * (1 + 4 + 4), 0,
+                                           &error_fatal);
     vmstate_register_ram_global(&s->vram_mem);
     memory_region_set_log(&s->vram_mem, true, DIRTY_MEMORY_VGA);
     vram_base = memory_region_get_ram_ptr(&s->vram_mem);
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 59a65cbbff..ee7d97b5c2 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2235,8 +2235,8 @@ bool vga_common_init(VGACommonState *s, Object *obj, Error **errp)
         return false;
     }
 
-    memory_region_init_ram_nomigrate(&s->vram, obj, "vga.vram", s->vram_size,
-                                     &local_err);
+    memory_region_init_ram_flags_nomigrate(&s->vram, obj, "vga.vram",
+                                           s->vram_size, 0, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return false;
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 0c0d658d30..66a21ae86c 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -587,8 +587,10 @@ static void idreg_realize(DeviceState *ds, Error **errp)
     IDRegState *s = MACIO_ID_REGISTER(ds);
     SysBusDevice *dev = SYS_BUS_DEVICE(ds);
 
-    if (!memory_region_init_ram_nomigrate(&s->mem, OBJECT(ds), "sun4m.idreg",
-                                          sizeof(idreg_data), errp)) {
+    if (!memory_region_init_ram_flags_nomigrate(&s->mem, OBJECT(ds),
+                                                "sun4m.idreg",
+                                                sizeof(idreg_data),
+                                                0, errp)) {
         return;
     }
 
@@ -638,8 +640,8 @@ static void afx_realize(DeviceState *ds, Error **errp)
     AFXState *s = TCX_AFX(ds);
     SysBusDevice *dev = SYS_BUS_DEVICE(ds);
 
-    if (!memory_region_init_ram_nomigrate(&s->mem, OBJECT(ds), "sun4m.afx",
-                                          4, errp)) {
+    if (!memory_region_init_ram_flags_nomigrate(&s->mem, OBJECT(ds),
+                                                "sun4m.afx", 4, 0, errp)) {
         return;
     }
 
@@ -719,8 +721,9 @@ static void prom_realize(DeviceState *ds, Error **errp)
     PROMState *s = OPENPROM(ds);
     SysBusDevice *dev = SYS_BUS_DEVICE(ds);
 
-    if (!memory_region_init_ram_nomigrate(&s->prom, OBJECT(ds), "sun4m.prom",
-                                          PROM_SIZE_MAX, errp)) {
+    if (!memory_region_init_ram_flags_nomigrate(&s->prom, OBJECT(ds),
+                                                "sun4m.prom", PROM_SIZE_MAX, 0,
+                                                errp)) {
         return;
     }
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 5d7787fc1a..0da0eef74c 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -455,8 +455,9 @@ static void prom_realize(DeviceState *ds, Error **errp)
     PROMState *s = OPENPROM(ds);
     SysBusDevice *dev = SYS_BUS_DEVICE(ds);
 
-    if (!memory_region_init_ram_nomigrate(&s->prom, OBJECT(ds), "sun4u.prom",
-                                          PROM_SIZE_MAX, errp)) {
+    if (!memory_region_init_ram_flags_nomigrate(&s->prom, OBJECT(ds),
+                                                "sun4u.prom", PROM_SIZE_MAX, 0,
+                                                errp)) {
         return;
     }
 
@@ -498,8 +499,8 @@ static void ram_realize(DeviceState *dev, Error **errp)
     RamDevice *d = SUN4U_RAM(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
-    memory_region_init_ram_nomigrate(&d->ram, OBJECT(d), "sun4u.ram", d->size,
-                           &error_fatal);
+    memory_region_init_ram_flags_nomigrate(&d->ram, OBJECT(d), "sun4u.ram",
+                                           d->size, 0, &error_fatal);
     vmstate_register_ram_global(&d->ram);
     sysbus_init_mmio(sbd, &d->ram);
 }
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index d427d68e50..b025cc53a8 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -163,8 +163,8 @@ static void xtfpga_net_init(MemoryRegion *address_space,
             sysbus_mmio_get_region(s, 1));
 
     ram = g_malloc(sizeof(*ram));
-    memory_region_init_ram_nomigrate(ram, OBJECT(s), "open_eth.ram", 16 * KiB,
-                           &error_fatal);
+    memory_region_init_ram_flags_nomigrate(ram, OBJECT(s), "open_eth.ram",
+                                           16 * KiB, 0, &error_fatal);
     vmstate_register_ram_global(ram);
     memory_region_add_subregion(address_space, buffers, ram);
 }
diff --git a/include/system/memory.h b/include/system/memory.h
index 7117699b10..d4793a08a7 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -1374,29 +1374,6 @@ void memory_region_init_io(MemoryRegion *mr,
                            const char *name,
                            uint64_t size);
 
-/**
- * memory_region_init_ram_nomigrate:  Initialize RAM memory region.  Accesses
- *                                    into the region will modify memory
- *                                    directly.
- *
- * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
- * @name: Region name, becomes part of RAMBlock name used in migration stream
- *        must be unique within any device
- * @size: size of the region.
- * @errp: pointer to Error*, to store an error if it happens.
- *
- * Note that this function does not do anything to cause the data in the
- * RAM memory region to be migrated; that is the responsibility of the caller.
- *
- * Return: true on success, else false setting @errp with error.
- */
-bool memory_region_init_ram_nomigrate(MemoryRegion *mr,
-                                      Object *owner,
-                                      const char *name,
-                                      uint64_t size,
-                                      Error **errp);
-
 /**
  * memory_region_init_ram_flags_nomigrate:  Initialize RAM memory region.
  *                                          Accesses into the region will
diff --git a/scripts/coccinelle/memory-region-housekeeping.cocci b/scripts/coccinelle/memory-region-housekeeping.cocci
index 8ec9a06c18..40a8f7beb1 100644
--- a/scripts/coccinelle/memory-region-housekeeping.cocci
+++ b/scripts/coccinelle/memory-region-housekeeping.cocci
@@ -26,15 +26,9 @@ symbol true;
 expression E1, E2, E3, E4, E5;
 position p;
 @@
-(
   memory_region_init_ram@p(E1, E2, E3, E4, E5);
   ...
   memory_region_set_readonly(E1, true);
-|
-  memory_region_init_ram_nomigrate@p(E1, E2, E3, E4, E5);
-  ...
-  memory_region_set_readonly(E1, true);
-)
 @script:python@
 p << possible_memory_region_init_rom.p;
 @@
@@ -52,7 +46,7 @@ expression ALIAS, E5, E6, E7, E8;
 -  memory_region_set_readonly(ALIAS, true);
 
 
-// Replace by-hand memory_region_init_ram_nomigrate/vmstate_register_ram
+// Replace by-hand memory_region_init_ram/vmstate_register_ram
 // code sequences with use of the new memory_region_init_ram function.
 // Similarly for the _rom and _rom_device functions.
 // We don't try to replace sequences with a non-NULL owner, because
@@ -64,16 +58,6 @@ expression NAME;
 expression SIZE;
 expression ERRP;
 @@
--memory_region_init_ram_nomigrate(MR, NULL, NAME, SIZE, ERRP);
-+memory_region_init_ram(MR, NULL, NAME, SIZE, ERRP);
- ...
--vmstate_register_ram_global(MR);
-@@
-expression MR;
-expression NAME;
-expression SIZE;
-expression ERRP;
-@@
 expression MR;
 expression OPS;
 expression OPAQUE;
diff --git a/system/memory.c b/system/memory.c
index b2b00ba7b4..921e96554b 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1572,16 +1572,6 @@ void memory_region_init_io(MemoryRegion *mr,
     mr->terminates = true;
 }
 
-bool memory_region_init_ram_nomigrate(MemoryRegion *mr,
-                                      Object *owner,
-                                      const char *name,
-                                      uint64_t size,
-                                      Error **errp)
-{
-    return memory_region_init_ram_flags_nomigrate(mr, owner, name,
-                                                  size, 0, errp);
-}
-
 bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
                                             Object *owner,
                                             const char *name,
@@ -3690,7 +3680,8 @@ bool memory_region_init_ram(MemoryRegion *mr,
 {
     DeviceState *owner_dev;
 
-    if (!memory_region_init_ram_nomigrate(mr, owner, name, size, errp)) {
+    if (!memory_region_init_ram_flags_nomigrate(mr, owner, name,
+                                                size, 0, errp)) {
         return false;
     }
     /* This will assert if owner is neither NULL nor a DeviceState.
-- 
2.41.3

[PATCH v2 5/7] memory: Add internal memory_region_set_ops helper function

[BALATON Zoltan]

This is a common operation used at multiple places, add a helper
function for it.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 system/memory.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 921e96554b..e15f931a8a 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1559,6 +1559,15 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
     }
 }
 
+static void memory_region_set_ops(MemoryRegion *mr,
+                                  const MemoryRegionOps *ops,
+                                  void *opaque)
+{
+    mr->ops = ops ?: &unassigned_mem_ops;
+    mr->opaque = opaque;
+    mr->terminates = true;
+}
+
 void memory_region_init_io(MemoryRegion *mr,
                            Object *owner,
                            const MemoryRegionOps *ops,
@@ -1567,9 +1576,7 @@ void memory_region_init_io(MemoryRegion *mr,
                            uint64_t size)
 {
     memory_region_init(mr, owner, name, size);
-    mr->ops = ops ? ops : &unassigned_mem_ops;
-    mr->opaque = opaque;
-    mr->terminates = true;
+    memory_region_set_ops(mr, ops, opaque);
 }
 
 bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
@@ -1700,10 +1707,8 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
 {
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
-    mr->terminates = true;
     mr->ram_device = true;
-    mr->ops = &ram_device_mem_ops;
-    mr->opaque = mr;
+    memory_region_set_ops(mr, &ram_device_mem_ops, mr);
     mr->destructor = memory_region_destructor_ram;
 
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
@@ -3758,9 +3763,7 @@ bool memory_region_init_rom_device(MemoryRegion *mr,
 
     assert(ops);
     memory_region_init(mr, owner, name, size);
-    mr->ops = ops;
-    mr->opaque = opaque;
-    mr->terminates = true;
+    memory_region_set_ops(mr, ops, opaque);
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_ram;
     mr->ram_block = qemu_ram_alloc(size, 0, mr, &err);
-- 
2.41.3

[PATCH v2 6/7] memory: Factor out common ram region initialization

[BALATON Zoltan]

Introduce internal memory_region_do_init_ram() function to remove
duplicated code from different memory_region_init_*ram functions.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 system/memory.c | 123 ++++++++++++++++--------------------------------
 1 file changed, 41 insertions(+), 82 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index e15f931a8a..96a09d7cf3 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1579,19 +1579,12 @@ void memory_region_init_io(MemoryRegion *mr,
     memory_region_set_ops(mr, ops, opaque);
 }
 
-bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
-                                            Object *owner,
-                                            const char *name,
-                                            uint64_t size,
-                                            uint32_t ram_flags,
-                                            Error **errp)
+static bool memory_region_do_init_ram(MemoryRegion *mr,
+                                      Error *err, Error **errp)
 {
-    Error *err = NULL;
-    memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
     if (err) {
         mr->size = int128_zero();
         object_unparent(OBJECT(mr));
@@ -1601,6 +1594,17 @@ bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
     return true;
 }
 
+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr, Object *owner,
+                                            const char *name, uint64_t size,
+                                            uint32_t ram_flags, Error **errp)
+{
+    Error *err = NULL;
+
+    memory_region_init(mr, owner, name, size);
+    mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
+    return memory_region_do_init_ram(mr, err, errp);
+}
+
 bool memory_region_init_resizeable_ram(MemoryRegion *mr,
                                        Object *owner,
                                        const char *name,
@@ -1612,108 +1616,66 @@ bool memory_region_init_resizeable_ram(MemoryRegion *mr,
                                        Error **errp)
 {
     Error *err = NULL;
+
     memory_region_init(mr, owner, name, size);
-    mr->ram = true;
-    mr->terminates = true;
-    mr->destructor = memory_region_destructor_ram;
-    mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized,
-                                              mr, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
-        return false;
-    }
-    return true;
+    mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr,
+                                              &err);
+    return memory_region_do_init_ram(mr, err, errp);
 }
 
 #if defined(CONFIG_POSIX) && !defined(EMSCRIPTEN)
-bool memory_region_init_ram_from_file(MemoryRegion *mr,
-                                      Object *owner,
-                                      const char *name,
-                                      uint64_t size,
-                                      uint64_t align,
-                                      uint32_t ram_flags,
-                                      const char *path,
-                                      ram_addr_t offset,
+bool memory_region_init_ram_from_file(MemoryRegion *mr, Object *owner,
+                                      const char *name, uint64_t size,
+                                      uint64_t align, uint32_t ram_flags,
+                                      const char *path, ram_addr_t offset,
                                       Error **errp)
 {
     Error *err = NULL;
+
     memory_region_init(mr, owner, name, size);
-    mr->ram = true;
     mr->readonly = !!(ram_flags & RAM_READONLY);
-    mr->terminates = true;
-    mr->destructor = memory_region_destructor_ram;
     mr->align = align;
-    mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path,
-                                             offset, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
-        return false;
-    }
-    return true;
+    mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path, offset,
+                                             &err);
+    return memory_region_do_init_ram(mr, err, errp);
 }
 
-bool memory_region_init_ram_from_fd(MemoryRegion *mr,
-                                    Object *owner,
-                                    const char *name,
-                                    uint64_t size,
-                                    uint32_t ram_flags,
-                                    int fd,
-                                    ram_addr_t offset,
-                                    Error **errp)
+bool memory_region_init_ram_from_fd(MemoryRegion *mr, Object *owner,
+                                    const char *name, uint64_t size,
+                                    uint32_t ram_flags, int fd,
+                                    ram_addr_t offset, Error **errp)
 {
     Error *err = NULL;
+
     memory_region_init(mr, owner, name, size);
-    mr->ram = true;
     mr->readonly = !!(ram_flags & RAM_READONLY);
-    mr->terminates = true;
-    mr->destructor = memory_region_destructor_ram;
     mr->ram_block = qemu_ram_alloc_from_fd(size, size, NULL, mr, ram_flags, fd,
                                            offset, false, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
-        return false;
-    }
-    return true;
+    return memory_region_do_init_ram(mr, err, errp);
 }
 #endif
 
-void memory_region_init_ram_ptr(MemoryRegion *mr,
-                                Object *owner,
-                                const char *name,
-                                uint64_t size,
-                                void *ptr)
+void memory_region_init_ram_ptr(MemoryRegion *mr, Object *owner,
+                                const char *name, uint64_t size, void *ptr)
 {
     memory_region_init(mr, owner, name, size);
-    mr->ram = true;
-    mr->terminates = true;
-    mr->destructor = memory_region_destructor_ram;
-
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
     mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_abort);
+    memory_region_do_init_ram(mr, NULL, NULL);
 }
 
-void memory_region_init_ram_device_ptr(MemoryRegion *mr,
-                                       Object *owner,
-                                       const char *name,
-                                       uint64_t size,
+void memory_region_init_ram_device_ptr(MemoryRegion *mr, Object *owner,
+                                       const char *name, uint64_t size,
                                        void *ptr)
 {
     memory_region_init(mr, owner, name, size);
-    mr->ram = true;
-    mr->ram_device = true;
     memory_region_set_ops(mr, &ram_device_mem_ops, mr);
-    mr->destructor = memory_region_destructor_ram;
-
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
     mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_abort);
+    memory_region_do_init_ram(mr, NULL, NULL);
+    mr->ram_device = true;
 }
 
 void memory_region_init_alias(MemoryRegion *mr,
@@ -3764,15 +3726,12 @@ bool memory_region_init_rom_device(MemoryRegion *mr,
     assert(ops);
     memory_region_init(mr, owner, name, size);
     memory_region_set_ops(mr, ops, opaque);
-    mr->rom_device = true;
-    mr->destructor = memory_region_destructor_ram;
     mr->ram_block = qemu_ram_alloc(size, 0, mr, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
+    if (!memory_region_do_init_ram(mr, err, errp)) {
         return false;
     }
+    mr->ram = false;
+    mr->rom_device = true;
     /* This will assert if owner is neither NULL nor a DeviceState.
      * We only want the owner here for the purposes of defining a
      * unique name for migration. TODO: Ideally we should implement
-- 
2.41.3

[PATCH v2 7/7] memory: Add internal memory_region_register_ram function

[BALATON Zoltan]

Factor out common operation from memory_region_init_{ram,rom}
functions to register the region for migration. This avoids
duplicating the long comment in several functions.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/memory.c | 76 ++++++++++++++-----------------------------------
 1 file changed, 21 insertions(+), 55 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 96a09d7cf3..1fa8b9c974 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3639,18 +3639,10 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
     }
 }
 
-bool memory_region_init_ram(MemoryRegion *mr,
-                            Object *owner,
-                            const char *name,
-                            uint64_t size,
-                            Error **errp)
+static void memory_region_register_ram(MemoryRegion *mr, Object *owner)
 {
     DeviceState *owner_dev;
 
-    if (!memory_region_init_ram_flags_nomigrate(mr, owner, name,
-                                                size, 0, errp)) {
-        return false;
-    }
     /* This will assert if owner is neither NULL nor a DeviceState.
      * We only want the owner here for the purposes of defining a
      * unique name for migration. TODO: Ideally we should implement
@@ -3659,68 +3651,50 @@ bool memory_region_init_ram(MemoryRegion *mr,
      */
     owner_dev = DEVICE(owner);
     vmstate_register_ram(mr, owner_dev);
+}
 
+bool memory_region_init_ram(MemoryRegion *mr, Object *owner,
+                            const char *name, uint64_t size,
+                            Error **errp)
+{
+    if (!memory_region_init_ram_flags_nomigrate(mr, owner, name, size,
+                                                0, errp)) {
+        return false;
+    }
+    memory_region_register_ram(mr, owner);
     return true;
 }
 
-bool memory_region_init_ram_guest_memfd(MemoryRegion *mr,
-                                        Object *owner,
-                                        const char *name,
-                                        uint64_t size,
+bool memory_region_init_ram_guest_memfd(MemoryRegion *mr, Object *owner,
+                                        const char *name, uint64_t size,
                                         Error **errp)
 {
-    DeviceState *owner_dev;
-
     if (!memory_region_init_ram_flags_nomigrate(mr, owner, name, size,
                                                 RAM_GUEST_MEMFD, errp)) {
         return false;
     }
-    /* This will assert if owner is neither NULL nor a DeviceState.
-     * We only want the owner here for the purposes of defining a
-     * unique name for migration. TODO: Ideally we should implement
-     * a naming scheme for Objects which are not DeviceStates, in
-     * which case we can relax this restriction.
-     */
-    owner_dev = DEVICE(owner);
-    vmstate_register_ram(mr, owner_dev);
-
+    memory_region_register_ram(mr, owner);
     return true;
 }
 
-bool memory_region_init_rom(MemoryRegion *mr,
-                            Object *owner,
-                            const char *name,
-                            uint64_t size,
+bool memory_region_init_rom(MemoryRegion *mr, Object *owner,
+                            const char *name, uint64_t size,
                             Error **errp)
 {
-    DeviceState *owner_dev;
-
     if (!memory_region_init_ram_flags_nomigrate(mr, owner, name,
                                                 size, 0, errp)) {
         return false;
     }
     mr->readonly = true;
-    /* This will assert if owner is neither NULL nor a DeviceState.
-     * We only want the owner here for the purposes of defining a
-     * unique name for migration. TODO: Ideally we should implement
-     * a naming scheme for Objects which are not DeviceStates, in
-     * which case we can relax this restriction.
-     */
-    owner_dev = DEVICE(owner);
-    vmstate_register_ram(mr, owner_dev);
-
+    memory_region_register_ram(mr, owner);
     return true;
 }
 
-bool memory_region_init_rom_device(MemoryRegion *mr,
-                                   Object *owner,
-                                   const MemoryRegionOps *ops,
-                                   void *opaque,
-                                   const char *name,
-                                   uint64_t size,
+bool memory_region_init_rom_device(MemoryRegion *mr, Object *owner,
+                                   const MemoryRegionOps *ops, void *opaque,
+                                   const char *name, uint64_t size,
                                    Error **errp)
 {
-    DeviceState *owner_dev;
     Error *err = NULL;
 
     assert(ops);
@@ -3732,15 +3706,7 @@ bool memory_region_init_rom_device(MemoryRegion *mr,
     }
     mr->ram = false;
     mr->rom_device = true;
-    /* This will assert if owner is neither NULL nor a DeviceState.
-     * We only want the owner here for the purposes of defining a
-     * unique name for migration. TODO: Ideally we should implement
-     * a naming scheme for Objects which are not DeviceStates, in
-     * which case we can relax this restriction.
-     */
-    owner_dev = DEVICE(owner);
-    vmstate_register_ram(mr, owner_dev);
-
+    memory_region_register_ram(mr, owner);
     return true;
 }
 
-- 
2.41.3

Re: [PATCH v2 5/7] memory: Add internal memory_region_set_ops helper function

[Philippe Mathieu-Daudé]

On 2/2/26 15:28, BALATON Zoltan wrote:
> This is a common operation used at multiple places, add a helper
> function for it.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   system/memory.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v2 2/7] hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()

[Mark Cave-Ayland]

On 02/02/2026 14:28, BALATON Zoltan wrote:

> We could simply convert these to use memory_region_init_rom but that
> would be a migration compatibility break so preserve current behaviour
> for now.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/display/cg3.c | 6 ++++--
>   hw/display/tcx.c | 6 ++++--
>   2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/display/cg3.c b/hw/display/cg3.c
> index 568d6048a6..44966e7586 100644
> --- a/hw/display/cg3.c
> +++ b/hw/display/cg3.c
> @@ -282,8 +282,10 @@ static void cg3_initfn(Object *obj)
>       SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>       CG3State *s = CG3(obj);
>   
> -    memory_region_init_rom_nomigrate(&s->rom, obj, "cg3.prom",
> -                                     FCODE_MAX_ROM_SIZE, &error_fatal);
> +    memory_region_init_ram_flags_nomigrate(&s->rom, obj, "cg3.prom",
> +                                           FCODE_MAX_ROM_SIZE, 0,
> +                                           &error_fatal);
> +    memory_region_set_readonly(&s->rom, true);
>       sysbus_init_mmio(sbd, &s->rom);
>   
>       memory_region_init_io(&s->reg, obj, &cg3_reg_ops, s, "cg3.reg",
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 36cad82abd..87fe7216ba 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -756,8 +756,10 @@ static void tcx_initfn(Object *obj)
>       SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>       TCXState *s = TCX(obj);
>   
> -    memory_region_init_rom_nomigrate(&s->rom, obj, "tcx.prom",
> -                                     FCODE_MAX_ROM_SIZE, &error_fatal);
> +    memory_region_init_ram_flags_nomigrate(&s->rom, obj, "tcx.prom",
> +                                           FCODE_MAX_ROM_SIZE, 0,
> +                                           &error_fatal);
> +    memory_region_set_readonly(&s->rom, true);
>       sysbus_init_mmio(sbd, &s->rom);
>   
>       /* 2/STIP : Stippler */

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH v2 6/7] memory: Factor out common ram region initialization

[Peter Xu]

On Mon, Feb 02, 2026 at 03:28:17PM +0100, BALATON Zoltan wrote:
> -bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
> -                                            Object *owner,
> -                                            const char *name,
> -                                            uint64_t size,
> -                                            uint32_t ram_flags,
> -                                            Error **errp)
> +static bool memory_region_do_init_ram(MemoryRegion *mr,
> +                                      Error *err, Error **errp)

I keep thinking taking two Errors here is almost not readable for an init
function.  It only makes sense to me if it's something like
error_propagate().  E.g. we also have migrate_error_propagate().  I can't
think of anything that should pass in two Errors..

I'm not sure how bad it is we keep the old code as-is.. but if we really
want to provide some helpers, IMHO we should still avoid this, maybe:

memory_region_ram_setup(mr)
{
  mr->ram = true;
  mr->terminates = true;
  mr->destructor = memory_region_destructor_ram;
}

memory_region_error_propagate(mr, err, errp)
{
  mr->size = int128_zero();
  object_unparent(OBJECT(mr));
  error_propagate(errp, err);
}

Then taking one example:

memory_region_init_ram_flags_nomigrate()
{
  Error *err = NULL;
  memory_region_init(mr, owner, name, size);
  memory_region_ram_setup(mr);
  mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
  if (err) {
    memory_region_error_propagate(mr, err, errp);
    return false;
  }
  return true;
}

Thanks,

-- 
Peter Xu