All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model
Date: Tue, 26 Jul 2011 20:26:16 +0200	[thread overview]
Message-ID: <4E2F06C8.30403@redhat.com> (raw)
In-Reply-To: <4E2EDE86.7020807@codemonkey.ws>

On 07/26/2011 05:34 PM, Anthony Liguori wrote:
>> And as such it can add data members. But when a device is on two buses,
>> you cannot have both of them adding data members. I know MI is hard to
>> get right, and in fact I'm not proposing to do MI---not even interface
>> inheritance. I don't want to have any base class but DeviceState.
>
> I could use a concrete example here, but here's how this would be
> expressed:
>
> class MyWeirdDevice : public MyBaseDevice, implements PciDevice, IsaDevice
> {
> PciBus *pci_bus;
> IsaDevice *isa_bus;
> };
>
> Which actually models hw pretty well. A device that exists on two busses
> has to have two sockets that can accept the plugs from the busses
> they're joining.
>
> It's pretty much exactly how it works in real life.

You could just as well say that real life works like this:

class PciSocket {
     PciBus *pci_bus;
     uint8_t *config;
     uint8_t *cmask;
     uint8_t *wmask;
     uint8_t *w1cmask;
     uint8_t *used;
     uint32_t devfn;
     char name[64];
     PCIIORegion io_regions[PCI_NUM_REGIONS];
     ...

};

class IsaSocket {
     IsaBus *isa_bus;
     uint32_t isairq[2];  // not sure this is a good idea, just
     int nirqs;           // mimicking qdev
     uint16_t ioports[32];// statically assigned unlike PCI bars
     int nioports;
}

class MyWeirdDevice : public MyBaseDevice {
     PciSocket pci;
     IsaSocket isa;
};


>> Yes, this is pretty much what I had imagined. But it does not scale to a
>> topology where you have two parents, both of which want to add data
>> members.
>
> Which would be true MI. The problem with true MI is that you'll end up
> with a diamond if you try to have anything useful (like properties) in
> the base.

See above for how you would avoid that.

>>>> 1) in a flexible manner, so that it can express complex topologies (as
>>>> long as "plugs" and "sockets" have the same shape of course);
>>>
>>> Right, this is what we do today in QOM. Plugs and Sockets are typed.
>>> Those types can be interfaces or base classes so there's a lot of
>>> flexibility.
>>
>> Interfaces (is-a) are less flexible than embedded objects (has-a).
>
> It depends on what you're trying to model I guess. In some cases where
> the interface is more or less stateless or that state isn't common among
> implementations, interfaces are quite nice.

Yes, I'm not sure this is the case here. :)

>  > Same as IDE when you used to set
>> jumpers to choose master/slave. Or ISA interrupt lines.
>
> The ISA dip switches literally select which line of the bus gets routed
> to the device. All devices have access to all interrupt lines through
> the bus.
>
> So when modelling, it makes sense to have the irq be a property of the
> device and then to have the bus interface expose the irqs. For instance:
>
> struct IsaBusClass
> {
> void (*set_irq_level)(IsaBus *bus, int irq, bool level);
> };

Agreed.  Though I'm not sure that the same is true for all fields in 
qemu's current PCIDevice.

>> Once you have something like this for a device that bridges two buses,
>> interfaces require a lot of boilerplate for useless getters/setters.
>
> Can you elaborate?

If you store data (configuration space etc.) in the device, and the bus 
has to access it, you need getters/setters in the interface.  Letting 
the bus hold an interior reference to the PciSocket (perhaps adding a 
single get_device_for_socket function to the PciSocketOps) solves the 
problem.

>>> The same applies equally to IDE.
>>>
>>> ide->primary.master = disk1;
>>> ide->secondary.master = cdrom;
>>
>> For IDE, an equally good model would be:
>>
>> ide->primary.add(disk1);
>> disk1.masterSlave = MASTER;
>> ide->secondary.add(cdrom);
>> cdrom.masterSlave = MASTER;
>
> There's a pin in the IDE cable that determines master or slave depending
> on whether it's raised high.

Yes, that's the "newer" way.  There used to be jumpers to choose between 
master, slave and cable-select.

