[PATCH v10 1/9] hw/display/{cg3.tcx}: Do not use memory_region_init_rom_nomigrate()

BALATON Zoltan posted 9 patches 1 month ago
Maintainers: Pierrick Bouvier <pierrick.bouvier@linaro.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Gerd Hoffmann <kraxel@redhat.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Max Filippov <jcmvbkbc@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
[PATCH v10 1/9] hw/display/{cg3.tcx}: Do not use memory_region_init_rom_nomigrate()
Posted by BALATON Zoltan 1 month ago
Use memory_region_init_rom() instead which is what other devices do.
This breaks migration but these devices are only used by sparc Sun
machines which have no migration compatibility guarantee.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/display/cg3.c | 5 ++---
 hw/display/tcx.c | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index 568d6048a6..61bdb0552e 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -282,8 +282,8 @@ 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_rom(&s->rom, obj, "cg3.prom", FCODE_MAX_ROM_SIZE,
+                           &error_fatal);
     sysbus_init_mmio(sbd, &s->rom);
 
     memory_region_init_io(&s->reg, obj, &cg3_reg_ops, s, "cg3.reg",
@@ -299,7 +299,6 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
     char *fcode_filename;
 
     /* FCode ROM */
-    vmstate_register_ram_global(&s->rom);
     fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
     if (fcode_filename) {
         ret = load_image_mr(fcode_filename, &s->rom);
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 36cad82abd..16114b9bb8 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -756,8 +756,8 @@ 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_rom(&s->rom, obj, "tcx.prom", FCODE_MAX_ROM_SIZE,
+                           &error_fatal);
     sysbus_init_mmio(sbd, &s->rom);
 
     /* 2/STIP : Stippler */
@@ -822,7 +822,6 @@ static void tcx_realizefn(DeviceState *dev, Error **errp)
     vram_base = memory_region_get_ram_ptr(&s->vram_mem);
 
     /* 10/ROM : FCode ROM */
-    vmstate_register_ram_global(&s->rom);
     fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, TCX_ROM_FILE);
     if (fcode_filename) {
         ret = load_image_mr(fcode_filename, &s->rom);
-- 
2.41.3
Re: [PATCH v10 1/9] hw/display/{cg3.tcx}: Do not use memory_region_init_rom_nomigrate()
Posted by Thomas Huth 3 weeks, 5 days ago
On 08/03/2026 00.05, BALATON Zoltan wrote:
> Use memory_region_init_rom() instead which is what other devices do.
> This breaks migration but these devices are only used by sparc Sun
> machines which have no migration compatibility guarantee.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
...
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 36cad82abd..16114b9bb8 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -756,8 +756,8 @@ 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_rom(&s->rom, obj, "tcx.prom", FCODE_MAX_ROM_SIZE,
> +                           &error_fatal);
>       sysbus_init_mmio(sbd, &s->rom);

  Hi!

This unfortunately break the device-introspect-test in thorough mode:

  $ export QTEST_QEMU_BINARY=./qemu-system-sparc
  $ tests/qtest/device-introspect-test -m thorough
  ...
  # Testing device 'sun-tcx'
  RAMBlock "tcx.prom" already registered, abort!
  Broken pipe
  ../../devel/qemu/tests/qtest/libqtest.c:210: kill_qemu() detected QEMU
  death from signal 6 (Aborted) (core dumped)
  Aborted (core dumped)

Could you please have a look? I guess the memory_region_init_rom should 
rather be done from the realize() function instead of doing it in 
instance_init()?

  Thanks,
   Thomas