From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=50250 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pokdx-000578-Jz for qemu-devel@nongnu.org; Sun, 13 Feb 2011 17:42:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pokdv-00071k-VM for qemu-devel@nongnu.org; Sun, 13 Feb 2011 17:42:13 -0500 Received: from mail-yx0-f173.google.com ([209.85.213.173]:64303) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pokdv-00071d-PN for qemu-devel@nongnu.org; Sun, 13 Feb 2011 17:42:11 -0500 Received: by yxl31 with SMTP id 31so2008424yxl.4 for ; Sun, 13 Feb 2011 14:42:11 -0800 (PST) Message-ID: <4D585E3C.9010404@codemonkey.ws> Date: Sun, 13 Feb 2011 16:42:04 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] KVM call minutes for Feb 8 References: <4D52F20A.7070009@codemonkey.ws> <4D539800.3070802@codemonkey.ws> <20110210090748.GD673@redhat.com> <4D53BD22.1040800@redhat.com> <20110210111354.GA21681@redhat.com> <4D53DF42.4030700@codemonkey.ws> <20110210132730.GB24525@redhat.com> <4D53F06C.9090500@codemonkey.ws> <20110210142044.GD24525@redhat.com> <4D540CC5.2@codemonkey.ws> <4D57F96B.7010004@codemonkey.ws> <4D583793.10409@codemonkey.ws> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Chris Wright , kvm@vger.kernel.org, Gleb Natapov , Markus Armbruster , qemu-devel@nongnu.org, Avi Kivity On 02/13/2011 03:00 PM, Blue Swirl wrote: > On Sun, Feb 13, 2011 at 9:57 PM, Anthony Liguori wrote: > >> On 02/13/2011 01:37 PM, Blue Swirl wrote: >> >>> On Sun, Feb 13, 2011 at 5:31 PM, Anthony Liguori >>> wrote: >>> >>> >>>> qdev doesn't expose any state today. qdev properties are >>>> construction-only >>>> properties that happen to be stored in each device state. >>>> >>>> What we really need is a full property framework that includes properties >>>> with hookable getters and setters along with the ability to mark >>>> properties >>>> as construct-only, read-only, or read-write. >>>> >>>> But I think it's reasonable to expose construct-only properties as just >>>> an >>>> initfn argument. >>>> >>>> >>> Sounds OK. About read-write properties, what happens if we one day >>> have extensive threading, and locks are pushed to device level? I can >>> imagine a deadlock involving one thread running in IO thread for a >>> device and another trying to access that device's properties. Maybe >>> that is not different from function call version. >>> >>> >> You need hookable setters/getters that can acquire a lock and do the right >> thing. It shouldn't be able to dead lock if the locking is designed right. >> >> >> >>>> Yes, but qemu_irq is very restricted as it only models a signal bit of >>>> information and doesn't really have a mechanism to attach/detach in any >>>> generic way. >>>> >>>> >>> Basic signals are already very useful for many purposes, since they >>> match digital logic signals in real HW. In theory, whole machines >>> could be constructed with just qemu_irq and NAND gate emulator. ;-) >>> >>> >> It's not just in theory. In the C++ port of QEMU that I wrote, I >> implemented an AND, OR, and XOR gate and implemented a full 32-bit adder by >> just using a device config file. >> >> If done correctly, using referencing can be extremely powerful. A full >> adder is a good example. The gates really don't have any concept of bus and >> the relationship between gates is definitely not a tree. >> >> >>> In the message passing IRQ discussion earlier, it was IIRC decided >>> that the one bit version would not be changed but a separate message >>> passing version would be created if ever needed. >>> >>> >> C already has a message passing interface that supports type safety called >> function pointers :-) >> >> An object that implements multiple interfaces where the interface becomes >> the "message passing interface" is exactly what I've been saying we need. >> It's flexible and the compiler helps us enforce typing. >> >> >>>> Any interfaces of a base class should make sense even for derived >>>> classes. >>>> >>>> That means if the base class is going to expose essentially a pin-out >>>> interface, that if I have a PCIDevice and cast it to Device, I should be >>>> able to interact with the GPIO interface to interact with the PCI device. >>>> Presumably, that means interfacing at the PCI signalling level. That's >>>> insane to model in QEMU :-) >>>> >>>> >>> This would be doable, if we built buses from a bunch of signals, like >>> in VHDL or Verilog. It would simplify aliased MMIO addresses nicely, >>> the undecoded address pins would be ignored. I don't think it would be >>> useful, but a separate interface could be added for connecting to >>> PCIBus with just qemu_irqs. >>> >>> >> Yeah, it's possible, but I don't want to spend my time doing this. >> >> >>>> In reality, GPIO only makes sense for a small class of simple devices >>>> where >>>> modelling the pin-out interface makes sense (like a 7-segment LCD). That >>>> suggests that GPIO should not be in the DeviceState interface but instead >>>> should be in a SimpleDevice subclass or something like that. >>>> >>>> >>>> >>>>> Could you point to examples of SystemBus overuse? >>>>> >>>>> >>>>> >>>> anthony@titi:~/git/qemu/hw$ grep qdev_create *.c | wc -l >>>> 73 >>>> anthony@titi:~/git/qemu/hw$ grep 'qdev_create(NULL' *.c | wc -l >>>> 56 >>>> >>>> SystemBus has become a catch-all for shallow qdev conversions. We've got >>>> Northbridges, RAM, and network devices sitting on the same bus... >>>> >>>> >>> On Sparc32 I have not bothered to create a SBus bus. Now it would be >>> useful to get bootindex corrected. Most devices (even on-board IO) >>> should use SBus. >>> >>> The only other bus (MBus) would exist between CPU, IOMMU and memory. >>> >>> But SysBus fitted the need until recently. >>> >>> >> A good way to judge where a device is using a bus interface correct: does >> all of it's interactions with any other part of the guest state involve >> method calls to the bus? >> >> Right now, the answer is no for just about every device in QEMU. This is >> the problem that qdev really was meant to solve and we're not really any >> further along solving it unfortunately. >> >> >>>> I'm not arguing against a generic factory interface, I'm arguing that it >>>> should be separate. >>>> >>>> IOW: >>>> >>>> SerialState *serial_create(int iobase, int irq, ...); >>>> >>>> static DeviceState *qdev_serial_create(QemuOpts *opts); >>>> >>>> static void serial_init(void) >>>> { >>>> qdev_register("serial", qdev_serial_create); >>>> } >>>> >>>> The key point is that when we create devices internally, we should have a >>>> C-friendly, type-safe interface to interact with. This will encourage >>>> composition and a richer device model than what we have today. >>>> >>>> >>> Isn't this what we have now in most cases? >>> >>> >> No. The common pattern of shallow conversion is: >> >> struct SerialState >> { >> // device state >> }; >> >> struct ISASerialState >> { >> ISADeviceState parent; >> SerialState serial; >> }; >> >> void serial_init(SerialState *s); >> >> void isa_serial_init(ISADevice *dev) >> { >> ISASerialState *s = DO_UPCAST(dev); >> serial_init(&s->serial); >> } >> >> The problem with this is that you cannot use serial_init() if you want to >> have the device be reflected in the device tree. >> > But why would serial_init() be used anymore since isa_serial_init() is as good? > The point is that it's not. Instead of doing error checking in one call, you've gotta do error checking in a half dozen calls because each of the set calls can fail. >> GObject takes a different approach than I'm suggesting that is equally >> valid. It supports a vararg constructor form and then provides C-friendly >> interfaces that use the vararg form. For instance: >> >> SerialState *serial_init(int iobase, int irq, ...) >> { >> return gobject_new(QEMU_SERIAL, "iobase", iobase, "irq", irq, ..., >> NULL); >> } >> >> This is not a bad solution but what I was trying to avoid in my suggestion >> is a lot of the ugliness of supporting a factory initializer. It may be a >> better approach in the long run though. >> > Producing varargs lists may get messy if the list is filled > differently based on some conditions. It's also not easy to do things > in between: > dev = isa_create(); > qdev_prop_set_uint32(dev); > if (flag1) { > qdev_prop_set_uint32(dev); > } > if (flag2) { > int f = func(); > qdev_prop_set_uint32(dev, f); > } > qdev_prop_set_uint32(dev); > qdev_init_nofail(dev); > Vararg is designed for direct invocation. You can still build a list of construction properties if you're so inclined. Regards, Anthony Liguori