On 4/22/21 3:21 PM, Markus Armbruster wrote: > Peter Maydell <peter.mayd...@linaro.org> writes: > >> On Sun, 18 Apr 2021 at 21:16, Philippe Mathieu-Daudé <f4...@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.