[PATCH v4 0/8] memory: Remove most _nomigrate variants

[PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

v4:
- separate patch converting Sun machines from memory_region_init_ram_nomigrate
- split helper to init ram into two functions: setup and error_propagate
- also use memory_region_init_io in memory_region_init_ram_device_ptr

v3:
- rebased on master after some patches were merged
- drop some more line from memory-region-housekeeping.cocci
- added comment to explain what factored out helper does
- some more clean ups included

BALATON Zoltan (8):
  hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
  memory: Remove memory_region_init_rom_nomigrate()
  sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
  memory: Remove memory_region_init_ram_nomigrate()
  memory: Factor out common ram region initialization
  memory: Add internal memory_region_register_ram function
  memory: Shorten memory_region_init_ram_device_ptr and
    memory_region_init_rom_device
  memory: Factor out more common ram region initialization

 docs/devel/memory.rst                         |   9 +-
 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                       |  49 ----
 .../memory-region-housekeeping.cocci          |  47 ---
 system/memory.c                               | 269 ++++++------------
 10 files changed, 120 insertions(+), 303 deletions(-)

-- 
2.41.3

[PATCH v4 1/8] 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>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 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 v4 2/8] 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          | 28 -------------------
 system/memory.c                               | 19 ++-----------
 4 files changed, 3 insertions(+), 71 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..e45703141a 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);
@@ -80,23 +69,6 @@ expression ERRP;
  ...
 -vmstate_register_ram_global(MR);
 @@
-expression MR;
-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;
-expression NAME;
-expression SIZE;
-expression ERRP;
-@@
 typedef DeviceState;
 identifier device_fn, dev, obj;
 expression E1, E2, E3, E4, E5;
diff --git a/system/memory.c b/system/memory.c
index c51d0798a8..65042bd9fa 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1738,21 +1738,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,
@@ -3757,9 +3742,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 v4 3/8] sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()

[BALATON Zoltan]

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

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/tcx.c   |  5 +++--
 hw/sparc/sun4m.c   | 15 +++++++++------
 hw/sparc64/sun4u.c |  9 +++++----
 3 files changed, 17 insertions(+), 12 deletions(-)

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/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);
 }
-- 
2.41.3

[PATCH v4 4/8] memory: Remove memory_region_init_ram_nomigrate()

[BALATON Zoltan]

This is rarely needed and we can use
memory_region_init_ram_flags_nomigrate() instead which is now the only
nomigrate variant left.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 docs/devel/memory.rst                         |  8 +++----
 hw/display/vga.c                              |  4 ++--
 hw/xtensa/xtfpga.c                            |  4 ++--
 include/system/memory.h                       | 23 -------------------
 .../memory-region-housekeeping.cocci          | 19 ---------------
 system/memory.c                               | 13 ++---------
 6 files changed, 9 insertions(+), 62 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/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/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 e45703141a..b23647a3d8 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,23 +46,10 @@ expression ALIAS, E5, E6, E7, E8;
 -  memory_region_set_readonly(ALIAS, true);
 
 
-// Replace by-hand memory_region_init_ram_nomigrate/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
 // there are none in the tree that can be automatically converted
 // (and only a handful that can be manually converted).
 @@
-expression MR;
-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);
-@@
 typedef DeviceState;
 identifier device_fn, dev, obj;
 expression E1, E2, E3, E4, E5;
diff --git a/system/memory.c b/system/memory.c
index 65042bd9fa..e15f931a8a 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1579,16 +1579,6 @@ void memory_region_init_io(MemoryRegion *mr,
     memory_region_set_ops(mr, ops, opaque);
 }
 
-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,
@@ -3695,7 +3685,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 v4 5/8] memory: Factor out common ram region initialization

[BALATON Zoltan]

Introduce internal helper functions to remove duplicated code from
different memory_region_init_*ram functions.

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

