From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 3/9] KVM: PPC: Book3S: Add kernel emulation for the XICS interrupt controller Date: Fri, 15 Feb 2013 14:05:41 -0600 Message-ID: <1360958741.6960.4@snotra> References: <20130215000108.GD17099@iris.ozlabs.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Alexander Graf , , To: Paul Mackerras Return-path: In-Reply-To: <20130215000108.GD17099@iris.ozlabs.ibm.com> (from paulus@samba.org on Thu Feb 14 18:01:08 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 02/14/2013 06:01:08 PM, Paul Mackerras wrote: > From: Benjamin Herrenschmidt > > This adds in-kernel emulation of the XICS (eXternal Interrupt > Controller Specification) interrupt controller specified by PAPR, for > both HV and PR KVM guests. > > This adds a new KVM_CREATE_IRQCHIP_ARGS ioctl, which is like > KVM_CREATE_IRQCHIP in that it indicates that the virtual machine > should use in-kernel interrupt controller emulation, but also takes an > argument struct that contains the type of interrupt controller > architecture and an optional parameter. Currently only one type value > is defined, that which indicates the XICS architecture. Would the device config API I posted a couple days ago work for you? > The XICS emulation supports up to 1048560 interrupt sources. > Interrupt source numbers below 16 are reserved; 0 is used to mean no > interrupt and 2 is used for IPIs. Internally these are represented in > blocks of 1024, called ICS (interrupt controller source) entities, but > that is not visible to userspace. > > Two other new ioctls allow userspace to control the interrupt > sources. The KVM_IRQCHIP_SET_SOURCES ioctl sets the priority, > destination cpu, level/edge sensitivity and pending state of a range > of interrupt sources, creating them if they don't already exist. The > KVM_IRQCHIP_GET_SOURCES ioctl returns that information for a range of > interrupt sources (they are required to already exist). Why is it userspace's job to control these? If you use KVM_IRQ_PENDING for interrupt injection, what if there's a race with the user changing other flags via MMIO? Maybe this isn't an issue with XICS, but this is being presented as a generic API. > +4.80 KVM_CREATE_IRQCHIP_ARGS > + > +Capability: KVM_CAP_IRQCHIP_ARGS > +Architectures: ppc Why just ppc? > +struct kvm_irq_sources { > + __u32 irq; > + __u32 nr_irqs; > + __u64 __user *irqbuf; > +}; Please no pointers in UAPI -- this would require a compat wrapper with 32-bit user and 64-bit kernel. > +/* irqbuf entries are laid out like this: */ > +#define KVM_IRQ_SERVER_SHIFT 0 > +#define KVM_IRQ_SERVER_MASK 0xffffffffULL > +#define KVM_IRQ_PRIORITY_SHIFT 32 > +#define KVM_IRQ_PRIORITY_MASK 0xff > +#define KVM_IRQ_LEVEL_SENSITIVE (1ULL << 40) > +#define KVM_IRQ_MASKED (1ULL << 41) > +#define KVM_IRQ_PENDING (1ULL << 42) What does "server" mean? Do you mean "laid out like this for XICS"? Let's please have a clean separation between what is generic and what is implementation-specific. -Scott