From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37996) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmUrj-0004XB-MW for qemu-devel@nongnu.org; Thu, 28 Jul 2011 13:59:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QmUri-0002D7-7h for qemu-devel@nongnu.org; Thu, 28 Jul 2011 13:59:23 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:55592) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmUri-0002Cw-0t for qemu-devel@nongnu.org; Thu, 28 Jul 2011 13:59:22 -0400 Received: by gxk26 with SMTP id 26so2399284gxk.4 for ; Thu, 28 Jul 2011 10:59:21 -0700 (PDT) Message-ID: <4E31A374.7020705@codemonkey.ws> Date: Thu, 28 Jul 2011 12:59:16 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1311558293-5855-1-git-send-email-aliguori@us.ibm.com> <4E2EBA1E.90006@redhat.com> <4E2EC90E.8090409@codemonkey.ws> <4E2ED0AA.3020101@redhat.com> <4E2EDE86.7020807@codemonkey.ws> <4E2F06C8.30403@redhat.com> <4E2F1448.3040106@codemonkey.ws> <4E2FD28F.2070206@redhat.com> <4E30091C.3070404@codemonkey.ws> <4E302FBF.4040500@redhat.com> <4E303CBF.8070801@codemonkey.ws> <4E305E3F.2050405@redhat.com> <4E306EA5.5020207@codemonkey.ws> <4E311160.8060506@redhat.com> <4E315A3F.90804@codemonkey.ws> <4E316924.8020001@redhat.com> <4E316C36.1050405@codemonkey.ws> <4E317514.30505@redhat.com> <4E317A96.7000604@codemonkey.ws> <4E318482.9030806@redhat.com> In-Reply-To: <4E318482.9030806@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Peter Maydell , qemu-devel@nongnu.org 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 >