> I think modelling it that way makes more sense from an end user
> perspective since it prevents the possibility of have two master devices
> (which is incorrect).

... but you could do that, if for some reason you wanted to model the 
jumper-based world.  But this is a mostly irrelevant digression.

>>> Interfaces are the right way to do this. Getting MI right is fairly hard
>>
>> But we don't need is-a, we need has-a. Multiple is-a is harder than
>> single is-a. Multiple has-a does not add any complication.
>
> Yeah, that's what plug properties are for :-)

I agree, but at the cost of pointer chasing and making it harder to 
implement get_device_for_socket for buses that need it (in the above 
sketch it can be a simple container_of).

>>> I think all of the requirements you've outlined are currently handled in
>>> QOM.
>>
>> They more than likely are. The question is whether they're handled in
>> the most programmer-efficient manner, and whether the advantages of a
>> single grand unified object model for host and guest devices is worth
>> the effort.
>
> Indeed. I think that it's a no brainer for the backends and that's why
> I'm starting there.

I don't think it's a no brainer.  It's simply much easier, but right now 
it is also a solution in search of a problem (if all you want is dynamic 
creation of character devices, you could do that without a generic 
object model).

If starting from a blank slate, I would be much more enthusiastic.  But 
all that QEMU does _not_ need is yet another incomplete transition.

Paolo

  reply	other threads:[~2011-07-26 18:26 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-25  1:44 [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 01/21] qom: add make infrastructure Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 02/21] qom: convert QAPI to use Qconfig build system Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 03/21] qom: Add core type system Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 04/21] qom: add Plug class Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 05/21] plug: add Plug property type Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 06/21] plug: add socket " Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 07/21] plug: add generated property types Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 08/21] qom: add plug_create QMP command Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 09/21] qom: add plug_list " Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 10/21] qom: add plug_get " Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 11/21] qom: add plug_set " Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 12/21] qom: add plug_list_props " Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 13/21] qom: add plug_destroy command Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 14/21] qom: add example qsh command Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 15/21] qom: add Device class Anthony Liguori
2011-07-27 15:10   ` Peter Maydell
2011-07-27 16:07     ` Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 16/21] qom-devices: add a Pin class Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 17/21] qom: add CharDriver class Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 18/21] qom-chrdrv: add memory character driver Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 19/21] qom-chrdrv: add Socket base class Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 20/21] qom-chrdrv: add TCPServer class Anthony Liguori
2011-07-25  1:44 ` [Qemu-devel] [PATCH 21/21] qom-chrdrv: add UnixServer Anthony Liguori
2011-07-25 11:21 ` [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model Kevin Wolf
2011-07-25 12:45   ` Anthony Liguori
2011-07-25 13:08     ` Kevin Wolf
2011-07-25 13:10       ` Anthony Liguori
2011-07-26 12:59 ` Paolo Bonzini
2011-07-26 14:02   ` Anthony Liguori
2011-07-26 14:35     ` Paolo Bonzini
2011-07-26 15:34       ` Anthony Liguori
2011-07-26 18:26         ` Paolo Bonzini [this message]
2011-07-26 19:23           ` Anthony Liguori
2011-07-27  8:55             ` Paolo Bonzini
2011-07-27 12:48               ` Anthony Liguori
2011-07-27 15:33                 ` Paolo Bonzini
2011-07-27 16:19                   ` Anthony Liguori
2011-07-27 16:28                   ` Anthony Liguori
2011-07-27 18:51                     ` Paolo Bonzini
2011-07-27 20:01                       ` Anthony Liguori
2011-07-28  7:36                         ` Paolo Bonzini
2011-07-28 12:46                           ` Anthony Liguori
2011-07-28 13:50                             ` Paolo Bonzini
2011-07-28 14:03                               ` Anthony Liguori
2011-07-28 14:41                                 ` Paolo Bonzini
2011-07-28 15:04                                   ` Anthony Liguori
2011-07-28 15:47                                     ` Paolo Bonzini
2011-07-28 17:59                                       ` Anthony Liguori
2011-07-29  7:19                                         ` Paolo Bonzini
2011-07-27 21:33     ` Peter Maydell
2011-07-27 22:31       ` Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E2F06C8.30403@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.