From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=52899 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OnHgy-00048c-EU for qemu-devel@nongnu.org; Sun, 22 Aug 2010 17:03:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OnHgx-0003yY-9l for qemu-devel@nongnu.org; Sun, 22 Aug 2010 17:03:00 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:33540) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OnHgx-0003yS-5B for qemu-devel@nongnu.org; Sun, 22 Aug 2010 17:02:59 -0400 Received: by gxk5 with SMTP id 5so2060138gxk.4 for ; Sun, 22 Aug 2010 14:02:58 -0700 (PDT) Message-ID: <4C719080.4030202@codemonkey.ws> Date: Sun, 22 Aug 2010 16:02:56 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2 0/7] APIC/IOAPIC cleanup References: <4C6D86F9.3010602@codemonkey.ws> <4C6D98E7.9020109@codemonkey.ws> <4C6DA75D.40303@codemonkey.ws> <4C6ECBB7.7060608@codemonkey.ws> <4C718865.7010807@redhat.com> In-Reply-To: <4C718865.7010807@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Blue Swirl , "Liu >> \"Liu, Jinsong\"" , qemu-devel , Markus Armbruster , Paul Brook On 08/22/2010 03:28 PM, Avi Kivity wrote: > On 08/20/2010 09:38 PM, Anthony Liguori wrote: >>> While that might be useful, I don't quite see what makes CPUs so >>> special >>> that they need to be kept out of qdev. Could be just my ignorance, of >>> course. >> >> CPUs have special relationships with things like memory in QEMU. You >> can argue that a device is anything with pins and that CPUs are just >> like any other chip. > > We're not modelling chips! If we declare something a device, we do it > because it's functionally a device. It could be part of a chip, or > spread along multiple chips. This is really a fundamental discussion. If you look closely at qdev in it's current form, what it actually models is a device with GPIO input and output whereas the GPIO input and output correspond to qemu_irqs which really model pins that can be raised and lowered. To me, this is insane and I'm looking to move the GPIO stuff out of qdev. There are some devices where it makes sense to model the interactions between pins but not for the vast majority of devices. But we really need to spend some more energy on how we model the device tree because it's important to work out the interface that we want to represent without going too low. >> But do we really want to model memory chipsets, a north and south >> bridge, and long with cache hierarchies? > > We do model devices within the north and south bridges. The > aggregation into two chips is largely meaningless from a functional > point of view, as are cache hierarchies. Right, but it's an issue of how we want devices to interact. This is the important thing to figure out. Implementations languages don't matter compared to getting the object model right. If we tried to model the device tree based on pin-outs being the interface between devices at the lowest level, we're going to fail in any language. >> If we wanted to add per-device locking based on putting a lock in >> DeviceState that was acquired and released whenever you executed a >> PIO, how would you do that today? >> >> You would convert cpu_register_ioport_* to take a DeviceState in >> serial_init_core. Sure, you could add a layer of indirection in >> ISASerialDevice, but what about timers? We would want to implement >> device based timers to do the same thing but again, when we register >> the timers we don't have a DeviceState. >> >> All device callbacks should be based on DeviceState * pointers which >> means if we want to share device code between multiple interfaces (be >> it ISA, PCI, or a SysBus device), we need to have a bus in between. > > How can you do that? Do you mean that a timer calls > DeviceState::ops->timer(DeviceState *)? How do you handle multiple > timers then? No. We have two types of timers today. vm_clock based timers and rt_clock based timers. It's always a bug for a device model to use an rt_clock based timer. We ought to have a separate API for vm_clock based timers and it makes sense to tie that API to DeviceState. For instance: typedef struct Timer Timer; void timer_init(DeviceState *, void (*fn)(Timer *)); void timer_update_rel_ns(Timer *); void timer_cancel(Timer *); void timer_release(Timer *); Timer objects get embedded into the device's state and container_of can be used to get to the original device state. We could also pass DeviceState. It's not clear to me which is better. But being able to associate timers with devices seems like a very good idea to me because it means that you can see which devices are registering timers. Regards, Anthony Liguori > Much better to call a traditional callback which then uses > container_of() to locate its state. >