Re: [RFC PATCH 0/9] KVM: PPC: In-kernel PAPR interrupt controller emulation

[Paul Mackerras <paulus@samba.org>]

On Sat, Dec 15, 2012 at 01:06:13PM +1100, Benjamin Herrenschmidt wrote:
> On Sat, 2012-12-15 at 01:46 +0100, Alexander Graf wrote:
> > On 05.11.2012, at 04:18, Paul Mackerras wrote:
> > 
> > > This series implements in-kernel emulation of the XICS interrupt
> > > controller specified in the PAPR specification and used by pseries
> > > guests.  Since the XICS is organized a bit differently to the
> > > interrupt controllers used on x86 machines, I have defined some new
> > > ioctls for setting up the XICS and for saving and restoring its
> > > state.  It may be possible to use some of the currently x86-specific
> > > ioctls instead.
> > 
> > Is this one already following the new world order that we discussed during KVM Forum? :)
> 
> The "new world order" .... (sorry, looks like nobody took notes and
> people expect me to do a write up from memory now ... :-)
> 
> Well, mostly we agreed that the x86 stuff wasn't going to work for us.
> 
> So basically what we discussed boils down to:
> 
>  - Move the existing "generic" KVM ioctl to create the kernel APIC to
> x86 since it's no as-is useful for other archs who, among other things,
> might need to pass argument based on the machine type (type of interrupt
> subsystem for example, etc...)

Assuming you're talking about KVM_CREATE_IRQCHIP, it is already
handled entirely in arch code (arch/x86/kvm/x86.c and
arch/ia64/kvm/kvm-ia64.c), along with KVM_GET_IRQCHIP and
KVM_SET_IRQCHIP.

>  - Once that's done, well, instanciating interrupt controller objects
> becomes pretty much an arch specific matter. We could try to keep some
> ioctls somewhat common though it's not *that* useful because the various
> types & arguments are going to be fairly arch specific, so goes for
> configuration. Examples of what could be kept common are:
> 
>    * Create irq chip, takes at least a "type" argument, possibly a few
> more type-specific args (or a union, but then let's keep space in there
> since we can't change the size of the struct later as it would impact
> the ioctl number afaik).

The existing KVM_CREATE_IRQCHIP is defined as _IO(...) which means
that it doesn't read or write memory, but there is still the 3rd
argument to the ioctl, which would give us 64 bits to indicate the
type of the top-level IRQ controller (XICS, MPIC, ...), but not much
more.

It sounds like the agreement at the meeting was more along the lines
of the KVM_CREATE_IRQCHIP_ARGS ioctl (as in the patches I posted)
which can be called multiple times to instantiate pieces of the
interrupt framework, rather than having a KVM_CREATE_IRQCHIP that gets
called once early on to say that there we are using in-kernel
interrupt controller emulation, followed by other calls to configure
the various parts of the framework.  Is that accurate?

>   * Assigning the address of an irq chip when it can change (see ARM
> patches)
> 
>  - The existing business with irqfd etc... is mostly ok, except the
> interfaces messing around with MSIs (see virtio-pci use of kvm functions
> directly). The assignment of an irq number for an MSI must be a hook,
> probably a PCI controller hook, so x86 can get it done via its existing
> kernel interfaces and sane architectures can keep the assignment in qemu
> where it belongs.

So, if I've understood you correctly about what was agreed, the set of
ioctls that is implemented in the patches I posted is in line with
what was agreed, isn't it?  If not, where does it differ?  (To recap,
I had KVM_CREATE_IRQCHIP_ARGS, KVM_IRQCHIP_GET_SOURCES and
KVM_IRQCHIP_SET_SOURCES, plus a one-reg interface to get/set the
vcpu-specific state.)

Paul.