Re: [Qemu-devel] [RFC] Plan for moving forward with QOM

[Anthony Liguori <anthony@codemonkey.ws>]

On 12/13/2011 02:36 PM, Paul Brook wrote:
>> Composition == "etching to the same piece of silicon".  Nothing more,
>> nothing less.
>
> That's a really crappy definition.  Most ARM systems are comprised of
> approximately one piece of silicon.  There may be dozens of variants of a
> particular chip, with the same device models stamped into the silicon slightly
> different ways.
>
> IMO it would be much better to split on logical device boundaries.  Especially
> when the functionality provided (pci to ISA bridge) works perfectly well
> without the child device.

What I'm really after is to have a way to do "-device piix3" and get all of the 
devices that would normally live on the piix3.  If it's desirable to split out 
piix3 into piix3-isa-bridge which is part of piix3 and can be used independently 
such that piix3 is just a thin container of piix3-isa-bridge and everything 
behind it, that's fine by me.

We should do whatever makes sense for the machine/target.  For pc, I've seen 
absolutely zero interest in someone creating a pc machine without an i8042 or an 
RTC.  I don't think it's the use-case to optimize for.

>> A device has no knowledge of the fact that it is a child of another device.
>>
>>> If you're wanting to provide a convenient way of instantiating common
>>> clumps of devices then I think that should be separate.  Likewise
>>> defining convenient machine specific aliases for users.
>>
>> There is a separate container device that lets you create arbitrary
>> collections of devices, but in many cases, we can achieve better code
>> reuse by using composition.
>>
>> Consider the serial device.  In QOM, I've got it modelled as:
>>
>> MMSerial has-a UART
>> ISASerial has-a UART
>>
>> And there are lots of cases where a design is compatible with a UART16650A
>> but adds additional logic/features.  In those cases, we would do:
>>
>> MCFSerial is-a UART
>
> Are you using UART as shorthand for UART16550A?

Yes.

> Or is UART a base object that
> implements pure virtual methods for register access and properties for
> CharDevice and IRQ output. UART16550 inherits from and provides the
> implementation? So a UART8250 or UART6551 could act as drop-in replacements
> (replacing has-a UART with links-to-a UART).
>
>> Now you could certainly remove the has-a logic and make it so something had
>> to instantiate a UART separately from the ISASerial device and connect
>> things up but that's overly complicated IMHO.
>
> Why not ISASerial is-a UART?

That's actually how I implemented MMSerial just to show that it's possible.

I prefer has-a because if you had a real life ISA card with a UART16550A on it, 
the ISA card would have an identifiable UART16650A on it.  I think it's unlikely 
that you would have a single chip with an ISA-compatible pin-out that happened 
to be compatible with a UART16550A.

> I guess because both are stateful, so not allowed
> by our inheritance model?

Right, but ISADevice could very easily become an Interface as it has no state today.

> We could presumably do:
>    ISASerial is-a UART has-a ISADevice
> It seems like a map-isa-io-to-registers device has at least as may reuse
> opportunities as register-based-uart.

We could also do that, but we still need to work out how to expose MemoryRegions 
and AddressSpaces in the new device model.

>
> How is MCFSerial different?  Are we expecting MCFSerial to poke around in
> private UART16550 rather than just feeding it cooked data?  If so isn't it
> mode likely to end up as:

Yes, it could go either way really but I would expect that MCFSerial would 
override the ioport_read() and ioport_write functions and implement custom 
functionality that way.

>
> UARTMCF is-a UART16550 // with knobs on.
> MCFSerial is-a ColdfireBusDevice has-a UART
>
>>> So the link is to an Interface, not a device?
>>
>> Devices are Objects
>>
>> All Objects have Types.  An interface is just another Type.  Types can
>> inherit from other types.
>
> Ah, I think I see.  The trick is that this effectively gives multiple
> inheritance as long as no more than one base type has state (i.e. data
> members).

Exactly.

>
>> All devices inherit from the DeviceState type.
>>
>> A link has a Type associated with it.  The Type can be an interface, or it
>> can be a stateful base class, or it can be a Type that isn't derived at
>> all from Device (like a CharDriverState).
>
> Ok. I suspect we may have slightly different ideas what constitutes a
> "Device", but I'll leave that for now.
>
>>>   To support multiple instances
>>> of the same interface we use a closure-like proxy object?
>>
>> If you needed to, but I think most of the time, that's not necessary.
>
> I don't see how this can work without a closure object.  We need a central
> device that is capable of recieving signals from many client devices.  Those
> client devices don't know where they are, they just shout down a point-point
> link. I'd say this is a fairly common topology.
>
> If the central device implements that point-point interface directly then it
> has no idea which device is talking to it.  We need to create child objects
> with a port ID property and implementing the p-p interface, then bind each
> client device to one of these child objects.  The child objects can't do
> anything useful on their own, so need to proxy the signal to an interface on
> the main object, adding in their port id.

If you aren't using inheritance, yes, you need to pass closures to the child 
objects.  I dislike that kind of proxy modeling.

I wonder how common is it though to have this kind of scenario where the bus 
protocol doesn't have any kind of identifier that identifies the target bus.  I 
presume in most cases you're thinking about, there's a single transport that can 
multiplex communications, no?

If there is a bus identifier in the various interface commands, there is no need 
to implement the interface multiple times.  You can just inherit from it once 
and use the bus identifier to determine which bus it's intended for.

Regards,

Anthony Liguori

>
> Paul
>