From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [STRAWMAN PATCH] KVM: PPC: Add ioctl to specify interrupt controller architecture to emulate Date: Thu, 14 Mar 2013 13:15:35 -0500 Message-ID: <1363284935.28440.8@snotra> References: <20130314012619.GB12273@drongo> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Alexander Graf , "kvm@vger.kernel.org" , "kvm-ppc@vger.kernel.org" , Gleb Natapov , Stuart Yoder To: Paul Mackerras Return-path: Received: from tx2ehsobe004.messaging.microsoft.com ([65.55.88.14]:2269 "EHLO tx2outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932979Ab3CNSPm convert rfc822-to-8bit (ORCPT ); Thu, 14 Mar 2013 14:15:42 -0400 In-Reply-To: <20130314012619.GB12273@drongo> (from paulus@samba.org on Wed Mar 13 20:26:20 2013) Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On 03/13/2013 08:26:20 PM, Paul Mackerras wrote: > On Wed, Mar 13, 2013 at 07:14:48PM -0500, Scott Wood wrote: > > On 03/08/2013 05:04:30 AM, Alexander Graf wrote: > > > > > > > > >Am 08.03.2013 um 11:37 schrieb Paul Mackerras : > > > > > >> On Thu, Mar 07, 2013 at 03:00:52PM +0100, Alexander Graf wrote: > > >>> > > >>> Could you please (in a quick and drafty way) try and see if > > >setting the IRQ arch (using enable_cap) after the vcpu got created > > >would work for you? > > >>> > > >>> That enable_cap would then have to loop through all devices and > > >notify irq controllers that a new cpu got spawned. > > >>> All vcpu local payloads would have to get allocated and > > >initialized outside of vcpu_create too then. > > >> > > >> So, the first thing I noticed is that KVM_ENABLE_CAP is a vcpu > > >ioctl, > > >> not a vm ioctl. Apparently qemu calls it once for every vcpu > > >when it > > >> calls it on ppc targets. That means that it doesn't have to loop > > >> through all vcpus; it just needs to connect up the one it's > called > > >> for, which simplifies things. > > > > > >That's the point, yes :). And if for some weird reason one vcpu > > >isn't connected to the interrupt controller (or to a different > > >one), we can model that too ;). > > > > > >> I'm coding it up now and porting my XICS emulation to the kvm > device > > >> API proposed by Scott. It looks like it's going to be OK. > > > > > >Awesome! Scott is going to prototype whether using fds as tokens > > >makes sense. But even if we change it to an fd model, there should > > >be very little work to do to move xics to it too if it's already > > >modeled for create_device. > > > > It looks like the fd approach will be workable. Paul, do you want > > to post what you have in terms of the capability approach, so I can > > base an fd version of the device control patchset on it, or should I > > fd-ize the current patchset without it, and then rework mpic on top > > of the capability stuff once you've posted your device-control-using > > patchset? > > I have a complete patchset based on your "kvm: add device control API" > patch, tested and ready to go. :) I just posted the first patch of > that series, the one that adds the KVM_CAP_IRQ_ARCH capability. If > you're going to change the device API then I'll hold off posting the > rest of the series for now. > > > >> I have > > >> used the first argument (cap->args[0]) to specify which interrupt > > >> controller you want to connect the vcpu to. > > > > > >Ah, nice idea. So you basically make the vcpu connection explicit. > > >Perfect! Then just pass the interrupt controller pin id in > > >cap->args[1] so we don't need to guess which vcpu we're talking > > >about and all is well :). No implicit assumptions left in the > > >kernel. > > > > Is the IRQ architecture now implicit based on what sort of irqchip > > you point at, or is there a separate capability for each IRQ > > architecture? The latter may make more sense -- you can test for > > specific architectures, provide architecture-specific arguments, > > some architectures may not require pointing at a device (e.g. the > > "LAPIC in kernel, IO-APIC in userspace" model), etc. > > The way I have done it, there is one capability, and args[0] is a > token for the IRQ architecture (not a device ID). OK, so you don't specify the individual interrupt controller instance... > I arbitrarily > assigned 0x58494353 for KVM_CAP_IRQ_XICS as the args[0] value to > indicate XICS. Why is it called KVM_CAP_ if it's not a capability? > I think it would be better if we don't have to get a > new capability number assigned every time we want to add a new type of > interrupt controller. How often does it really happen? If a simple enumeration is good enough for identifying the main IRQ controller device type, it should be good enough for identifying the vcpu irq arch. -Scott