hw/arm/integratorcp.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
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.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/arm/integratorcp.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index e8303b8..9e2df34 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;
--
1.8.3.1
Hi Thomas, On 04/04/2018 04:15 PM, Thomas Huth 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. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > hw/arm/integratorcp.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c > index e8303b8..9e2df34 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, To keep consistency: memory_region_init_ram(&s->flash, OBJECT(d), "integrator.flash", ... > + &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + > + memory_region_init_io(&s->iomem, (Object *)d, &integratorcm_ops, s, 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; > With the macro instead of the cast: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Regards, Phil.
© 2016 - 2024 Red Hat, Inc.