Re: [Qemu-arm] [PATCH] hw/arm/bcm283x: Fix crash with device_add bcm2837 on unsupported machines

[Peter Maydell]

On 9 July 2018 at 22:03, Thomas Huth <address@hidden> wrote:
> When trying to "device_add bcm2837" on a machine that is not suitable for
> this device, you can quickly crash QEMU afterwards, e.g. with "info qtree":
>
> echo "{'execute':'qmp_capabilities'} {'execute':'device_add', " \
>  "'arguments':{'driver':'bcm2837'}} {'execute': 'human-monitor-command', " \
>  "'arguments': {'command-line': 'info qtree'}}" | \
>  aarch64-softmmu/qemu-system-aarch64 -M integratorcp,accel=qtest -S -qmp stdio
>
> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2},
>  "package": "build-all"}, "capabilities": []}}
> {"return": {}}
> {"error": {"class": "GenericError", "desc": "Device 'bcm2837' can not be
>  hotplugged on this machine"}}
> Segmentation fault (core dumped)
>
> The problem is that qdev_set_parent_bus() from instance_init adds a link
> to the child devices which is not valid anymore after the device init
> failed. Thus the qdev_set_parent_bus() must rather be done in the realize
> function instead.

Yuck. The real problem here is that we're still requiring the
code that creates these QOM devices to manually set the parent
in the first place. It's not surprising that we don't get it right
(either parenting in the wrong place or not at all). I'd much
rather see us fix that properly than keep papering over places
where we get it wrong.

Also, "device_add bcm2837" is just nonsensical: there's no
way to create an SoC device like this with device_add.
We should not allow it (for this and all the other
devices that device_add will never work with)...

thanks
-- PMM