Re: Resetting non-qdev children in a 3-phase reset device

[Philippe Mathieu-Daudé]

On 4/22/21 3:21 PM, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On Sun, 18 Apr 2021 at 21:16, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>> +Markus
>>>
>>> On 4/9/21 8:13 PM, Peter Maydell wrote:
>>>> 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?
>>
>> No, I meant QOM, ie the relation you see below in the "info qom-tree"
>> output:
>>
>>> 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,
>>
>> I thought they always got parented into somewhere, even if it
>> was a catch-all bucket somewhere ?
> 
> If a *device* has no QOM parent at realize time, realize sets the QOM
> parent to the /machine/unattached/ orphanage.  In device_set_realized():
> 
>     if (value && !dev->realized) {
>         ...
>         if (!obj->parent) {
>             gchar *name = g_strdup_printf("device[%d]", unattached_count++);
> 
>             object_property_add_child(container_get(qdev_get_machine(),
>                                                     "/unattached"),
>                                       name, obj);
>             unattached_parent = true;
>             g_free(name);
>         }
> 
> As far as I understand this is a crutch to help us cope with
> incompletely QOMified devices.
> 
> The crutch does not apply to QOM objects that aren't devices.
> 
>>> as opposed as recent changes from Markus to always have a qdev
>>> on a qbus).
> 
> Most qdevs plug into a qbus, but some don't.
> 
> DeviceClass member @bus_type names the kind of bus the device plugs
> into.  It's a QOM type name.  Example: for a PCI device, it's
> TYPE_PCI_BUS, and the device must be plugged into an instance of a
> (subtype of) TYPE_PCI_BUS.
> 
> If @bus_type is null, the device does not plug into any qbus.
> 
> The qbus a device is plugged into is also called the parent bus.  Not to
> be confused with the QOM parent.
> 
>>>             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.
>>
>> ...yes, /unattached is what I was thinking about.
>>
>> My current half-thought-through view is that where we ought
>> to try to end up is something like:
>>
>>  * "real" buses should continue to propagate reset
>>    (A "real" bus is like PCI, SCSI, and other buses where the real
>>    hardware has a concept of a "bus reset" or where the power to the
>>    plugged device comes from the bus so that powercycling the
>>    controller naturally powercycles the devices. Sysbus is not a
>>    "real" bus; I haven't checked the others to see if we have any
>>    other non-real buses)
>>  * reset should follow the QOM tree for objects not on a "real" bus
>>    (that is, the qdev "reset this device" function should do
>>    "iterate through my QOM children and reset those which are not
>>    on a real bus" as well as its current "reset myself" and "reset
>>    every qbus I have")
>>  * instead of reset starting with the sysbus and working along the
>>    qbus hierarchy, we start by resetting the machine. That should
>>    include resetting all the QOM children of the machine. Any
>>    device which has a qbus should reset the qbus as part of its
>>    reset, but only "real" buses reset their children when reset.
> 
> Sounds like an approximation of reset wire modelling :)
> 
> In a real machine, the reset signal travels along "wires" (in quotes,
> because it need not be a dedicated wire, although it commonly is)
> 
> We're not modelling these wires explicitly so far.  Instead, we make
> assumptions such as "reset flows along the qdev tree", which are close
> enough except when they aren't.
> 
> What you propose is likely closer to reality than what we have now.

Then maybe reality is easier to model =)

> Do I make sense?

I guess so. Now I wonder if Peter's approach is doable while still
having "incompletely QOMified devices".

But if we can propagate reset tree via QOM, it is a good excuse
to finish QOM'ifying devices and enforce the API to prohibit non-QOM
ones.

And remove the crutch in device_set_realized().

>> That means that, for instance, if you reset an SoC container object
>> it will reset all the sub-devices within the SoC and the miscellaneous
>> bits of glue logic like OR gates it might also own[*]. It also means that
>> CPU objects should no longer need weird special casing, because they
>> are part of the QOM hierarchy and get reset that way.
>>
>> [*] Fun fact: TYPE_OR_IRQ inherits directly from TYPE_DEVICE which
>> means that pretty much no instances of it ever get reset.
>>
>> There is of course a massive unsolved problem with this idea, which
>> is the usual "how do we get there from here" one.
>>
>> (Eventually I think we might be able to collapse TYPE_SYS_BUS_DEVICE
>> down into TYPE_DEVICE: there is no particular reason why a TYPE_DEVICE
>> can have GPIO inputs and outputs but only a TYPE_SYS_BUS_DEVICE can
>> claim to have MMIO regions and IRQs. "Only sysbus devices get reset"
>> is a big part of why a lot of devices today are sysbus.)
> 
> Sysbus may habe been a design mistake.  It goes back the qdev design
> assumption "every device plugs into exactly one bus, every bus is part
> of exactly one device, and the main system bus is the root of this
> tree".  The assumption ceased to hold long ago, but we still have
> sysbus.

This might explain the undocumented API called
'device_listener' in qdev which instead uses sysbus:

void device_listener_register(DeviceListener *listener);
void device_listener_unregister(DeviceListener *listener);

Thanks,

Phil.