From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1N2SQd-0007O6-AL for qemu-devel@nongnu.org; Mon, 26 Oct 2009 12:28:19 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1N2SQY-0007Kq-84 for qemu-devel@nongnu.org; Mon, 26 Oct 2009 12:28:18 -0400 Received: from [199.232.76.173] (port=52445 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1N2SQX-0007Ka-UN for qemu-devel@nongnu.org; Mon, 26 Oct 2009 12:28:14 -0400 Received: from qw-out-1920.google.com ([74.125.92.144]:11460) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1N2SQX-0005jC-Ie for qemu-devel@nongnu.org; Mon, 26 Oct 2009 12:28:13 -0400 Received: by qw-out-1920.google.com with SMTP id 5so1652763qwc.4 for ; Mon, 26 Oct 2009 09:28:12 -0700 (PDT) Message-ID: <4AE5CE1A.4040007@codemonkey.ws> Date: Mon, 26 Oct 2009 11:28:10 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [RFC] in-kernel irqchip : split devices References: <20091014143042.GD8092@mothafucka.localdomain> <4AE427EB.7080602@redhat.com> In-Reply-To: <4AE427EB.7080602@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: Glauber Costa , qemu-devel@nongnu.org Avi Kivity wrote: > On 10/14/2009 04:30 PM, Glauber Costa wrote: >> Hello people, >> >> As I promised, I am sending a very brief PoC wrt split devices and >> in-kernel irqchip. >> In this mail, I am including only the ioapic version for apreciation. >> I also have i8259, >> and apic will take me a little bit more. This is just to try to bind >> the discussion to real >> code. >> >> > > I still can't say I like it. The reset function is duplicated, the > state representation (which is an ABI) is gratuitously forked. > > You can't save/restore in-kernel irqchip and userspace irqchip, even > though where the code is located is an implementation detail. While > we may not care much for the ioapic, it sets a bad precedent for > vhost-net, where we'd like to migrate from non-vhost-net hosts to > vhost-net hosts without the user noticing anything. > >> Note that we end up with a very slim representation of the device, >> and the code is much less >> confusing, IMHO. >> > > You can always remove if statements by duplicating the code and > pushing the if one level upwards. In total, there is more code, and > it is more confusing (since you need to deal with implementation > details at a higher level). I'm surprised you feel this way. Maybe this is an issue of having the model in your head vs. not having it because the current in-kernel code is extremely confusing IMHO. When you look at ioapic.c in qemu-kvm, the first question I ask is, "what parts of this code is used when using in-kernel apic?". The answer is not at all obvious. To understand it, you have to first search for kvm_enabled() and you'll see that during save/restore the state is synced with the in-kernel state. However, it's not clear whether pio/mmio operations still get processed and certainly not clear whether ioapic_set_irq() is not called anymore. In fact, I think you to start with the assumption that it is which leads you to wonder why it doesn't do kvm_set_irq(). The answers are all subtle and have to do with weird things about how the isa irqs are allocated. It's extremely confusing to someone who doesn't know exactly what's going on. OTOH, the split model makes this all very obvious. Sure there's some duplication but at the end of the day, you don't have to understand very much to see what's going on. We just use userspace for device save/restore and reset support. Code readability wins in my mind over reducing a couple dozen lines of code. Regards, Anthony Liguori