Re: [Qemu-devel] [PATCH RFC 0/3] Recursive QOM realize

[Andreas Färber]

Am 15.07.2013 17:37, schrieb Paolo Bonzini:
> Il 15/07/2013 17:06, Andreas Färber ha scritto:
>> Am 15.07.2013 16:43, schrieb Paolo Bonzini:
>>> Il 15/07/2013 15:40, Andreas Färber ha scritto:
>>>> Originally Paolo and me had implemented QOM realize at Object level.
>>>> Paolo's goal was to set realized = true on /machine and it propagating from
>>>> there on. This series now implements {realize,unrealize}_children at
>>>> DeviceState level instead and propagates realized changes along busses 
>>>> rather
>>>> than child<> properties.
>>>
>>> You are right that realize must be done after the bus is realized (and
>>> unrealize must be done before the bus).  But I'm afraid this opens a can
>>> of worms.
>>>
>>>> On machine creation done, a depth-first search is done
>>>> for devices from /machine, which are then expected to further propagate the
>>>> property change.
>>>
>>> How do you ensure that devices are realized before their bus's parent
>>> _and_ before their parent?  With two constraints for each device, we
>>> have a graph, not anymore a tree.  Example:
>>>
>>>
>>> (1) this is the composition tree
>>>
>>>            /machine
>>>      ,------'  |   '------.
>>> /pci-host    /isa      /superio
>>>                    ,----'  '----.
>>>                  /i8254        /i8259
>>>
>>>
>>> (2) this is the bus tree
>>>
>>>                 PCI (/pci-host)
>>>                  |
>>>                 ISA (/isa)
>>>     ,-----------' '------.
>>> /superio/i8254        /superio/i8259
>>>
>>>
>>> The constraints are:
>>>
>>> - pci-host before isa
>>> - superio before superio/i8254
>>> - superio before superio/i8259
>>> - isa before superio/i8254
>>> - isa before superio/i8259
>>>
>>> So the two valid orders are
>>>
>>> - /machine, pci-host, superio, isa, superio/i8254, superio/8259
>>> - /machine, pci-host, isa, superio, superio/i8254, superio/8259
>>>
>>> You cannot say whether superio or isa are encountered first, so you
>>> cannot say whether it is superio or isa that should "hold off" the visit
>>> of their children (in either the QOM tree or the bus tree).  What avoids
>>> us having to do a full topological ordering of the graph?
>>
>> I would say your example is wrong. :) There should be no /machine/isa
>> node.
> 
> Why not?  And anyway, just replace /superio with /pcnet-isa and
> /superio/i8254 with /pcnet-isa/pcnet, and you get the same scenario.
> 
> Perhaps you could say my example is wrong, because one of the two
> constraints should not be there.  If you have a good argument for that,
> I can buy it. :)
> 
>> Is this hypothetical or do we need to fix qemu.git?
> 
> It is hypothetical, PC is not QOMified yet.
> 
>> There will be a /machine/sysbus node though, which may lead to similar
>> ordering uncertainties. However SysBusDevices don't usually have a
>> hosting device today, so I don't think it's a problem at the moment. And
>> not for busses either since they are no devices. If we have a
>> /machine/superio that would be a SysBusDevice (in PReP it would be a
>> PCIDevice and thus not directly on /machine), we would need to walk its
>> children to their bus and its parent device and assure it is realized
>> before - I think there's still sufficient time until 1.6 to get
>> something minimal sorted out.
> 
> I don't think this is 1.6 material, and there is no need to start with
> something minimal.  Let's focus on getting things right.
> 
> Perhaps "right" means that only one of the two trees need to be visited.
>  That's what I did in my old prototype, but I'm fairly convinced it was
> wrong.
> 
>> Do you have a concrete example where we need such strict constraints?
> 
> Does there need to be a concrete example?

My interest is this: Take a look at my tegra branch or PReP PCI or other
composited SysBusDevices. Today I need to realize child devices in
DeviceClass::realize, when I know that they should be realized through
realize_children instead, outside of realize. So I don't want to convert
all SysBusDevices to hand-code recursive realization (or at least the
current version of it) and then go through and remove all that again in
favor of realize_children. And since we are converting some devices for
1.6 I would like to have the work-saving solution for 1.6 as well. So it
is not about /machine (2/3 is unused) but about the realize_children
infrastructure (1/3).

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg