| Message ID | 1522906473-11252-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] hw/arm/integratorcp: Don't do things that could be fatal in the instance_init | expand |
On 5 April 2018 at 06:34, Thomas Huth <thuth@redhat.com> wrote: > An instance_init function must not fail - and might be called multiple times, > e.g. during device introspection with the 'device-list-properties' QMP > command. Since the integratorcm device ignores this rule, QEMU currently > aborts in this case (though it really should not): > > echo "{'execute':'qmp_capabilities'}"\ > "{'execute':'device-list-properties',"\ > "'arguments':{'typename':'integrator_core'}}" \ > | arm-softmmu/qemu-system-arm -M integratorcp,accel=qtest -qmp stdio > {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, > "package": "build-all"}, "capabilities": []}} > {"return": {}} > RAMBlock "integrator.flash" already registered, abort! > Aborted (core dumped) > > Move the problematic code to the realize() function instead to fix this > problem. > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Signed-off-by: Thomas Huth <thuth@redhat.com> Applied to target-arm.next, thanks. -- PMM
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c index e8303b8..58b40ef 100644 --- a/hw/arm/integratorcp.c +++ b/hw/arm/integratorcp.c @@ -266,7 +266,6 @@ static const MemoryRegionOps integratorcm_ops = { static void integratorcm_init(Object *obj) { IntegratorCMState *s = INTEGRATOR_CM(obj); - SysBusDevice *dev = SYS_BUS_DEVICE(obj); s->cm_osc = 0x01000048; /* ??? What should the high bits of this value be? */ @@ -276,20 +275,28 @@ static void integratorcm_init(Object *obj) s->cm_init = 0x00000112; s->cm_refcnt_offset = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24, 1000); - memory_region_init_ram(&s->flash, obj, "integrator.flash", 0x100000, - &error_fatal); - memory_region_init_io(&s->iomem, obj, &integratorcm_ops, s, - "integratorcm", 0x00800000); - sysbus_init_mmio(dev, &s->iomem); - - integratorcm_do_remap(s); /* ??? Save/restore. */ } static void integratorcm_realize(DeviceState *d, Error **errp) { IntegratorCMState *s = INTEGRATOR_CM(d); + SysBusDevice *dev = SYS_BUS_DEVICE(d); + Error *local_err = NULL; + + memory_region_init_ram(&s->flash, OBJECT(d), "integrator.flash", 0x100000, + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + memory_region_init_io(&s->iomem, OBJECT(d), &integratorcm_ops, s, + "integratorcm", 0x00800000); + sysbus_init_mmio(dev, &s->iomem); + + integratorcm_do_remap(s); if (s->memsz >= 256) { integrator_spd[31] = 64;
An instance_init function must not fail - and might be called multiple times, e.g. during device introspection with the 'device-list-properties' QMP command. Since the integratorcm device ignores this rule, QEMU currently aborts in this case (though it really should not): echo "{'execute':'qmp_capabilities'}"\ "{'execute':'device-list-properties',"\ "'arguments':{'typename':'integrator_core'}}" \ | arm-softmmu/qemu-system-arm -M integratorcp,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} {"return": {}} RAMBlock "integrator.flash" already registered, abort! Aborted (core dumped) Move the problematic code to the realize() function instead to fix this problem. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Use "OBJECT(d)" instead of "(Object *)d" hw/arm/integratorcp.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)