+Markus
On 4/9/21 8:13 PM, Peter Maydell wrote:
> I wanted to convert the hw/misc/mps2-scc.c device from old-style
> to 3-phase reset as a prerequisite for another change to the device,
> but I ran into a problem because currently it has some TYPE_DEVICE
> QOM child objects, the LEDs. Because TYPE_DEVICE don't live in the
> qbus hierarchy, the device resets them manually in its DeviceClass::reset
> method:
>
> for (i = 0; i < ARRAY_SIZE(s->led); i++) {
> device_cold_reset(DEVICE(s->led[i]));
> }
>
> This makes converting to 3-phase reset awkward. The obvious "natural"
> approach would be to say "in this device's phase X, invoke phase X
> for these objects", but we have no API for that. (The functions which
> would do it, resettable_phase_enter() etc, are static inside resettable.c.)
>
> Ignoring the phasing and trying to just call device_cold_reset() in
> the 'enter' phase results in an assertion failure, because we trip
> the assert(!enter_phase_in_progress) in resettable_assert_reset(),
> which doesn't expect us to be triggering a reset inside a reset.
>
> Ideally one would want to be able to add the LEDs to the list of
> things which are children of this object for purposes of reset
> (so they are iterated as part of the resettable_child_foreach()
> logic and their phases are automatically invoked at the right point).
> But for a subclass of DeviceState that's device_reset_child_foreach()
> and it only iterates any qbus children of this device.
>
> Any clever ideas?
Not very clever... We could kludge it by calling device_legacy_reset()
instead of device_cold_reset() in mps2_fpgaio_reset()... But that
mean we are going backward with the API.
OK, back to read your previous explanations... and the threads around.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg723312.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg738242.html
"Note that [qdev/qbus hierarchy] is an entirely separate thing
from the QOM hierarchy of parent-and-child object relationships."
Hmm OK. I guess I'm confused seeing parts are overlapping when they
aren't. So setting the QOM parent relationship helps in having a
correct QOM path and the object is displayed nicely in the qom-tree,
but doesn't bring anything on the qdev side.
So back to qdev.
- TYPE_DEVICE (aka 'qdev') is abstract.
It inherits TYPE_OBJECT.
It can provide a bus (aka qbus) to plug things.
It implements TYPE_RESETTABLE_INTERFACE.
- TYPE_SYS_BUS_DEVICE is also abstract.
It inherits from TYPE_DEVICE, setting qbus=TYPE_SYSTEM_BUS
(
To confuse more, there is some undocumented API called
'device_listener' in qdev which instead uses sysbus:
void device_listener_register(DeviceListener *listener);
void device_listener_unregister(DeviceListener *listener);
)
Making MachineState inherit TYPE_DEVICE and re-implement the
TYPE_RESETTABLE_INTERFACE doesn't seem going in the right
direction...
If TYPE_MACHINE were qdev, its qbus could be a ResetBus. Again
it feels wrong (over engineering).
> Maybe some mechanism for marking "these things which are my
> QOM children I want to be reset when I am reset (so make them> reset children
> of me and don't reset them as part of the
> qbus-tree-walking)" would be useful. I do think that in a
> lot of cases we want to be doing something closer to "reset
> along the QOM tree".
Eh here you mention QOM again... Shouldn't it be qdev?
I know the LED is just an example of a broader problem.
I indeed took care to add the QOM parent relation:
(qemu) info qom-tree
/machine (mps2-an385-machine)
/fpgaio (mps2-fpgaio)
/mps2-fpgaio[0] (memory-region)
/userled0 (led)
/unnamed-gpio-in[0] (irq)
/userled1 (led)
/unnamed-gpio-in[0] (irq)
/scc (mps2-scc)
/mps2-scc[0] (memory-region)
/scc-led0 (led)
/unnamed-gpio-in[0] (irq)
/scc-led1 (led)
/unnamed-gpio-in[0] (irq)
...
So looking at this qom-tree, the reset tree seems to me
more natural than the sysbus one, but IIRC not many models
set this QOM relationship.
QOM objects aren't enforced to have a relation with a parent,
as opposed as recent changes from Markus to always have a qdev
on a qbus). But even without parent they end in the /unattached
container below /machine, so if the reset were there, the
machine could still iterate over the /unattached children.
> I really do need to spend some time working
> out what the right thing with reset is and how we might get
> from where we are now to there...
Well, finally this QOM-tree reset is appealing.
Sorry if I haven't been very helpful :S Still processing the
problem in background...
Regards,
Phil.
