* [Qemu-devel] [PATCH v2] hw/arm/integratorcp: Don't do things that could be fatal in the instance_init
@ 2018-04-05 5:34 Thomas Huth
2018-04-05 10:52 ` Peter Maydell
0 siblings, 1 reply; 2+ messages in thread
From: Thomas Huth @ 2018-04-05 5:34 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: qemu-arm
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 2794 bytes --]
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(-)
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;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/arm/integratorcp: Don't do things that could be fatal in the instance_init
2018-04-05 5:34 [Qemu-devel] [PATCH v2] hw/arm/integratorcp: Don't do things that could be fatal in the instance_init Thomas Huth
@ 2018-04-05 10:52 ` Peter Maydell
0 siblings, 0 replies; 2+ messages in thread
From: Peter Maydell @ 2018-04-05 10:52 UTC (permalink / raw)
To: Thomas Huth; +Cc: QEMU Developers, qemu-arm
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-04-05 10:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-05 5:34 [Qemu-devel] [PATCH v2] hw/arm/integratorcp: Don't do things that could be fatal in the instance_init Thomas Huth
2018-04-05 10:52 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.