diff --git a/system/memory.c b/system/memory.c
index e15f931a8a..e1f073e1be 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1579,19 +1579,16 @@ 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 void memory_region_setup_ram(MemoryRegion *mr)
 {
-    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);
+}
+
+static bool memory_region_error_propagate(MemoryRegion *mr,
+                                          Error *err, Error **errp)
+{
     if (err) {
         mr->size = int128_zero();
         object_unparent(OBJECT(mr));
@@ -1601,6 +1598,18 @@ 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);
+    memory_region_setup_ram(mr);
+    mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
+    return memory_region_error_propagate(mr, err, errp);
+}
+
 bool memory_region_init_resizeable_ram(MemoryRegion *mr,
                                        Object *owner,
                                        const char *name,
@@ -1612,108 +1621,69 @@ 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;
+    memory_region_setup_ram(mr);
+    mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr,
+                                              &err);
+    return memory_region_error_propagate(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;
+    memory_region_setup_ram(mr);
     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_error_propagate(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;
+    memory_region_setup_ram(mr);
     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_error_propagate(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;
-
+    memory_region_setup_ram(mr);
     /* 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);
 }
 
-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;
-
+    memory_region_setup_ram(mr);
     /* 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);
+    mr->ram_device = true;
 }
 
 void memory_region_init_alias(MemoryRegion *mr,
@@ -3764,15 +3734,13 @@ 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;
+    memory_region_setup_ram(mr);
     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_error_propagate(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 v4 6/8] 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 e1f073e1be..198d1d0b92 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3647,18 +3647,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
@@ -3667,68 +3659,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);
@@ -3741,15 +3715,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

[PATCH v4 7/8] memory: Shorten memory_region_init_ram_device_ptr and memory_region_init_rom_device

[BALATON Zoltan]

We can reuse memory_region_init_io in these functions. Also shorten
some other function prototypes.

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

diff --git a/system/memory.c b/system/memory.c
index 198d1d0b92..492bf51f48 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1568,12 +1568,9 @@ static void memory_region_set_ops(MemoryRegion *mr,
     mr->terminates = true;
 }
 
-void memory_region_init_io(MemoryRegion *mr,
-                           Object *owner,
-                           const MemoryRegionOps *ops,
-                           void *opaque,
-                           const char *name,
-                           uint64_t size)
+void memory_region_init_io(MemoryRegion *mr, Object *owner,
+                           const MemoryRegionOps *ops, void *opaque,
+                           const char *name, uint64_t size)
 {
     memory_region_init(mr, owner, name, size);
     memory_region_set_ops(mr, ops, opaque);
@@ -1677,8 +1674,7 @@ 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);
-    memory_region_set_ops(mr, &ram_device_mem_ops, mr);
+    memory_region_init_io(mr, owner, &ram_device_mem_ops, mr, name, size);
     memory_region_setup_ram(mr);
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
@@ -1686,12 +1682,9 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, Object *owner,
     mr->ram_device = true;
 }
 
-void memory_region_init_alias(MemoryRegion *mr,
-                              Object *owner,
-                              const char *name,
-                              MemoryRegion *orig,
-                              hwaddr offset,
-                              uint64_t size)
+void memory_region_init_alias(MemoryRegion *mr, Object *owner,
+                              const char *name, MemoryRegion *orig,
+                              hwaddr offset, uint64_t size)
 {
     memory_region_init(mr, owner, name, size);
     mr->alias = orig;
@@ -3706,8 +3699,7 @@ bool memory_region_init_rom_device(MemoryRegion *mr, Object *owner,
     Error *err = NULL;
 
     assert(ops);
-    memory_region_init(mr, owner, name, size);
-    memory_region_set_ops(mr, ops, opaque);
+    memory_region_init_io(mr, owner, ops, opaque, name, size);
     memory_region_setup_ram(mr);
     mr->ram_block = qemu_ram_alloc(size, 0, mr, &err);
     if (!memory_region_error_propagate(mr, err, errp)) {
-- 
2.41.3

[PATCH v4 8/8] memory: Factor out more common ram region initialization

[BALATON Zoltan]

Introduce internal memory_region_setup_ram_ptr() function to remove
duplicated code from different memory_region_init_ram_*ptr functions.

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

diff --git a/system/memory.c b/system/memory.c
index 492bf51f48..fa3e19e1fd 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1660,25 +1660,28 @@ bool memory_region_init_ram_from_fd(MemoryRegion *mr, Object *owner,
 }
 #endif
 
-void memory_region_init_ram_ptr(MemoryRegion *mr, Object *owner,
-                                const char *name, uint64_t size, void *ptr)
+static void memory_region_setup_ram_ptr(MemoryRegion *mr, uint64_t size,
+                                        void *ptr)
 {
-    memory_region_init(mr, owner, name, size);
     memory_region_setup_ram(mr);
     /* 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);
 }
 
+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);
+    memory_region_setup_ram_ptr(mr, size, ptr);
+}
+
 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);
-    memory_region_setup_ram(mr);
-    /* 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_setup_ram_ptr(mr, size, ptr);
     mr->ram_device = true;
 }
 
-- 
2.41.3

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

On Thu, 5 Feb 2026, BALATON Zoltan wrote:
> v4:
> - separate patch converting Sun machines from memory_region_init_ram_nomigrate
> - split helper to init ram into two functions: setup and error_propagate
> - also use memory_region_init_io in memory_region_init_ram_device_ptr
>
> v3:
> - rebased on master after some patches were merged
> - drop some more line from memory-region-housekeeping.cocci
> - added comment to explain what factored out helper does
> - some more clean ups included
>
> BALATON Zoltan (8):
>  hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
>  memory: Remove memory_region_init_rom_nomigrate()
>  sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>  memory: Remove memory_region_init_ram_nomigrate()
>  memory: Factor out common ram region initialization
>  memory: Add internal memory_region_register_ram function
>  memory: Shorten memory_region_init_ram_device_ptr and
>    memory_region_init_rom_device
>  memory: Factor out more common ram region initialization
>
> docs/devel/memory.rst                         |   9 +-
> 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                       |  49 ----
> .../memory-region-housekeeping.cocci          |  47 ---
> system/memory.c                               | 269 ++++++------------
> 10 files changed, 120 insertions(+), 303 deletions(-)

Ping?

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[Peter Xu]

On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
> v4:
> - separate patch converting Sun machines from memory_region_init_ram_nomigrate
> - split helper to init ram into two functions: setup and error_propagate
> - also use memory_region_init_io in memory_region_init_ram_device_ptr
> 
> v3:
> - rebased on master after some patches were merged
> - drop some more line from memory-region-housekeeping.cocci
> - added comment to explain what factored out helper does
> - some more clean ups included
> 
> BALATON Zoltan (8):
>   hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
>   memory: Remove memory_region_init_rom_nomigrate()
>   sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>   memory: Remove memory_region_init_ram_nomigrate()

Could you help explain why we need to remove the above two small helpers?

Not saying that we can't remove them, but I don't yet see the point of
open-code those readonly=true setup either, or any problem with having the
smaller helpers when ram_flags=0.  Small helpers sometimes help readability.

Some of them are going backwards againist what the cocci scripts were
suggesting previously, like:

- 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);

So you suggest open-code instead.  Why?

Is it required for your follow up cleanups?

Thanks,

>   memory: Factor out common ram region initialization
>   memory: Add internal memory_region_register_ram function
>   memory: Shorten memory_region_init_ram_device_ptr and
>     memory_region_init_rom_device
>   memory: Factor out more common ram region initialization
> 
>  docs/devel/memory.rst                         |   9 +-
>  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                       |  49 ----
>  .../memory-region-housekeeping.cocci          |  47 ---
>  system/memory.c                               | 269 ++++++------------
>  10 files changed, 120 insertions(+), 303 deletions(-)
> 
> -- 
> 2.41.3
> 

-- 
Peter Xu

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

On Tue, 24 Feb 2026, Peter Xu wrote:
> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>> v4:
>> - separate patch converting Sun machines from memory_region_init_ram_nomigrate
>> - split helper to init ram into two functions: setup and error_propagate
>> - also use memory_region_init_io in memory_region_init_ram_device_ptr
>>
>> v3:
>> - rebased on master after some patches were merged
>> - drop some more line from memory-region-housekeeping.cocci
>> - added comment to explain what factored out helper does
>> - some more clean ups included
>>
>> BALATON Zoltan (8):
>>   hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
>>   memory: Remove memory_region_init_rom_nomigrate()
>>   sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>   memory: Remove memory_region_init_ram_nomigrate()
>
> Could you help explain why we need to remove the above two small helpers?

Ideally we should remove all of the nomigrate variants as there are only 
about 3 uses left, everything else is already moved to the init_{ram,rom} 
variants that register the memory region for migration. The is probably 
one legitimate use that is served by memory_region_init_flags_nomigrate 
and other 3 uses are the xtfpga which I think is just old code and does 
not support migration so it will be converted in the second series to 
memory_region_new as the maintainers did not say anyting to that so far. 
Then there's one in hw/display/vga but that's gated behind a compatibility 
switch that's only set in 2.12 (global-vmstate=true) and earlier. This is 
not active any more but cannot yet be removed as there are eariler compat 
props still need cleaning up but the last visible machine version is 5.1 
now so this is long deprecated and not used. And there are these Sun 
machines that I've converted in the first version but Mark said he'd like 
to keep the current behaviour until it's the last user left. See: 
https://patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/

> Not saying that we can't remove them, but I don't yet see the point of
> open-code those readonly=true setup either, or any problem with having the
> smaller helpers when ram_flags=0.  Small helpers sometimes help readability.
>
> Some of them are going backwards againist what the cocci scripts were
> suggesting previously, like:
>
> - 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);
>
> So you suggest open-code instead.  Why?

This is temporary until the usage in vga is gone with hw_compat_2_12 then 
the Sun machines will be the last users so they can also be converted to 
init_{ram,rom} (unless Mark changes his mind and agrees changing these 
now as was in the first version).

> Is it required for your follow up cleanups?

It makes those simpler as then we don't need to add memory_region_new 
variants for the nomigrate versions that we would remove later anyway.

Regards,
BALATON Zoltan

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[Peter Maydell]

On Tue, 24 Feb 2026 at 17:45, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Tue, 24 Feb 2026, Peter Xu wrote:
> > On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
> >> v4:
> >> - separate patch converting Sun machines from memory_region_init_ram_nomigrate
> >> - split helper to init ram into two functions: setup and error_propagate
> >> - also use memory_region_init_io in memory_region_init_ram_device_ptr
> >>
> >> v3:
> >> - rebased on master after some patches were merged
> >> - drop some more line from memory-region-housekeeping.cocci
> >> - added comment to explain what factored out helper does
> >> - some more clean ups included
> >>
> >> BALATON Zoltan (8):
> >>   hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
> >>   memory: Remove memory_region_init_rom_nomigrate()
> >>   sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
> >>   memory: Remove memory_region_init_ram_nomigrate()
> >
> > Could you help explain why we need to remove the above two small helpers?
>
> Ideally we should remove all of the nomigrate variants as there are only
> about 3 uses left, everything else is already moved to the init_{ram,rom}
> variants that register the memory region for migration. The is probably
> one legitimate use that is served by memory_region_init_flags_nomigrate
> and other 3 uses are the xtfpga which I think is just old code and does
> not support migration so it will be converted in the second series to
> memory_region_new as the maintainers did not say anyting to that so far.
> Then there's one in hw/display/vga but that's gated behind a compatibility
> switch that's only set in 2.12 (global-vmstate=true) and earlier. This is
> not active any more but cannot yet be removed as there are eariler compat
> props still need cleaning up but the last visible machine version is 5.1
> now so this is long deprecated and not used. And there are these Sun
> machines that I've converted in the first version but Mark said he'd like
> to keep the current behaviour until it's the last user left. See:
> https://patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>
> > Not saying that we can't remove them, but I don't yet see the point of
> > open-code those readonly=true setup either, or any problem with having the
> > smaller helpers when ram_flags=0.  Small helpers sometimes help readability.
> >
> > Some of them are going backwards againist what the cocci scripts were
> > suggesting previously, like:
> >
> > - 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);
> >
> > So you suggest open-code instead.  Why?
>
> This is temporary until the usage in vga is gone with hw_compat_2_12 then
> the Sun machines will be the last users so they can also be converted to
> init_{ram,rom} (unless Mark changes his mind and agrees changing these
> now as was in the first version).

I think we should take the migration-compat break in the Sun machines
for the sake of cleaning up those callsites now. Sun isn't quite the
last thing standing using these functions, but it almost is, and as
you say the xtfpga can go too.

-- PMM

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[Peter Xu]

On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
> On Tue, 24 Feb 2026, Peter Xu wrote:
> > On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
> > > v4:
> > > - separate patch converting Sun machines from memory_region_init_ram_nomigrate
> > > - split helper to init ram into two functions: setup and error_propagate
> > > - also use memory_region_init_io in memory_region_init_ram_device_ptr
> > > 
> > > v3:
> > > - rebased on master after some patches were merged
> > > - drop some more line from memory-region-housekeeping.cocci
> > > - added comment to explain what factored out helper does
> > > - some more clean ups included
> > > 
> > > BALATON Zoltan (8):
> > >   hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
> > >   memory: Remove memory_region_init_rom_nomigrate()
> > >   sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
> > >   memory: Remove memory_region_init_ram_nomigrate()
> > 
> > Could you help explain why we need to remove the above two small helpers?
> 
> Ideally we should remove all of the nomigrate variants as there are only
> about 3 uses left, everything else is already moved to the init_{ram,rom}
> variants that register the memory region for migration. The is probably one
> legitimate use that is served by memory_region_init_flags_nomigrate and
> other 3 uses are the xtfpga which I think is just old code and does not
> support migration so it will be converted in the second series to
> memory_region_new as the maintainers did not say anyting to that so far.
> Then there's one in hw/display/vga but that's gated behind a compatibility
> switch that's only set in 2.12 (global-vmstate=true) and earlier. This is
> not active any more but cannot yet be removed as there are eariler compat
> props still need cleaning up but the last visible machine version is 5.1 now
> so this is long deprecated and not used. And there are these Sun machines
> that I've converted in the first version but Mark said he'd like to keep the
> current behaviour until it's the last user left. See: https://patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
> 
> > Not saying that we can't remove them, but I don't yet see the point of
> > open-code those readonly=true setup either, or any problem with having the
> > smaller helpers when ram_flags=0.  Small helpers sometimes help readability.
> > 
> > Some of them are going backwards againist what the cocci scripts were
> > suggesting previously, like:
> > 
> > - 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);
> > 
> > So you suggest open-code instead.  Why?
> 
> This is temporary until the usage in vga is gone with hw_compat_2_12 then
> the Sun machines will be the last users so they can also be converted to
> init_{ram,rom} (unless Mark changes his mind and agrees changing these now
> as was in the first version).
> 
> > Is it required for your follow up cleanups?
> 
> It makes those simpler as then we don't need to add memory_region_new
> variants for the nomigrate versions that we would remove later anyway.

Personally I don't see the memory_region_new() whole thing solid yet, but
still during review.  I shared my thoughts.  So far, I'm not fully
convinced, but I also don't have the full picture to say no, either.  We
can wait for others to chime in with more thoughts.

For this series, I would prefer if we can discuss it separately from the
other work.

Removing _nomigrate() variances makes sense to me in general, it means we
want to by default suggest new devices only use RAM regions that is
migratable, with proper owner device specified along with the ramblock name
(as part of migration API).

However, we have two sets of these _nomigrate APIs:

  (1) memory_region_init_r[ao]m_nomigrate
  (2) memory_region_init_r[ao]m_flags_nomigrate

What I am still confused about is why this series used (2) to replace (1)
rather than removing both.  I don't understand how this helps if we still
have (2) around: it means new devices at least can still use (2) to bypass
migration.

There's actually one way to enforce migratable RAMs, taking ROM as example,
it could be:

  memory_region_init_rom_internal(..., global_vmstate=XXX)

Then memory_region_init_rom() should use that, setting global_vmstate=off.

Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() but
use that _internal() in relevant paths; we should also mark the _internal()
functions to say "new code is suggested to use the normal version".  But I
do not know if that is a slight overkill just for this..

If the other series (to avoid introducing new APIs) is the only reason for
this part, then IMHO this part of the series needs to be in the other
series, not this one.  The last cleanup patches look more self contained
OTOH to me, if the whole purpose of the latter half was code dedup, then it
makes more sense at least to me.

Thanks,

-- 
Peter Xu

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

On Tue, 24 Feb 2026, Peter Maydell wrote:
> On Tue, 24 Feb 2026 at 17:45, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>
>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>> v4:
>>>> - separate patch converting Sun machines from memory_region_init_ram_nomigrate
>>>> - split helper to init ram into two functions: setup and error_propagate
>>>> - also use memory_region_init_io in memory_region_init_ram_device_ptr
>>>>
>>>> v3:
>>>> - rebased on master after some patches were merged
>>>> - drop some more line from memory-region-housekeeping.cocci
>>>> - added comment to explain what factored out helper does
>>>> - some more clean ups included
>>>>
>>>> BALATON Zoltan (8):
>>>>   hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
>>>>   memory: Remove memory_region_init_rom_nomigrate()
>>>>   sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>   memory: Remove memory_region_init_ram_nomigrate()
>>>
>>> Could you help explain why we need to remove the above two small helpers?
>>
>> Ideally we should remove all of the nomigrate variants as there are only
>> about 3 uses left, everything else is already moved to the init_{ram,rom}
>> variants that register the memory region for migration. The is probably
>> one legitimate use that is served by memory_region_init_flags_nomigrate
>> and other 3 uses are the xtfpga which I think is just old code and does
>> not support migration so it will be converted in the second series to
>> memory_region_new as the maintainers did not say anyting to that so far.
>> Then there's one in hw/display/vga but that's gated behind a compatibility
>> switch that's only set in 2.12 (global-vmstate=true) and earlier. This is
>> not active any more but cannot yet be removed as there are eariler compat
>> props still need cleaning up but the last visible machine version is 5.1
>> now so this is long deprecated and not used. And there are these Sun
>> machines that I've converted in the first version but Mark said he'd like
>> to keep the current behaviour until it's the last user left. See:
>> https://patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>
>>> Not saying that we can't remove them, but I don't yet see the point of
>>> open-code those readonly=true setup either, or any problem with having the
>>> smaller helpers when ram_flags=0.  Small helpers sometimes help readability.
>>>
>>> Some of them are going backwards againist what the cocci scripts were
>>> suggesting previously, like:
>>>
>>> - 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);
>>>
>>> So you suggest open-code instead.  Why?
>>
>> This is temporary until the usage in vga is gone with hw_compat_2_12 then
>> the Sun machines will be the last users so they can also be converted to
>> init_{ram,rom} (unless Mark changes his mind and agrees changing these
>> now as was in the first version).
>
> I think we should take the migration-compat break in the Sun machines
> for the sake of cleaning up those callsites now. Sun isn't quite the
> last thing standing using these functions, but it almost is, and as
> you say the xtfpga can go too.

Mark, would it be OK to convert the Sun machines to use 
memory_region_init_rom now so we can remove the nomigrate variant more 
easily? The xtfpga can be converted now and the usage in vga is only there 
because the 2.12 hw_compat is not yet removed but is not anymore used and 
should be gone eventually so at the latest then these usages will be the 
last one and would be converted. If you agree to not wait until then, I 
can send another version of this clean up series with the first version of 
the patch that converts the Sun display devices to memory_region_init_rom.

Regards,
BALATON Zoltan

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[Akihiko Odaki]

On 2026/02/26 1:30, Peter Xu wrote:
> On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>> v4:
>>>> - separate patch converting Sun machines from memory_region_init_ram_nomigrate
>>>> - split helper to init ram into two functions: setup and error_propagate
>>>> - also use memory_region_init_io in memory_region_init_ram_device_ptr
>>>>
>>>> v3:
>>>> - rebased on master after some patches were merged
>>>> - drop some more line from memory-region-housekeeping.cocci
>>>> - added comment to explain what factored out helper does
>>>> - some more clean ups included
>>>>
>>>> BALATON Zoltan (8):
>>>>    hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
>>>>    memory: Remove memory_region_init_rom_nomigrate()
>>>>    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>    memory: Remove memory_region_init_ram_nomigrate()
>>>
>>> Could you help explain why we need to remove the above two small helpers?
>>
>> Ideally we should remove all of the nomigrate variants as there are only
>> about 3 uses left, everything else is already moved to the init_{ram,rom}
>> variants that register the memory region for migration. The is probably one
>> legitimate use that is served by memory_region_init_flags_nomigrate and
>> other 3 uses are the xtfpga which I think is just old code and does not
>> support migration so it will be converted in the second series to
>> memory_region_new as the maintainers did not say anyting to that so far.
>> Then there's one in hw/display/vga but that's gated behind a compatibility
>> switch that's only set in 2.12 (global-vmstate=true) and earlier. This is
>> not active any more but cannot yet be removed as there are eariler compat
>> props still need cleaning up but the last visible machine version is 5.1 now
>> so this is long deprecated and not used. And there are these Sun machines
>> that I've converted in the first version but Mark said he'd like to keep the
>> current behaviour until it's the last user left. See: https://patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>
>>> Not saying that we can't remove them, but I don't yet see the point of
>>> open-code those readonly=true setup either, or any problem with having the
>>> smaller helpers when ram_flags=0.  Small helpers sometimes help readability.
>>>
>>> Some of them are going backwards againist what the cocci scripts were
>>> suggesting previously, like:
>>>
>>> - 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);
>>>
>>> So you suggest open-code instead.  Why?
>>
>> This is temporary until the usage in vga is gone with hw_compat_2_12 then
>> the Sun machines will be the last users so they can also be converted to
>> init_{ram,rom} (unless Mark changes his mind and agrees changing these now
>> as was in the first version).
>>
>>> Is it required for your follow up cleanups?
>>
>> It makes those simpler as then we don't need to add memory_region_new
>> variants for the nomigrate versions that we would remove later anyway.
> 
> Personally I don't see the memory_region_new() whole thing solid yet, but
> still during review.  I shared my thoughts.  So far, I'm not fully
> convinced, but I also don't have the full picture to say no, either.  We
> can wait for others to chime in with more thoughts.
> 
> For this series, I would prefer if we can discuss it separately from the
> other work.
> 
> Removing _nomigrate() variances makes sense to me in general, it means we
> want to by default suggest new devices only use RAM regions that is
> migratable, with proper owner device specified along with the ramblock name
> (as part of migration API).
> 
> However, we have two sets of these _nomigrate APIs:
> 
>    (1) memory_region_init_r[ao]m_nomigrate
>    (2) memory_region_init_r[ao]m_flags_nomigrate
> 
> What I am still confused about is why this series used (2) to replace (1)
> rather than removing both.  I don't understand how this helps if we still
> have (2) around: it means new devices at least can still use (2) to bypass
> migration.
> 
> There's actually one way to enforce migratable RAMs, taking ROM as example,
> it could be:
> 
>    memory_region_init_rom_internal(..., global_vmstate=XXX)
> 
> Then memory_region_init_rom() should use that, setting global_vmstate=off.
> 
> Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() but
> use that _internal() in relevant paths; we should also mark the _internal()
> functions to say "new code is suggested to use the normal version".  But I
> do not know if that is a slight overkill just for this..
> 
> If the other series (to avoid introducing new APIs) is the only reason for
> this part, then IMHO this part of the series needs to be in the other
> series, not this one.  The last cleanup patches look more self contained
> OTOH to me, if the whole purpose of the latter half was code dedup, then it
> makes more sense at least to me.

+1 for what Peter Xu suggested. Most part of this series looks 
independently useful, but I don't think the removal of 
memory_region_init_ram_nomigrate() is.

Regards,
Akihiko Odaki

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

On Mon, 2 Mar 2026, Akihiko Odaki wrote:
> On 2026/02/26 1:30, Peter Xu wrote:
>> On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
>>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>>> v4:
>>>>> - separate patch converting Sun machines from 
>>>>> memory_region_init_ram_nomigrate
>>>>> - split helper to init ram into two functions: setup and error_propagate
>>>>> - also use memory_region_init_io in memory_region_init_ram_device_ptr
>>>>> 
>>>>> v3:
>>>>> - rebased on master after some patches were merged
>>>>> - drop some more line from memory-region-housekeeping.cocci
>>>>> - added comment to explain what factored out helper does
>>>>> - some more clean ups included
>>>>> 
>>>>> BALATON Zoltan (8):
>>>>>    hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
>>>>>    memory: Remove memory_region_init_rom_nomigrate()
>>>>>    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>>    memory: Remove memory_region_init_ram_nomigrate()
>>>> 
>>>> Could you help explain why we need to remove the above two small helpers?
>>> 
>>> Ideally we should remove all of the nomigrate variants as there are only
>>> about 3 uses left, everything else is already moved to the init_{ram,rom}
>>> variants that register the memory region for migration. The is probably 
>>> one
>>> legitimate use that is served by memory_region_init_flags_nomigrate and
>>> other 3 uses are the xtfpga which I think is just old code and does not
>>> support migration so it will be converted in the second series to
>>> memory_region_new as the maintainers did not say anyting to that so far.
>>> Then there's one in hw/display/vga but that's gated behind a compatibility
>>> switch that's only set in 2.12 (global-vmstate=true) and earlier. This is
>>> not active any more but cannot yet be removed as there are eariler compat
>>> props still need cleaning up but the last visible machine version is 5.1 
>>> now
>>> so this is long deprecated and not used. And there are these Sun machines
>>> that I've converted in the first version but Mark said he'd like to keep 
>>> the
>>> current behaviour until it's the last user left. See: 
>>> https://patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>> 
>>>> Not saying that we can't remove them, but I don't yet see the point of
>>>> open-code those readonly=true setup either, or any problem with having 
>>>> the
>>>> smaller helpers when ram_flags=0.  Small helpers sometimes help 
>>>> readability.
>>>> 
>>>> Some of them are going backwards againist what the cocci scripts were
>>>> suggesting previously, like:
>>>> 
>>>> - 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);
>>>> 
>>>> So you suggest open-code instead.  Why?
>>> 
>>> This is temporary until the usage in vga is gone with hw_compat_2_12 then
>>> the Sun machines will be the last users so they can also be converted to
>>> init_{ram,rom} (unless Mark changes his mind and agrees changing these now
>>> as was in the first version).
>>> 
>>>> Is it required for your follow up cleanups?
>>> 
>>> It makes those simpler as then we don't need to add memory_region_new
>>> variants for the nomigrate versions that we would remove later anyway.
>> 
>> Personally I don't see the memory_region_new() whole thing solid yet, but
>> still during review.  I shared my thoughts.  So far, I'm not fully
>> convinced, but I also don't have the full picture to say no, either.  We
>> can wait for others to chime in with more thoughts.
>> 
>> For this series, I would prefer if we can discuss it separately from the
>> other work.
>> 
>> Removing _nomigrate() variances makes sense to me in general, it means we
>> want to by default suggest new devices only use RAM regions that is
>> migratable, with proper owner device specified along with the ramblock name
>> (as part of migration API).
>> 
>> However, we have two sets of these _nomigrate APIs:
>>
>>    (1) memory_region_init_r[ao]m_nomigrate
>>    (2) memory_region_init_r[ao]m_flags_nomigrate
>> 
>> What I am still confused about is why this series used (2) to replace (1)
>> rather than removing both.  I don't understand how this helps if we still
>> have (2) around: it means new devices at least can still use (2) to bypass
>> migration.
>> 
>> There's actually one way to enforce migratable RAMs, taking ROM as example,
>> it could be:
>>
>>    memory_region_init_rom_internal(..., global_vmstate=XXX)
>> 
>> Then memory_region_init_rom() should use that, setting global_vmstate=off.
>> 
>> Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() but
>> use that _internal() in relevant paths; we should also mark the _internal()
>> functions to say "new code is suggested to use the normal version".  But I
>> do not know if that is a slight overkill just for this..
>> 
>> If the other series (to avoid introducing new APIs) is the only reason for
>> this part, then IMHO this part of the series needs to be in the other
>> series, not this one.  The last cleanup patches look more self contained
>> OTOH to me, if the whole purpose of the latter half was code dedup, then it
>> makes more sense at least to me.
>
> +1 for what Peter Xu suggested. Most part of this series looks independently 
> useful, but I don't think the removal of memory_region_init_ram_nomigrate() 
> is.

But if there are no useful uses why keep it? This version shows that 
converting the Sun machines as Peter Maydell suggested only leaves very 
few uses that can be removed eventually too after some further clean up 
that I don't want to take up now. Do you have any usage in mind that needs 
these nomigrate variants?

Regards,
BALATON Zoltan

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[Akihiko Odaki]

On 2026/03/02 21:26, BALATON Zoltan wrote:
> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>> On 2026/02/26 1:30, Peter Xu wrote:
>>> On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
>>>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>>>> v4:
>>>>>> - separate patch converting Sun machines from 
>>>>>> memory_region_init_ram_nomigrate
>>>>>> - split helper to init ram into two functions: setup and 
>>>>>> error_propagate
>>>>>> - also use memory_region_init_io in memory_region_init_ram_device_ptr
>>>>>>
>>>>>> v3:
>>>>>> - rebased on master after some patches were merged
>>>>>> - drop some more line from memory-region-housekeeping.cocci
>>>>>> - added comment to explain what factored out helper does
>>>>>> - some more clean ups included
>>>>>>
>>>>>> BALATON Zoltan (8):
>>>>>>    hw/display/{cg3,tcx}: Do not use 
>>>>>> memory_region_init_rom_nomigrate()
>>>>>>    memory: Remove memory_region_init_rom_nomigrate()
>>>>>>    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>>>    memory: Remove memory_region_init_ram_nomigrate()
>>>>>
>>>>> Could you help explain why we need to remove the above two small 
>>>>> helpers?
>>>>
>>>> Ideally we should remove all of the nomigrate variants as there are 
>>>> only
>>>> about 3 uses left, everything else is already moved to the 
>>>> init_{ram,rom}
>>>> variants that register the memory region for migration. The is 
>>>> probably one
>>>> legitimate use that is served by memory_region_init_flags_nomigrate and
>>>> other 3 uses are the xtfpga which I think is just old code and does not
>>>> support migration so it will be converted in the second series to
>>>> memory_region_new as the maintainers did not say anyting to that so 
>>>> far.
>>>> Then there's one in hw/display/vga but that's gated behind a 
>>>> compatibility
>>>> switch that's only set in 2.12 (global-vmstate=true) and earlier. 
>>>> This is
>>>> not active any more but cannot yet be removed as there are eariler 
>>>> compat
>>>> props still need cleaning up but the last visible machine version is 
>>>> 5.1 now
>>>> so this is long deprecated and not used. And there are these Sun 
>>>> machines
>>>> that I've converted in the first version but Mark said he'd like to 
>>>> keep the
>>>> current behaviour until it's the last user left. See: https:// 
>>>> patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/ 
>>>> b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>>>
>>>>> Not saying that we can't remove them, but I don't yet see the point of
>>>>> open-code those readonly=true setup either, or any problem with 
>>>>> having the
>>>>> smaller helpers when ram_flags=0.  Small helpers sometimes help 
>>>>> readability.
>>>>>
>>>>> Some of them are going backwards againist what the cocci scripts were
>>>>> suggesting previously, like:
>>>>>
>>>>> - 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);
>>>>>
>>>>> So you suggest open-code instead.  Why?
>>>>
>>>> This is temporary until the usage in vga is gone with hw_compat_2_12 
>>>> then
>>>> the Sun machines will be the last users so they can also be 
>>>> converted to
>>>> init_{ram,rom} (unless Mark changes his mind and agrees changing 
>>>> these now
>>>> as was in the first version).
>>>>
>>>>> Is it required for your follow up cleanups?
>>>>
>>>> It makes those simpler as then we don't need to add memory_region_new
>>>> variants for the nomigrate versions that we would remove later anyway.
>>>
>>> Personally I don't see the memory_region_new() whole thing solid yet, 
>>> but
>>> still during review.  I shared my thoughts.  So far, I'm not fully
>>> convinced, but I also don't have the full picture to say no, either.  We
>>> can wait for others to chime in with more thoughts.
>>>
>>> For this series, I would prefer if we can discuss it separately from the
>>> other work.
>>>
>>> Removing _nomigrate() variances makes sense to me in general, it 
>>> means we
>>> want to by default suggest new devices only use RAM regions that is
>>> migratable, with proper owner device specified along with the 
>>> ramblock name
>>> (as part of migration API).
>>>
>>> However, we have two sets of these _nomigrate APIs:
>>>
>>>    (1) memory_region_init_r[ao]m_nomigrate
>>>    (2) memory_region_init_r[ao]m_flags_nomigrate
>>>
>>> What I am still confused about is why this series used (2) to replace 
>>> (1)
>>> rather than removing both.  I don't understand how this helps if we 
>>> still
>>> have (2) around: it means new devices at least can still use (2) to 
>>> bypass
>>> migration.
>>>
>>> There's actually one way to enforce migratable RAMs, taking ROM as 
>>> example,
>>> it could be:
>>>
>>>    memory_region_init_rom_internal(..., global_vmstate=XXX)
>>>
>>> Then memory_region_init_rom() should use that, setting 
>>> global_vmstate=off.
>>>
>>> Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() 
>>> but
>>> use that _internal() in relevant paths; we should also mark the 
>>> _internal()
>>> functions to say "new code is suggested to use the normal version".  
>>> But I
>>> do not know if that is a slight overkill just for this..
>>>
>>> If the other series (to avoid introducing new APIs) is the only 
>>> reason for
>>> this part, then IMHO this part of the series needs to be in the other
>>> series, not this one.  The last cleanup patches look more self contained
>>> OTOH to me, if the whole purpose of the latter half was code dedup, 
>>> then it
>>> makes more sense at least to me.
>>
>> +1 for what Peter Xu suggested. Most part of this series looks 
>> independently useful, but I don't think the removal of 
>> memory_region_init_ram_nomigrate() is.
> 
> But if there are no useful uses why keep it? This version shows that 
> converting the Sun machines as Peter Maydell suggested only leaves very 
> few uses that can be removed eventually too after some further clean up 
> that I don't want to take up now. Do you have any usage in mind that 
> needs these nomigrate variants?

Well, my threshold for "useful" is > 1 uses, but maintainers may have 
other preferences.

Regards,
Akihiko Odaki

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 7741 bytes --]

On Mon, 2 Mar 2026, Akihiko Odaki wrote:
> On 2026/03/02 21:26, BALATON Zoltan wrote:
>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>> On 2026/02/26 1:30, Peter Xu wrote:
>>>> On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
>>>>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>>>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>>>>> v4:
>>>>>>> - separate patch converting Sun machines from 
>>>>>>> memory_region_init_ram_nomigrate
>>>>>>> - split helper to init ram into two functions: setup and 
>>>>>>> error_propagate
>>>>>>> - also use memory_region_init_io in memory_region_init_ram_device_ptr
>>>>>>> 
>>>>>>> v3:
>>>>>>> - rebased on master after some patches were merged
>>>>>>> - drop some more line from memory-region-housekeeping.cocci
>>>>>>> - added comment to explain what factored out helper does
>>>>>>> - some more clean ups included
>>>>>>> 
>>>>>>> BALATON Zoltan (8):
>>>>>>>    hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
>>>>>>>    memory: Remove memory_region_init_rom_nomigrate()
>>>>>>>    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>>>>    memory: Remove memory_region_init_ram_nomigrate()
>>>>>> 
>>>>>> Could you help explain why we need to remove the above two small 
>>>>>> helpers?
>>>>> 
>>>>> Ideally we should remove all of the nomigrate variants as there are only
>>>>> about 3 uses left, everything else is already moved to the 
>>>>> init_{ram,rom}
>>>>> variants that register the memory region for migration. The is probably 
>>>>> one
>>>>> legitimate use that is served by memory_region_init_flags_nomigrate and
>>>>> other 3 uses are the xtfpga which I think is just old code and does not
>>>>> support migration so it will be converted in the second series to
>>>>> memory_region_new as the maintainers did not say anyting to that so far.
>>>>> Then there's one in hw/display/vga but that's gated behind a 
>>>>> compatibility
>>>>> switch that's only set in 2.12 (global-vmstate=true) and earlier. This 
>>>>> is
>>>>> not active any more but cannot yet be removed as there are eariler 
>>>>> compat
>>>>> props still need cleaning up but the last visible machine version is 5.1 
>>>>> now
>>>>> so this is long deprecated and not used. And there are these Sun 
>>>>> machines
>>>>> that I've converted in the first version but Mark said he'd like to keep 
>>>>> the
>>>>> current behaviour until it's the last user left. See: https:// 
>>>>> patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/ 
>>>>> b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>>>> 
>>>>>> Not saying that we can't remove them, but I don't yet see the point of
>>>>>> open-code those readonly=true setup either, or any problem with having 
>>>>>> the
>>>>>> smaller helpers when ram_flags=0.  Small helpers sometimes help 
>>>>>> readability.
>>>>>> 
>>>>>> Some of them are going backwards againist what the cocci scripts were
>>>>>> suggesting previously, like:
>>>>>> 
>>>>>> - 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);
>>>>>> 
>>>>>> So you suggest open-code instead.  Why?
>>>>> 
>>>>> This is temporary until the usage in vga is gone with hw_compat_2_12 
>>>>> then
>>>>> the Sun machines will be the last users so they can also be converted to
>>>>> init_{ram,rom} (unless Mark changes his mind and agrees changing these 
>>>>> now
>>>>> as was in the first version).
>>>>> 
>>>>>> Is it required for your follow up cleanups?
>>>>> 
>>>>> It makes those simpler as then we don't need to add memory_region_new
>>>>> variants for the nomigrate versions that we would remove later anyway.
>>>> 
>>>> Personally I don't see the memory_region_new() whole thing solid yet, but
>>>> still during review.  I shared my thoughts.  So far, I'm not fully
>>>> convinced, but I also don't have the full picture to say no, either.  We
>>>> can wait for others to chime in with more thoughts.
>>>> 
>>>> For this series, I would prefer if we can discuss it separately from the
>>>> other work.
>>>> 
>>>> Removing _nomigrate() variances makes sense to me in general, it means we
>>>> want to by default suggest new devices only use RAM regions that is
>>>> migratable, with proper owner device specified along with the ramblock 
>>>> name
>>>> (as part of migration API).
>>>> 
>>>> However, we have two sets of these _nomigrate APIs:
>>>> 
>>>>    (1) memory_region_init_r[ao]m_nomigrate
>>>>    (2) memory_region_init_r[ao]m_flags_nomigrate
>>>> 
>>>> What I am still confused about is why this series used (2) to replace (1)
>>>> rather than removing both.  I don't understand how this helps if we still
>>>> have (2) around: it means new devices at least can still use (2) to 
>>>> bypass
>>>> migration.
>>>> 
>>>> There's actually one way to enforce migratable RAMs, taking ROM as 
>>>> example,
>>>> it could be:
>>>> 
>>>>    memory_region_init_rom_internal(..., global_vmstate=XXX)
>>>> 
>>>> Then memory_region_init_rom() should use that, setting 
>>>> global_vmstate=off.
>>>> 
>>>> Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() but
>>>> use that _internal() in relevant paths; we should also mark the 
>>>> _internal()
>>>> functions to say "new code is suggested to use the normal version".  But 
>>>> I
>>>> do not know if that is a slight overkill just for this..
>>>> 
>>>> If the other series (to avoid introducing new APIs) is the only reason 
>>>> for
>>>> this part, then IMHO this part of the series needs to be in the other
>>>> series, not this one.  The last cleanup patches look more self contained
>>>> OTOH to me, if the whole purpose of the latter half was code dedup, then 
>>>> it
>>>> makes more sense at least to me.
>>> 
>>> +1 for what Peter Xu suggested. Most part of this series looks 
>>> independently useful, but I don't think the removal of 
>>> memory_region_init_ram_nomigrate() is.
>> 
>> But if there are no useful uses why keep it? This version shows that 
>> converting the Sun machines as Peter Maydell suggested only leaves very few 
>> uses that can be removed eventually too after some further clean up that I 
>> don't want to take up now. Do you have any usage in mind that needs these 
>> nomigrate variants?
>
> Well, my threshold for "useful" is > 1 uses, but maintainers may have other 
> preferences.

After this series when converting the Sun machines as in v5 the only 
nomigrate variant left is memory_region_init_ram_flags_nomigrate that is 
equivalent to memory_region_init_ram_nomigrate with flags=0 which appears 
outside of memory.c only in hw/xtensa/xtfpga.c (that will be converted in 
the next series to memory_region_new) and in hw/display/vga.c that can be 
removed when the hw_compat_2_12 is removed. There are two more uses of 
memory_region_init_ram_flags_nomigrate in backends/hostmem-ram.c and 
hw/m68k/next-cube.c but those have non-0 flags parameter. Therefore I 
think there's no need to keep other nomigrate variants than 
memory_region_init_ram_flags_nomigrate as the only two uses outside of 
memory.c that would need memory_region_init_ram_nomigrate are to be 
removed soon and can use flags=0 until then.

I don't know if the other two uses with non-0 flags can be replaced and 
memory_region_init_ram_flags_nomigrate made internal to memory.c but 
that's beyond the scope of this series and could be considered separately. 
Creating nomigrate memory region is not something that should be done or 
need several functions and the very few cases where this might be needed 
can use memory_region_init_ram_flags_nomigrate.

Regards,
BALATON Zoltan

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[Peter Xu]

On Mon, Mar 02, 2026 at 04:20:10PM +0100, BALATON Zoltan wrote:
> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
> > On 2026/03/02 21:26, BALATON Zoltan wrote:
> > > On Mon, 2 Mar 2026, Akihiko Odaki wrote:
> > > > On 2026/02/26 1:30, Peter Xu wrote:
> > > > > On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
> > > > > > On Tue, 24 Feb 2026, Peter Xu wrote:
> > > > > > > On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
> > > > > > > > v4:
> > > > > > > > - separate patch converting Sun machines from
> > > > > > > > memory_region_init_ram_nomigrate
> > > > > > > > - split helper to init ram into two functions:
> > > > > > > > setup and error_propagate
> > > > > > > > - also use memory_region_init_io in memory_region_init_ram_device_ptr
> > > > > > > > 
> > > > > > > > v3:
> > > > > > > > - rebased on master after some patches were merged
> > > > > > > > - drop some more line from memory-region-housekeeping.cocci
> > > > > > > > - added comment to explain what factored out helper does
> > > > > > > > - some more clean ups included
> > > > > > > > 
> > > > > > > > BALATON Zoltan (8):
> > > > > > > >    hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
> > > > > > > >    memory: Remove memory_region_init_rom_nomigrate()
> > > > > > > >    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
> > > > > > > >    memory: Remove memory_region_init_ram_nomigrate()
> > > > > > > 
> > > > > > > Could you help explain why we need to remove the
> > > > > > > above two small helpers?
> > > > > > 
> > > > > > Ideally we should remove all of the nomigrate variants as there are only
> > > > > > about 3 uses left, everything else is already moved to
> > > > > > the init_{ram,rom}
> > > > > > variants that register the memory region for migration.
> > > > > > The is probably one
> > > > > > legitimate use that is served by memory_region_init_flags_nomigrate and
> > > > > > other 3 uses are the xtfpga which I think is just old code and does not
> > > > > > support migration so it will be converted in the second series to
> > > > > > memory_region_new as the maintainers did not say anyting to that so far.
> > > > > > Then there's one in hw/display/vga but that's gated
> > > > > > behind a compatibility
> > > > > > switch that's only set in 2.12 (global-vmstate=true) and
> > > > > > earlier. This is
> > > > > > not active any more but cannot yet be removed as there
> > > > > > are eariler compat
> > > > > > props still need cleaning up but the last visible
> > > > > > machine version is 5.1 now
> > > > > > so this is long deprecated and not used. And there are
> > > > > > these Sun machines
> > > > > > that I've converted in the first version but Mark said
> > > > > > he'd like to keep the
> > > > > > current behaviour until it's the last user left. See:
> > > > > > https://
> > > > > > patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/ b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
> > > > > > 
> > > > > > > Not saying that we can't remove them, but I don't yet see the point of
> > > > > > > open-code those readonly=true setup either, or any
> > > > > > > problem with having the
> > > > > > > smaller helpers when ram_flags=0.  Small helpers
> > > > > > > sometimes help readability.
> > > > > > > 
> > > > > > > Some of them are going backwards againist what the cocci scripts were
> > > > > > > suggesting previously, like:
> > > > > > > 
> > > > > > > - 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);
> > > > > > > 
> > > > > > > So you suggest open-code instead.  Why?
> > > > > > 
> > > > > > This is temporary until the usage in vga is gone with
> > > > > > hw_compat_2_12 then
> > > > > > the Sun machines will be the last users so they can also be converted to
> > > > > > init_{ram,rom} (unless Mark changes his mind and agrees
> > > > > > changing these now
> > > > > > as was in the first version).
> > > > > > 
> > > > > > > Is it required for your follow up cleanups?
> > > > > > 
> > > > > > It makes those simpler as then we don't need to add memory_region_new
> > > > > > variants for the nomigrate versions that we would remove later anyway.
> > > > > 
> > > > > Personally I don't see the memory_region_new() whole thing solid yet, but
> > > > > still during review.  I shared my thoughts.  So far, I'm not fully
> > > > > convinced, but I also don't have the full picture to say no, either.  We
> > > > > can wait for others to chime in with more thoughts.
> > > > > 
> > > > > For this series, I would prefer if we can discuss it separately from the
> > > > > other work.
> > > > > 
> > > > > Removing _nomigrate() variances makes sense to me in general, it means we
> > > > > want to by default suggest new devices only use RAM regions that is
> > > > > migratable, with proper owner device specified along with
> > > > > the ramblock name
> > > > > (as part of migration API).
> > > > > 
> > > > > However, we have two sets of these _nomigrate APIs:
> > > > > 
> > > > >    (1) memory_region_init_r[ao]m_nomigrate
> > > > >    (2) memory_region_init_r[ao]m_flags_nomigrate
> > > > > 
> > > > > What I am still confused about is why this series used (2) to replace (1)
> > > > > rather than removing both.  I don't understand how this helps if we still
> > > > > have (2) around: it means new devices at least can still use
> > > > > (2) to bypass
> > > > > migration.
> > > > > 
> > > > > There's actually one way to enforce migratable RAMs, taking
> > > > > ROM as example,
> > > > > it could be:
> > > > > 
> > > > >    memory_region_init_rom_internal(..., global_vmstate=XXX)
> > > > > 
> > > > > Then memory_region_init_rom() should use that, setting
> > > > > global_vmstate=off.
> > > > > 
> > > > > Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() but
> > > > > use that _internal() in relevant paths; we should also mark
> > > > > the _internal()
> > > > > functions to say "new code is suggested to use the normal
> > > > > version".  But I
> > > > > do not know if that is a slight overkill just for this..
> > > > > 
> > > > > If the other series (to avoid introducing new APIs) is the
> > > > > only reason for
> > > > > this part, then IMHO this part of the series needs to be in the other
> > > > > series, not this one.  The last cleanup patches look more self contained
> > > > > OTOH to me, if the whole purpose of the latter half was code
> > > > > dedup, then it
> > > > > makes more sense at least to me.
> > > > 
> > > > +1 for what Peter Xu suggested. Most part of this series looks
> > > > independently useful, but I don't think the removal of
> > > > memory_region_init_ram_nomigrate() is.
> > > 
> > > But if there are no useful uses why keep it? This version shows that
> > > converting the Sun machines as Peter Maydell suggested only leaves
> > > very few uses that can be removed eventually too after some further
> > > clean up that I don't want to take up now. Do you have any usage in
> > > mind that needs these nomigrate variants?
> > 
> > Well, my threshold for "useful" is > 1 uses, but maintainers may have
> > other preferences.

Personally I share the same preference..

When >1 it means we can dedup some code with a function and I normally
think it worthwhile.  That's also why I don't understand why we need to
open code _set_readonly() in some of this series.

Even if for the sake of a new set of APIs, it doesn't need to provide every
single API that we have had in the old set if we don't need it, IMHO.

> 
> After this series when converting the Sun machines as in v5 the only
> nomigrate variant left is memory_region_init_ram_flags_nomigrate that is
> equivalent to memory_region_init_ram_nomigrate with flags=0 which appears
> outside of memory.c only in hw/xtensa/xtfpga.c (that will be converted in
> the next series to memory_region_new) and in hw/display/vga.c that can be
> removed when the hw_compat_2_12 is removed. There are two more uses of
> memory_region_init_ram_flags_nomigrate in backends/hostmem-ram.c and
> hw/m68k/next-cube.c but those have non-0 flags parameter. Therefore I think
> there's no need to keep other nomigrate variants than
> memory_region_init_ram_flags_nomigrate as the only two uses outside of
> memory.c that would need memory_region_init_ram_nomigrate are to be removed
> soon and can use flags=0 until then.
> 
> I don't know if the other two uses with non-0 flags can be replaced and
> memory_region_init_ram_flags_nomigrate made internal to memory.c but that's
> beyond the scope of this series and could be considered separately. Creating
> nomigrate memory region is not something that should be done or need several
> functions and the very few cases where this might be needed can use
> memory_region_init_ram_flags_nomigrate.

The hostmem-ram.c one is slightly special, when created as an object it
still doesn't know its parent device.

I still think it might be helpful if you can split your "cleanup" patches
out of this series.  Note that softfreeze will be next week.

Thanks,

-- 
Peter Xu

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 10310 bytes --]

On Mon, 2 Mar 2026, Peter Xu wrote:
> On Mon, Mar 02, 2026 at 04:20:10PM +0100, BALATON Zoltan wrote:
>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>> On 2026/03/02 21:26, BALATON Zoltan wrote:
>>>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>>>> On 2026/02/26 1:30, Peter Xu wrote:
>>>>>> On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
>>>>>>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>>>>>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>>>>>>> v4:
>>>>>>>>> - separate patch converting Sun machines from
>>>>>>>>> memory_region_init_ram_nomigrate
>>>>>>>>> - split helper to init ram into two functions:
>>>>>>>>> setup and error_propagate
>>>>>>>>> - also use memory_region_init_io in memory_region_init_ram_device_ptr
>>>>>>>>>
>>>>>>>>> v3:
>>>>>>>>> - rebased on master after some patches were merged
>>>>>>>>> - drop some more line from memory-region-housekeeping.cocci
>>>>>>>>> - added comment to explain what factored out helper does
>>>>>>>>> - some more clean ups included
>>>>>>>>>
>>>>>>>>> BALATON Zoltan (8):
>>>>>>>>>    hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
>>>>>>>>>    memory: Remove memory_region_init_rom_nomigrate()
>>>>>>>>>    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>>>>>>    memory: Remove memory_region_init_ram_nomigrate()
>>>>>>>>
>>>>>>>> Could you help explain why we need to remove the
>>>>>>>> above two small helpers?
>>>>>>>
>>>>>>> Ideally we should remove all of the nomigrate variants as there are only
>>>>>>> about 3 uses left, everything else is already moved to
>>>>>>> the init_{ram,rom}
>>>>>>> variants that register the memory region for migration.
>>>>>>> The is probably one
>>>>>>> legitimate use that is served by memory_region_init_flags_nomigrate and
>>>>>>> other 3 uses are the xtfpga which I think is just old code and does not
>>>>>>> support migration so it will be converted in the second series to
>>>>>>> memory_region_new as the maintainers did not say anyting to that so far.
>>>>>>> Then there's one in hw/display/vga but that's gated
>>>>>>> behind a compatibility
>>>>>>> switch that's only set in 2.12 (global-vmstate=true) and
>>>>>>> earlier. This is
>>>>>>> not active any more but cannot yet be removed as there
>>>>>>> are eariler compat
>>>>>>> props still need cleaning up but the last visible
>>>>>>> machine version is 5.1 now
>>>>>>> so this is long deprecated and not used. And there are
>>>>>>> these Sun machines
>>>>>>> that I've converted in the first version but Mark said
>>>>>>> he'd like to keep the
>>>>>>> current behaviour until it's the last user left. See:
>>>>>>> https://
>>>>>>> patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/ b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>>>>>>
>>>>>>>> Not saying that we can't remove them, but I don't yet see the point of
>>>>>>>> open-code those readonly=true setup either, or any
>>>>>>>> problem with having the
>>>>>>>> smaller helpers when ram_flags=0.  Small helpers
>>>>>>>> sometimes help readability.
>>>>>>>>
>>>>>>>> Some of them are going backwards againist what the cocci scripts were
>>>>>>>> suggesting previously, like:
>>>>>>>>
>>>>>>>> - 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);
>>>>>>>>
>>>>>>>> So you suggest open-code instead.  Why?
>>>>>>>
>>>>>>> This is temporary until the usage in vga is gone with
>>>>>>> hw_compat_2_12 then
>>>>>>> the Sun machines will be the last users so they can also be converted to
>>>>>>> init_{ram,rom} (unless Mark changes his mind and agrees
>>>>>>> changing these now
>>>>>>> as was in the first version).
>>>>>>>
>>>>>>>> Is it required for your follow up cleanups?
>>>>>>>
>>>>>>> It makes those simpler as then we don't need to add memory_region_new
>>>>>>> variants for the nomigrate versions that we would remove later anyway.
>>>>>>
>>>>>> Personally I don't see the memory_region_new() whole thing solid yet, but
>>>>>> still during review.  I shared my thoughts.  So far, I'm not fully
>>>>>> convinced, but I also don't have the full picture to say no, either.  We
>>>>>> can wait for others to chime in with more thoughts.
>>>>>>
>>>>>> For this series, I would prefer if we can discuss it separately from the
>>>>>> other work.
>>>>>>
>>>>>> Removing _nomigrate() variances makes sense to me in general, it means we
>>>>>> want to by default suggest new devices only use RAM regions that is
>>>>>> migratable, with proper owner device specified along with
>>>>>> the ramblock name
>>>>>> (as part of migration API).
>>>>>>
>>>>>> However, we have two sets of these _nomigrate APIs:
>>>>>>
>>>>>>    (1) memory_region_init_r[ao]m_nomigrate
>>>>>>    (2) memory_region_init_r[ao]m_flags_nomigrate
>>>>>>
>>>>>> What I am still confused about is why this series used (2) to replace (1)
>>>>>> rather than removing both.  I don't understand how this helps if we still
>>>>>> have (2) around: it means new devices at least can still use
>>>>>> (2) to bypass
>>>>>> migration.
>>>>>>
>>>>>> There's actually one way to enforce migratable RAMs, taking
>>>>>> ROM as example,
>>>>>> it could be:
>>>>>>
>>>>>>    memory_region_init_rom_internal(..., global_vmstate=XXX)
>>>>>>
>>>>>> Then memory_region_init_rom() should use that, setting
>>>>>> global_vmstate=off.
>>>>>>
>>>>>> Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() but
>>>>>> use that _internal() in relevant paths; we should also mark
>>>>>> the _internal()
>>>>>> functions to say "new code is suggested to use the normal
>>>>>> version".  But I
>>>>>> do not know if that is a slight overkill just for this..
>>>>>>
>>>>>> If the other series (to avoid introducing new APIs) is the
>>>>>> only reason for
>>>>>> this part, then IMHO this part of the series needs to be in the other
>>>>>> series, not this one.  The last cleanup patches look more self contained
>>>>>> OTOH to me, if the whole purpose of the latter half was code
>>>>>> dedup, then it
>>>>>> makes more sense at least to me.
>>>>>
>>>>> +1 for what Peter Xu suggested. Most part of this series looks
>>>>> independently useful, but I don't think the removal of
>>>>> memory_region_init_ram_nomigrate() is.
>>>>
>>>> But if there are no useful uses why keep it? This version shows that
>>>> converting the Sun machines as Peter Maydell suggested only leaves
>>>> very few uses that can be removed eventually too after some further
>>>> clean up that I don't want to take up now. Do you have any usage in
>>>> mind that needs these nomigrate variants?
>>>
>>> Well, my threshold for "useful" is > 1 uses, but maintainers may have
>>> other preferences.
>
> Personally I share the same preference..
>
> When >1 it means we can dedup some code with a function and I normally
> think it worthwhile.  That's also why I don't understand why we need to
> open code _set_readonly() in some of this series.

Have you read the latest v5 at all? No open coding of set_readonly or 
useful uses of memory_region_init_r[ao]m_nomigrate left if we accept 
converting the Sun machines now to memory_region_init_r[ao]m which they 
should really use as everything else does that and these are the last 
remaining uses of these nomigrate variants apart from the few special 
cases. It seems these Sun machines were left behind when everything else 
converted away from global vmstate and use the normal 
memory_region_init_r[ao]m where the _nomigrate were left for backward 
compatibility during API conversion. So it's time to finish that 
conversion at last.

> Even if for the sake of a new set of APIs, it doesn't need to provide every
> single API that we have had in the old set if we don't need it, IMHO.
>
>>
>> After this series when converting the Sun machines as in v5 the only
>> nomigrate variant left is memory_region_init_ram_flags_nomigrate that is
>> equivalent to memory_region_init_ram_nomigrate with flags=0 which appears
>> outside of memory.c only in hw/xtensa/xtfpga.c (that will be converted in
>> the next series to memory_region_new) and in hw/display/vga.c that can be
>> removed when the hw_compat_2_12 is removed. There are two more uses of
>> memory_region_init_ram_flags_nomigrate in backends/hostmem-ram.c and
>> hw/m68k/next-cube.c but those have non-0 flags parameter. Therefore I think
>> there's no need to keep other nomigrate variants than
>> memory_region_init_ram_flags_nomigrate as the only two uses outside of
>> memory.c that would need memory_region_init_ram_nomigrate are to be removed
>> soon and can use flags=0 until then.

I tried to explain here that the two uses left that could use 
memory_region_init_ram_nomigrate will be gone shortly so IMO not worth 
keeping this function around for that.

>> I don't know if the other two uses with non-0 flags can be replaced and
>> memory_region_init_ram_flags_nomigrate made internal to memory.c but that's
>> beyond the scope of this series and could be considered separately. Creating
>> nomigrate memory region is not something that should be done or need several
>> functions and the very few cases where this might be needed can use
>> memory_region_init_ram_flags_nomigrate.
>
> The hostmem-ram.c one is slightly special, when created as an object it
> still doesn't know its parent device.

Yes so there might be some special cases but for those it's enough to have 
a single nomigrate variant which this series proposes.

> I still think it might be helpful if you can split your "cleanup" patches
> out of this series.  Note that softfreeze will be next week.

This series _is_ the clean up part split from the add memory_region_new 
functions series so these are all clean ups and the rest is in the other 
series. I don't theres much more to split. Maybe you missed some patch 
emails. Here's the clean up series:

https://patchew.org/QEMU/cover.1772397447.git.balaton@eik.bme.hu/

and here's the rest of my original series based on these clean ups that 
continue with adding and using the memory_region_new functions added in 
that series:

https://patchew.org/QEMU/cover.1770753117.git.balaton@eik.bme.hu/

I'm not sure what do you want to split.

Regards,
BALATON Zoltan

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[Akihiko Odaki]

On 2026/03/03 6:06, BALATON Zoltan wrote:
> On Mon, 2 Mar 2026, Peter Xu wrote:
>> On Mon, Mar 02, 2026 at 04:20:10PM +0100, BALATON Zoltan wrote:
>>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>>> On 2026/03/02 21:26, BALATON Zoltan wrote:
>>>>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>>>>> On 2026/02/26 1:30, Peter Xu wrote:
>>>>>>> On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
>>>>>>>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>>>>>>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>>>>>>>> v4:
>>>>>>>>>> - separate patch converting Sun machines from
>>>>>>>>>> memory_region_init_ram_nomigrate
>>>>>>>>>> - split helper to init ram into two functions:
>>>>>>>>>> setup and error_propagate
>>>>>>>>>> - also use memory_region_init_io in 
>>>>>>>>>> memory_region_init_ram_device_ptr
>>>>>>>>>>
>>>>>>>>>> v3:
>>>>>>>>>> - rebased on master after some patches were merged
>>>>>>>>>> - drop some more line from memory-region-housekeeping.cocci
>>>>>>>>>> - added comment to explain what factored out helper does
>>>>>>>>>> - some more clean ups included
>>>>>>>>>>
>>>>>>>>>> BALATON Zoltan (8):
>>>>>>>>>>    hw/display/{cg3,tcx}: Do not use 
>>>>>>>>>> memory_region_init_rom_nomigrate()
>>>>>>>>>>    memory: Remove memory_region_init_rom_nomigrate()
>>>>>>>>>>    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>>>>>>>    memory: Remove memory_region_init_ram_nomigrate()
>>>>>>>>>
>>>>>>>>> Could you help explain why we need to remove the
>>>>>>>>> above two small helpers?
>>>>>>>>
>>>>>>>> Ideally we should remove all of the nomigrate variants as there 
>>>>>>>> are only
>>>>>>>> about 3 uses left, everything else is already moved to
>>>>>>>> the init_{ram,rom}
>>>>>>>> variants that register the memory region for migration.
>>>>>>>> The is probably one
>>>>>>>> legitimate use that is served by 
>>>>>>>> memory_region_init_flags_nomigrate and
>>>>>>>> other 3 uses are the xtfpga which I think is just old code and 
>>>>>>>> does not
>>>>>>>> support migration so it will be converted in the second series to
>>>>>>>> memory_region_new as the maintainers did not say anyting to that 
>>>>>>>> so far.
>>>>>>>> Then there's one in hw/display/vga but that's gated
>>>>>>>> behind a compatibility
>>>>>>>> switch that's only set in 2.12 (global-vmstate=true) and
>>>>>>>> earlier. This is
>>>>>>>> not active any more but cannot yet be removed as there
>>>>>>>> are eariler compat
>>>>>>>> props still need cleaning up but the last visible
>>>>>>>> machine version is 5.1 now
>>>>>>>> so this is long deprecated and not used. And there are
>>>>>>>> these Sun machines
>>>>>>>> that I've converted in the first version but Mark said
>>>>>>>> he'd like to keep the
>>>>>>>> current behaviour until it's the last user left. See:
>>>>>>>> https://
>>>>>>>> patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/ 
>>>>>>>> b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>>>>>>>
>>>>>>>>> Not saying that we can't remove them, but I don't yet see the 
>>>>>>>>> point of
>>>>>>>>> open-code those readonly=true setup either, or any
>>>>>>>>> problem with having the
>>>>>>>>> smaller helpers when ram_flags=0.  Small helpers
>>>>>>>>> sometimes help readability.
>>>>>>>>>
>>>>>>>>> Some of them are going backwards againist what the cocci 
>>>>>>>>> scripts were
>>>>>>>>> suggesting previously, like:
>>>>>>>>>
>>>>>>>>> - 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);
>>>>>>>>>
>>>>>>>>> So you suggest open-code instead.  Why?
>>>>>>>>
>>>>>>>> This is temporary until the usage in vga is gone with
>>>>>>>> hw_compat_2_12 then
>>>>>>>> the Sun machines will be the last users so they can also be 
>>>>>>>> converted to
>>>>>>>> init_{ram,rom} (unless Mark changes his mind and agrees
>>>>>>>> changing these now
>>>>>>>> as was in the first version).
>>>>>>>>
>>>>>>>>> Is it required for your follow up cleanups?
>>>>>>>>
>>>>>>>> It makes those simpler as then we don't need to add 
>>>>>>>> memory_region_new
>>>>>>>> variants for the nomigrate versions that we would remove later 
>>>>>>>> anyway.
>>>>>>>
>>>>>>> Personally I don't see the memory_region_new() whole thing solid 
>>>>>>> yet, but
>>>>>>> still during review.  I shared my thoughts.  So far, I'm not fully
>>>>>>> convinced, but I also don't have the full picture to say no, 
>>>>>>> either.  We
>>>>>>> can wait for others to chime in with more thoughts.
>>>>>>>
>>>>>>> For this series, I would prefer if we can discuss it separately 
>>>>>>> from the
>>>>>>> other work.
>>>>>>>
>>>>>>> Removing _nomigrate() variances makes sense to me in general, it 
>>>>>>> means we
>>>>>>> want to by default suggest new devices only use RAM regions that is
>>>>>>> migratable, with proper owner device specified along with
>>>>>>> the ramblock name
>>>>>>> (as part of migration API).
>>>>>>>
>>>>>>> However, we have two sets of these _nomigrate APIs:
>>>>>>>
>>>>>>>    (1) memory_region_init_r[ao]m_nomigrate
>>>>>>>    (2) memory_region_init_r[ao]m_flags_nomigrate
>>>>>>>
>>>>>>> What I am still confused about is why this series used (2) to 
>>>>>>> replace (1)
>>>>>>> rather than removing both.  I don't understand how this helps if 
>>>>>>> we still
>>>>>>> have (2) around: it means new devices at least can still use
>>>>>>> (2) to bypass
>>>>>>> migration.
>>>>>>>
>>>>>>> There's actually one way to enforce migratable RAMs, taking
>>>>>>> ROM as example,
>>>>>>> it could be:
>>>>>>>
>>>>>>>    memory_region_init_rom_internal(..., global_vmstate=XXX)
>>>>>>>
>>>>>>> Then memory_region_init_rom() should use that, setting
>>>>>>> global_vmstate=off.
>>>>>>>
>>>>>>> Then we can unexport/remove 
>>>>>>> memory_region_init_rom_[flag_]nomigrate() but
>>>>>>> use that _internal() in relevant paths; we should also mark
>>>>>>> the _internal()
>>>>>>> functions to say "new code is suggested to use the normal
>>>>>>> version".  But I
>>>>>>> do not know if that is a slight overkill just for this..
>>>>>>>
>>>>>>> If the other series (to avoid introducing new APIs) is the
>>>>>>> only reason for
>>>>>>> this part, then IMHO this part of the series needs to be in the 
>>>>>>> other
>>>>>>> series, not this one.  The last cleanup patches look more self 
>>>>>>> contained
>>>>>>> OTOH to me, if the whole purpose of the latter half was code
>>>>>>> dedup, then it
>>>>>>> makes more sense at least to me.
>>>>>>
>>>>>> +1 for what Peter Xu suggested. Most part of this series looks
>>>>>> independently useful, but I don't think the removal of
>>>>>> memory_region_init_ram_nomigrate() is.
>>>>>
>>>>> But if there are no useful uses why keep it? This version shows that
>>>>> converting the Sun machines as Peter Maydell suggested only leaves
>>>>> very few uses that can be removed eventually too after some further
>>>>> clean up that I don't want to take up now. Do you have any usage in
>>>>> mind that needs these nomigrate variants?
>>>>
>>>> Well, my threshold for "useful" is > 1 uses, but maintainers may have
>>>> other preferences.
>>
>> Personally I share the same preference..
>>
>> When >1 it means we can dedup some code with a function and I normally
>> think it worthwhile.  That's also why I don't understand why we need to
>> open code _set_readonly() in some of this series.
> 
> Have you read the latest v5 at all? No open coding of set_readonly or 
> useful uses of memory_region_init_r[ao]m_nomigrate left if we accept 
> converting the Sun machines now to memory_region_init_r[ao]m which they 
> should really use as everything else does that and these are the last 
> remaining uses of these nomigrate variants apart from the few special 
> cases. It seems these Sun machines were left behind when everything else 
> converted away from global vmstate and use the normal 
> memory_region_init_r[ao]m where the _nomigrate were left for backward 
> compatibility during API conversion. So it's time to finish that 
> conversion at last.
> 
>> Even if for the sake of a new set of APIs, it doesn't need to provide 
>> every
>> single API that we have had in the old set if we don't need it, IMHO.
>>
>>>
>>> After this series when converting the Sun machines as in v5 the only
>>> nomigrate variant left is memory_region_init_ram_flags_nomigrate that is
>>> equivalent to memory_region_init_ram_nomigrate with flags=0 which 
>>> appears
>>> outside of memory.c only in hw/xtensa/xtfpga.c (that will be 
>>> converted in
>>> the next series to memory_region_new) and in hw/display/vga.c that 
>>> can be
>>> removed when the hw_compat_2_12 is removed. There are two more uses of
>>> memory_region_init_ram_flags_nomigrate in backends/hostmem-ram.c and
>>> hw/m68k/next-cube.c but those have non-0 flags parameter. Therefore I 
>>> think
>>> there's no need to keep other nomigrate variants than
>>> memory_region_init_ram_flags_nomigrate as the only two uses outside of
>>> memory.c that would need memory_region_init_ram_nomigrate are to be 
>>> removed
>>> soon and can use flags=0 until then.
> 
> I tried to explain here that the two uses left that could use 
> memory_region_init_ram_nomigrate will be gone shortly so IMO not worth 
> keeping this function around for that.

Whether it will be gone shortly or not requires reviewing additional 
change to remove the two use cases.

> 
>>> I don't know if the other two uses with non-0 flags can be replaced and
>>> memory_region_init_ram_flags_nomigrate made internal to memory.c but 
>>> that's
>>> beyond the scope of this series and could be considered separately. 
>>> Creating
>>> nomigrate memory region is not something that should be done or need 
>>> several
>>> functions and the very few cases where this might be needed can use
>>> memory_region_init_ram_flags_nomigrate.
>>
>> The hostmem-ram.c one is slightly special, when created as an object it
>> still doesn't know its parent device.
> 
> Yes so there might be some special cases but for those it's enough to 
> have a single nomigrate variant which this series proposes.
> 
>> I still think it might be helpful if you can split your "cleanup" patches
>> out of this series.  Note that softfreeze will be next week.
> 
> This series _is_ the clean up part split from the add memory_region_new 
> functions series so these are all clean ups and the rest is in the other 
> series. I don't theres much more to split. Maybe you missed some patch 
> emails. Here's the clean up series:
> 
> https://patchew.org/QEMU/cover.1772397447.git.balaton@eik.bme.hu/
> 
> and here's the rest of my original series based on these clean ups that 
> continue with adding and using the memory_region_new functions added in 
> that series:
> 
> https://patchew.org/QEMU/cover.1770753117.git.balaton@eik.bme.hu/
> 
> I'm not sure what do you want to split.

In that case, I suggest moving this particular patch into the 
memory_region_new() series (or whichever series reduces the use cases to 
one or zero). That way, each series remains a self-contained logical 
unit for review.

Regards,
Akihiko Odaki

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 12394 bytes --]

On Tue, 3 Mar 2026, Akihiko Odaki wrote:
> On 2026/03/03 6:06, BALATON Zoltan wrote:
>> On Mon, 2 Mar 2026, Peter Xu wrote:
>>> On Mon, Mar 02, 2026 at 04:20:10PM +0100, BALATON Zoltan wrote:
>>>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>>>> On 2026/03/02 21:26, BALATON Zoltan wrote:
>>>>>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>>>>>> On 2026/02/26 1:30, Peter Xu wrote:
>>>>>>>> On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
>>>>>>>>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>>>>>>>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>>>>>>>>> v4:
>>>>>>>>>>> - separate patch converting Sun machines from
>>>>>>>>>>> memory_region_init_ram_nomigrate
>>>>>>>>>>> - split helper to init ram into two functions:
>>>>>>>>>>> setup and error_propagate
>>>>>>>>>>> - also use memory_region_init_io in 
>>>>>>>>>>> memory_region_init_ram_device_ptr
>>>>>>>>>>> 
>>>>>>>>>>> v3:
>>>>>>>>>>> - rebased on master after some patches were merged
>>>>>>>>>>> - drop some more line from memory-region-housekeeping.cocci
>>>>>>>>>>> - added comment to explain what factored out helper does
>>>>>>>>>>> - some more clean ups included
>>>>>>>>>>> 
>>>>>>>>>>> BALATON Zoltan (8):
>>>>>>>>>>>    hw/display/{cg3,tcx}: Do not use 
>>>>>>>>>>> memory_region_init_rom_nomigrate()
>>>>>>>>>>>    memory: Remove memory_region_init_rom_nomigrate()
>>>>>>>>>>>    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>>>>>>>>    memory: Remove memory_region_init_ram_nomigrate()
>>>>>>>>>> 
>>>>>>>>>> Could you help explain why we need to remove the
>>>>>>>>>> above two small helpers?
>>>>>>>>> 
>>>>>>>>> Ideally we should remove all of the nomigrate variants as there are 
>>>>>>>>> only
>>>>>>>>> about 3 uses left, everything else is already moved to
>>>>>>>>> the init_{ram,rom}
>>>>>>>>> variants that register the memory region for migration.
>>>>>>>>> The is probably one
>>>>>>>>> legitimate use that is served by memory_region_init_flags_nomigrate 
>>>>>>>>> and
>>>>>>>>> other 3 uses are the xtfpga which I think is just old code and does 
>>>>>>>>> not
>>>>>>>>> support migration so it will be converted in the second series to
>>>>>>>>> memory_region_new as the maintainers did not say anyting to that so 
>>>>>>>>> far.
>>>>>>>>> Then there's one in hw/display/vga but that's gated
>>>>>>>>> behind a compatibility
>>>>>>>>> switch that's only set in 2.12 (global-vmstate=true) and
>>>>>>>>> earlier. This is
>>>>>>>>> not active any more but cannot yet be removed as there
>>>>>>>>> are eariler compat
>>>>>>>>> props still need cleaning up but the last visible
>>>>>>>>> machine version is 5.1 now
>>>>>>>>> so this is long deprecated and not used. And there are
>>>>>>>>> these Sun machines
>>>>>>>>> that I've converted in the first version but Mark said
>>>>>>>>> he'd like to keep the
>>>>>>>>> current behaviour until it's the last user left. See:
>>>>>>>>> https://
>>>>>>>>> patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/ 
>>>>>>>>> b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>>>>>>>> 
>>>>>>>>>> Not saying that we can't remove them, but I don't yet see the point 
>>>>>>>>>> of
>>>>>>>>>> open-code those readonly=true setup either, or any
>>>>>>>>>> problem with having the
>>>>>>>>>> smaller helpers when ram_flags=0.  Small helpers
>>>>>>>>>> sometimes help readability.
>>>>>>>>>> 
>>>>>>>>>> Some of them are going backwards againist what the cocci scripts 
>>>>>>>>>> were
>>>>>>>>>> suggesting previously, like:
>>>>>>>>>> 
>>>>>>>>>> - 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);
>>>>>>>>>> 
>>>>>>>>>> So you suggest open-code instead.  Why?
>>>>>>>>> 
>>>>>>>>> This is temporary until the usage in vga is gone with
>>>>>>>>> hw_compat_2_12 then
>>>>>>>>> the Sun machines will be the last users so they can also be 
>>>>>>>>> converted to
>>>>>>>>> init_{ram,rom} (unless Mark changes his mind and agrees
>>>>>>>>> changing these now
>>>>>>>>> as was in the first version).
>>>>>>>>> 
>>>>>>>>>> Is it required for your follow up cleanups?
>>>>>>>>> 
>>>>>>>>> It makes those simpler as then we don't need to add 
>>>>>>>>> memory_region_new
>>>>>>>>> variants for the nomigrate versions that we would remove later 
>>>>>>>>> anyway.
>>>>>>>> 
>>>>>>>> Personally I don't see the memory_region_new() whole thing solid yet, 
>>>>>>>> but
>>>>>>>> still during review.  I shared my thoughts.  So far, I'm not fully
>>>>>>>> convinced, but I also don't have the full picture to say no, either.  
>>>>>>>> We
>>>>>>>> can wait for others to chime in with more thoughts.
>>>>>>>> 
>>>>>>>> For this series, I would prefer if we can discuss it separately from 
>>>>>>>> the
>>>>>>>> other work.
>>>>>>>> 
>>>>>>>> Removing _nomigrate() variances makes sense to me in general, it 
>>>>>>>> means we
>>>>>>>> want to by default suggest new devices only use RAM regions that is
>>>>>>>> migratable, with proper owner device specified along with
>>>>>>>> the ramblock name
>>>>>>>> (as part of migration API).
>>>>>>>> 
>>>>>>>> However, we have two sets of these _nomigrate APIs:
>>>>>>>> 
>>>>>>>>    (1) memory_region_init_r[ao]m_nomigrate
>>>>>>>>    (2) memory_region_init_r[ao]m_flags_nomigrate
>>>>>>>> 
>>>>>>>> What I am still confused about is why this series used (2) to replace 
>>>>>>>> (1)
>>>>>>>> rather than removing both.  I don't understand how this helps if we 
>>>>>>>> still
>>>>>>>> have (2) around: it means new devices at least can still use
>>>>>>>> (2) to bypass
>>>>>>>> migration.
>>>>>>>> 
>>>>>>>> There's actually one way to enforce migratable RAMs, taking
>>>>>>>> ROM as example,
>>>>>>>> it could be:
>>>>>>>> 
>>>>>>>>    memory_region_init_rom_internal(..., global_vmstate=XXX)
>>>>>>>> 
>>>>>>>> Then memory_region_init_rom() should use that, setting
>>>>>>>> global_vmstate=off.
>>>>>>>> 
>>>>>>>> Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() 
>>>>>>>> but
>>>>>>>> use that _internal() in relevant paths; we should also mark
>>>>>>>> the _internal()
>>>>>>>> functions to say "new code is suggested to use the normal
>>>>>>>> version".  But I
>>>>>>>> do not know if that is a slight overkill just for this..
>>>>>>>> 
>>>>>>>> If the other series (to avoid introducing new APIs) is the
>>>>>>>> only reason for
>>>>>>>> this part, then IMHO this part of the series needs to be in the other
>>>>>>>> series, not this one.  The last cleanup patches look more self 
>>>>>>>> contained
>>>>>>>> OTOH to me, if the whole purpose of the latter half was code
>>>>>>>> dedup, then it
>>>>>>>> makes more sense at least to me.
>>>>>>> 
>>>>>>> +1 for what Peter Xu suggested. Most part of this series looks
>>>>>>> independently useful, but I don't think the removal of
>>>>>>> memory_region_init_ram_nomigrate() is.
>>>>>> 
>>>>>> But if there are no useful uses why keep it? This version shows that
>>>>>> converting the Sun machines as Peter Maydell suggested only leaves
>>>>>> very few uses that can be removed eventually too after some further
>>>>>> clean up that I don't want to take up now. Do you have any usage in
>>>>>> mind that needs these nomigrate variants?
>>>>> 
>>>>> Well, my threshold for "useful" is > 1 uses, but maintainers may have
>>>>> other preferences.
>>> 
>>> Personally I share the same preference..
>>> 
>>> When >1 it means we can dedup some code with a function and I normally
>>> think it worthwhile.  That's also why I don't understand why we need to
>>> open code _set_readonly() in some of this series.
>> 
>> Have you read the latest v5 at all? No open coding of set_readonly or 
>> useful uses of memory_region_init_r[ao]m_nomigrate left if we accept 
>> converting the Sun machines now to memory_region_init_r[ao]m which they 
>> should really use as everything else does that and these are the last 
>> remaining uses of these nomigrate variants apart from the few special 
>> cases. It seems these Sun machines were left behind when everything else 
>> converted away from global vmstate and use the normal 
>> memory_region_init_r[ao]m where the _nomigrate were left for backward 
>> compatibility during API conversion. So it's time to finish that conversion 
>> at last.
>> 
>>> Even if for the sake of a new set of APIs, it doesn't need to provide 
>>> every
>>> single API that we have had in the old set if we don't need it, IMHO.
>>> 
>>>> 
>>>> After this series when converting the Sun machines as in v5 the only
>>>> nomigrate variant left is memory_region_init_ram_flags_nomigrate that is
>>>> equivalent to memory_region_init_ram_nomigrate with flags=0 which appears
>>>> outside of memory.c only in hw/xtensa/xtfpga.c (that will be converted in
>>>> the next series to memory_region_new) and in hw/display/vga.c that can be
>>>> removed when the hw_compat_2_12 is removed. There are two more uses of
>>>> memory_region_init_ram_flags_nomigrate in backends/hostmem-ram.c and
>>>> hw/m68k/next-cube.c but those have non-0 flags parameter. Therefore I 
>>>> think
>>>> there's no need to keep other nomigrate variants than
>>>> memory_region_init_ram_flags_nomigrate as the only two uses outside of
>>>> memory.c that would need memory_region_init_ram_nomigrate are to be 
>>>> removed
>>>> soon and can use flags=0 until then.
>> 
>> I tried to explain here that the two uses left that could use 
>> memory_region_init_ram_nomigrate will be gone shortly so IMO not worth 
>> keeping this function around for that.
>
> Whether it will be gone shortly or not requires reviewing additional change 
> to remove the two use cases.

That's why I gave pointers to the places so if somebody is interested can 
look. How soon it is depends on how fast we get to the end of this review 
(that can resolve xtfpga) and how fast older compatibility settings are 
removed before we get to 2.12. The latter can take a while because they 
also need cleaning up the (mostly virtio) devices they refer to so I'm not 
trying to do anything about that but it should eventually be removed as 
machines older than 5.1 are not visible anyway so these are just dead 
code.

>>>> I don't know if the other two uses with non-0 flags can be replaced and
>>>> memory_region_init_ram_flags_nomigrate made internal to memory.c but 
>>>> that's
>>>> beyond the scope of this series and could be considered separately. 
>>>> Creating
>>>> nomigrate memory region is not something that should be done or need 
>>>> several
>>>> functions and the very few cases where this might be needed can use
>>>> memory_region_init_ram_flags_nomigrate.
>>> 
>>> The hostmem-ram.c one is slightly special, when created as an object it
>>> still doesn't know its parent device.
>> 
>> Yes so there might be some special cases but for those it's enough to have 
>> a single nomigrate variant which this series proposes.
>> 
>>> I still think it might be helpful if you can split your "cleanup" patches
>>> out of this series.  Note that softfreeze will be next week.
>> 
>> This series _is_ the clean up part split from the add memory_region_new 
>> functions series so these are all clean ups and the rest is in the other 
>> series. I don't theres much more to split. Maybe you missed some patch 
>> emails. Here's the clean up series:
>> 
>> https://patchew.org/QEMU/cover.1772397447.git.balaton@eik.bme.hu/
>> 
>> and here's the rest of my original series based on these clean ups that 
>> continue with adding and using the memory_region_new functions added in 
>> that series:
>> 
>> https://patchew.org/QEMU/cover.1770753117.git.balaton@eik.bme.hu/
>> 
>> I'm not sure what do you want to split.
>
> In that case, I suggest moving this particular patch into the 
> memory_region_new() series (or whichever series reduces the use cases to one 
> or zero). That way, each series remains a self-contained logical unit for 
> review.

This is not one patch but half of the series that later patches depend on 
so this would mean just merge the two series again and get nowhere. Looks 
like we could at least remove memory_region_init_rom_nomigrate now and 
keep memory_region_init_ram for just the two uses in xtfpga or if we 
convert that then only for vga but then why keep this function for only 
the use in vga?

Regards,
BALATON Zoltan

Re: [PATCH v4 0/8] memory: Remove most _nomigrate variants

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 13090 bytes --]

On Tue, 3 Mar 2026, BALATON Zoltan wrote:
> On Tue, 3 Mar 2026, Akihiko Odaki wrote:
>> On 2026/03/03 6:06, BALATON Zoltan wrote:
>>> On Mon, 2 Mar 2026, Peter Xu wrote:
>>>> On Mon, Mar 02, 2026 at 04:20:10PM +0100, BALATON Zoltan wrote:
>>>>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>>>>> On 2026/03/02 21:26, BALATON Zoltan wrote:
>>>>>>> On Mon, 2 Mar 2026, Akihiko Odaki wrote:
>>>>>>>> On 2026/02/26 1:30, Peter Xu wrote:
>>>>>>>>> On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
>>>>>>>>>> On Tue, 24 Feb 2026, Peter Xu wrote:
>>>>>>>>>>> On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
>>>>>>>>>>>> v4:
>>>>>>>>>>>> - separate patch converting Sun machines from
>>>>>>>>>>>> memory_region_init_ram_nomigrate
>>>>>>>>>>>> - split helper to init ram into two functions:
>>>>>>>>>>>> setup and error_propagate
>>>>>>>>>>>> - also use memory_region_init_io in 
>>>>>>>>>>>> memory_region_init_ram_device_ptr
>>>>>>>>>>>> 
>>>>>>>>>>>> v3:
>>>>>>>>>>>> - rebased on master after some patches were merged
>>>>>>>>>>>> - drop some more line from memory-region-housekeeping.cocci
>>>>>>>>>>>> - added comment to explain what factored out helper does
>>>>>>>>>>>> - some more clean ups included
>>>>>>>>>>>> 
>>>>>>>>>>>> BALATON Zoltan (8):
>>>>>>>>>>>>    hw/display/{cg3,tcx}: Do not use 
>>>>>>>>>>>> memory_region_init_rom_nomigrate()
>>>>>>>>>>>>    memory: Remove memory_region_init_rom_nomigrate()
>>>>>>>>>>>>    sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
>>>>>>>>>>>>    memory: Remove memory_region_init_ram_nomigrate()
>>>>>>>>>>> 
>>>>>>>>>>> Could you help explain why we need to remove the
>>>>>>>>>>> above two small helpers?
>>>>>>>>>> 
>>>>>>>>>> Ideally we should remove all of the nomigrate variants as there are 
>>>>>>>>>> only
>>>>>>>>>> about 3 uses left, everything else is already moved to
>>>>>>>>>> the init_{ram,rom}
>>>>>>>>>> variants that register the memory region for migration.
>>>>>>>>>> The is probably one
>>>>>>>>>> legitimate use that is served by memory_region_init_flags_nomigrate 
>>>>>>>>>> and
>>>>>>>>>> other 3 uses are the xtfpga which I think is just old code and does 
>>>>>>>>>> not
>>>>>>>>>> support migration so it will be converted in the second series to
>>>>>>>>>> memory_region_new as the maintainers did not say anyting to that so 
>>>>>>>>>> far.
>>>>>>>>>> Then there's one in hw/display/vga but that's gated
>>>>>>>>>> behind a compatibility
>>>>>>>>>> switch that's only set in 2.12 (global-vmstate=true) and
>>>>>>>>>> earlier. This is
>>>>>>>>>> not active any more but cannot yet be removed as there
>>>>>>>>>> are eariler compat
>>>>>>>>>> props still need cleaning up but the last visible
>>>>>>>>>> machine version is 5.1 now
>>>>>>>>>> so this is long deprecated and not used. And there are
>>>>>>>>>> these Sun machines
>>>>>>>>>> that I've converted in the first version but Mark said
>>>>>>>>>> he'd like to keep the
>>>>>>>>>> current behaviour until it's the last user left. See:
>>>>>>>>>> https://
>>>>>>>>>> patchew.org/QEMU/cover.1769703287.git.balaton@eik.bme.hu/ 
>>>>>>>>>> b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.balaton@eik.bme.hu/
>>>>>>>>>> 
>>>>>>>>>>> Not saying that we can't remove them, but I don't yet see the 
>>>>>>>>>>> point of
>>>>>>>>>>> open-code those readonly=true setup either, or any
>>>>>>>>>>> problem with having the
>>>>>>>>>>> smaller helpers when ram_flags=0.  Small helpers
>>>>>>>>>>> sometimes help readability.
>>>>>>>>>>> 
>>>>>>>>>>> Some of them are going backwards againist what the cocci scripts 
>>>>>>>>>>> were
>>>>>>>>>>> suggesting previously, like:
>>>>>>>>>>> 
>>>>>>>>>>> - 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);
>>>>>>>>>>> 
>>>>>>>>>>> So you suggest open-code instead.  Why?
>>>>>>>>>> 
>>>>>>>>>> This is temporary until the usage in vga is gone with
>>>>>>>>>> hw_compat_2_12 then
>>>>>>>>>> the Sun machines will be the last users so they can also be 
>>>>>>>>>> converted to
>>>>>>>>>> init_{ram,rom} (unless Mark changes his mind and agrees
>>>>>>>>>> changing these now
>>>>>>>>>> as was in the first version).
>>>>>>>>>> 
>>>>>>>>>>> Is it required for your follow up cleanups?
>>>>>>>>>> 
>>>>>>>>>> It makes those simpler as then we don't need to add 
>>>>>>>>>> memory_region_new
>>>>>>>>>> variants for the nomigrate versions that we would remove later 
>>>>>>>>>> anyway.
>>>>>>>>> 
>>>>>>>>> Personally I don't see the memory_region_new() whole thing solid 
>>>>>>>>> yet, but
>>>>>>>>> still during review.  I shared my thoughts.  So far, I'm not fully
>>>>>>>>> convinced, but I also don't have the full picture to say no, 
>>>>>>>>> either.  We
>>>>>>>>> can wait for others to chime in with more thoughts.
>>>>>>>>> 
>>>>>>>>> For this series, I would prefer if we can discuss it separately from 
>>>>>>>>> the
>>>>>>>>> other work.
>>>>>>>>> 
>>>>>>>>> Removing _nomigrate() variances makes sense to me in general, it 
>>>>>>>>> means we
>>>>>>>>> want to by default suggest new devices only use RAM regions that is
>>>>>>>>> migratable, with proper owner device specified along with
>>>>>>>>> the ramblock name
>>>>>>>>> (as part of migration API).
>>>>>>>>> 
>>>>>>>>> However, we have two sets of these _nomigrate APIs:
>>>>>>>>> 
>>>>>>>>>    (1) memory_region_init_r[ao]m_nomigrate
>>>>>>>>>    (2) memory_region_init_r[ao]m_flags_nomigrate
>>>>>>>>> 
>>>>>>>>> What I am still confused about is why this series used (2) to 
>>>>>>>>> replace (1)
>>>>>>>>> rather than removing both.  I don't understand how this helps if we 
>>>>>>>>> still
>>>>>>>>> have (2) around: it means new devices at least can still use
>>>>>>>>> (2) to bypass
>>>>>>>>> migration.
>>>>>>>>> 
>>>>>>>>> There's actually one way to enforce migratable RAMs, taking
>>>>>>>>> ROM as example,
>>>>>>>>> it could be:
>>>>>>>>> 
>>>>>>>>>    memory_region_init_rom_internal(..., global_vmstate=XXX)
>>>>>>>>> 
>>>>>>>>> Then memory_region_init_rom() should use that, setting
>>>>>>>>> global_vmstate=off.
>>>>>>>>> 
>>>>>>>>> Then we can unexport/remove 
>>>>>>>>> memory_region_init_rom_[flag_]nomigrate() but
>>>>>>>>> use that _internal() in relevant paths; we should also mark
>>>>>>>>> the _internal()
>>>>>>>>> functions to say "new code is suggested to use the normal
>>>>>>>>> version".  But I
>>>>>>>>> do not know if that is a slight overkill just for this..
>>>>>>>>> 
>>>>>>>>> If the other series (to avoid introducing new APIs) is the
>>>>>>>>> only reason for
>>>>>>>>> this part, then IMHO this part of the series needs to be in the 
>>>>>>>>> other
>>>>>>>>> series, not this one.  The last cleanup patches look more self 
>>>>>>>>> contained
>>>>>>>>> OTOH to me, if the whole purpose of the latter half was code
>>>>>>>>> dedup, then it
>>>>>>>>> makes more sense at least to me.
>>>>>>>> 
>>>>>>>> +1 for what Peter Xu suggested. Most part of this series looks
>>>>>>>> independently useful, but I don't think the removal of
>>>>>>>> memory_region_init_ram_nomigrate() is.
>>>>>>> 
>>>>>>> But if there are no useful uses why keep it? This version shows that
>>>>>>> converting the Sun machines as Peter Maydell suggested only leaves
>>>>>>> very few uses that can be removed eventually too after some further
>>>>>>> clean up that I don't want to take up now. Do you have any usage in
>>>>>>> mind that needs these nomigrate variants?
>>>>>> 
>>>>>> Well, my threshold for "useful" is > 1 uses, but maintainers may have
>>>>>> other preferences.
>>>> 
>>>> Personally I share the same preference..
>>>> 
>>>> When >1 it means we can dedup some code with a function and I normally
>>>> think it worthwhile.  That's also why I don't understand why we need to
>>>> open code _set_readonly() in some of this series.
>>> 
>>> Have you read the latest v5 at all? No open coding of set_readonly or 
>>> useful uses of memory_region_init_r[ao]m_nomigrate left if we accept 
>>> converting the Sun machines now to memory_region_init_r[ao]m which they 
>>> should really use as everything else does that and these are the last 
>>> remaining uses of these nomigrate variants apart from the few special 
>>> cases. It seems these Sun machines were left behind when everything else 
>>> converted away from global vmstate and use the normal 
>>> memory_region_init_r[ao]m where the _nomigrate were left for backward 
>>> compatibility during API conversion. So it's time to finish that 
>>> conversion at last.
>>> 
>>>> Even if for the sake of a new set of APIs, it doesn't need to provide 
>>>> every
>>>> single API that we have had in the old set if we don't need it, IMHO.
>>>> 
>>>>> 
>>>>> After this series when converting the Sun machines as in v5 the only
>>>>> nomigrate variant left is memory_region_init_ram_flags_nomigrate that is
>>>>> equivalent to memory_region_init_ram_nomigrate with flags=0 which 
>>>>> appears
>>>>> outside of memory.c only in hw/xtensa/xtfpga.c (that will be converted 
>>>>> in
>>>>> the next series to memory_region_new) and in hw/display/vga.c that can 
>>>>> be
>>>>> removed when the hw_compat_2_12 is removed. There are two more uses of
>>>>> memory_region_init_ram_flags_nomigrate in backends/hostmem-ram.c and
>>>>> hw/m68k/next-cube.c but those have non-0 flags parameter. Therefore I 
>>>>> think
>>>>> there's no need to keep other nomigrate variants than
>>>>> memory_region_init_ram_flags_nomigrate as the only two uses outside of
>>>>> memory.c that would need memory_region_init_ram_nomigrate are to be 
>>>>> removed
>>>>> soon and can use flags=0 until then.
>>> 
>>> I tried to explain here that the two uses left that could use 
>>> memory_region_init_ram_nomigrate will be gone shortly so IMO not worth 
>>> keeping this function around for that.
>> 
>> Whether it will be gone shortly or not requires reviewing additional change 
>> to remove the two use cases.
>
> That's why I gave pointers to the places so if somebody is interested can 
> look. How soon it is depends on how fast we get to the end of this review 
> (that can resolve xtfpga) and how fast older compatibility settings are 
> removed before we get to 2.12. The latter can take a while because they also 
> need cleaning up the (mostly virtio) devices they refer to so I'm not trying 
> to do anything about that but it should eventually be removed as machines 
> older than 5.1 are not visible anyway so these are just dead code.
>
>>>>> I don't know if the other two uses with non-0 flags can be replaced and
>>>>> memory_region_init_ram_flags_nomigrate made internal to memory.c but 
>>>>> that's
>>>>> beyond the scope of this series and could be considered separately. 
>>>>> Creating
>>>>> nomigrate memory region is not something that should be done or need 
>>>>> several
>>>>> functions and the very few cases where this might be needed can use
>>>>> memory_region_init_ram_flags_nomigrate.
>>>> 
>>>> The hostmem-ram.c one is slightly special, when created as an object it
>>>> still doesn't know its parent device.
>>> 
>>> Yes so there might be some special cases but for those it's enough to have 
>>> a single nomigrate variant which this series proposes.
>>> 
>>>> I still think it might be helpful if you can split your "cleanup" patches
>>>> out of this series.  Note that softfreeze will be next week.
>>> 
>>> This series _is_ the clean up part split from the add memory_region_new 
>>> functions series so these are all clean ups and the rest is in the other 
>>> series. I don't theres much more to split. Maybe you missed some patch 
>>> emails. Here's the clean up series:
>>> 
>>> https://patchew.org/QEMU/cover.1772397447.git.balaton@eik.bme.hu/
>>> 
>>> and here's the rest of my original series based on these clean ups that 
>>> continue with adding and using the memory_region_new functions added in 
>>> that series:
>>> 
>>> https://patchew.org/QEMU/cover.1770753117.git.balaton@eik.bme.hu/
>>> 
>>> I'm not sure what do you want to split.
>> 
>> In that case, I suggest moving this particular patch into the 
>> memory_region_new() series (or whichever series reduces the use cases to 
>> one or zero). That way, each series remains a self-contained logical unit 
>> for review.
>
> This is not one patch but half of the series that later patches depend on so 
> this would mean just merge the two series again and get nowhere. Looks like 
> we could at least remove memory_region_init_rom_nomigrate now and keep 
> memory_region_init_ram for just the two uses in xtfpga or if we convert that 
> then only for vga but then why keep this function for only the use in vga?

I mean keep memory_region_init_ram_nomigrate for xtfpga and vga but even 
xtfpga can be converted to memory_region_init_ram then the only use for 
memory_region_init_ram_nomigrate would be vga which could easily use 
memory_region_init_ram_flags_nomigrate that still has two other uses. 
Sorry for not being able to clearly express this above.

Regards,
BALATON Zoltan