All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model
Date: Thu, 28 Jul 2011 12:59:16 -0500	[thread overview]
Message-ID: <4E31A374.7020705@codemonkey.ws> (raw)
In-Reply-To: <4E318482.9030806@redhat.com>

On 07/28/2011 10:47 AM, Paolo Bonzini wrote:
> On 07/28/2011 05:04 PM, Anthony Liguori wrote:
>> The only way I can see is to teach each device about this interface and
>> then have a common bus. That implies that you have:
>>
>> class GoldfishEnumerator : public Device {
>> GoldfishDevice *slots[N];
>
> FWIW, there's no hardcoded limit in the interface, and the list of
> devices is unordered. But that only means you should attach it with
>
> plug-set goldfish_tty::enumerator goldfish_enum
>
> rather than
>
> plug-set goldfish_enum::slots[12] goldfish_tty
>
> If you can confirm that, that's fine.

Yes, you can certainly have an enumerator socket that when set, 
automatically connects the appropriate properties in the enumerator.

That way you don't have to connect things by hand.

>> };
>>
>> interface GoldfishDevice {
>> const char *get_name();
>> uint64_t get_mmio_base();
>> ...
>> };
>>
>> class GoldfishNic : public Device, implements GoldfishDevice
>> {
>> const char *get_name(void) {
>> return "nic";
>> }
>
> uint64_t mmio_base;
> uint64_t get_mmio_base() { return mmio_base; }
> uint64_t set_mmio_base(uint64_t addr) { mmio_base = addr; }
>
>> };
>
> And that's exactly my point. It's a "stupid" interface full of
> getters/setters, which is what you get if you use only interface
> inheritance instead of, where appropriate, data containment.
>

You can certainly do:

struct GoldfishEnumInfo
{
     const char *name;
     uint64_t mmio_base;
};

interface GoldfishDevice {
     GoldfishEnumInfo *get_info();
}

And then:

GoldfishEnumInfo *goldfish_nic_get_info(GoldFishNic *nic)
{
     return nic->enuminfo;
}

> Interfaces should be reserved for what really depends on the
> _implementation_ of the GoldfishNic, not for accessing a bunch of
> numbers.

Just define an interface that returns a struct then.  It's no more 
complicated than that.

What I struggle with is whether you're suggesting that the info isn't 
part of the device or whether it is part of the device and you just 
think that we shouldn't need 10 different accessors.

> There is no implementation-dependent detail of that kind in the
> GoldfishDevice (unlike other buses, even simple ones like I2C).
>
>>> The PIC's view is more complicated than a Pin, and more similar to ISA.
>>
>> ISA is just a pin. The ISA bus extender literally has five pins
>> corresponding to the ISA IRQs 7, 6, 5, 4, 3.
>
> ISA is many pins. :) Goldfish looks similar (32 pins).

Sorry, I meant to say ISA IRQs are just exposed as pins.

My real point is, doing:

class IsaDevice : public Device {
    Pin irq[16];
};

class MyIsaDevice : public IsaDevice {
    int irq_index;
};

And then doing:

my_isa_device->irq_index = 5;

Is a very good way to model ISA IRQ selection.

You can simplify it by having a single IRQ output for each ISA device 
instead of any possible IRQ and then route the IRQ as part of the bus 
connection.

Regards,

Anthony Liguori

> Paolo
>

  reply	other threads:[~2011-07-28 17:59 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
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 [this message]
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=4E31A374.7020705@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=pbonzini@redhat.com \
    --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.