On 10.01.2013, at 06:09, Paul Mackerras wrote: > 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. This part is about QEMU. > >> - 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? KVM_CREATE_IRQCHIP should get a parameter indicating the type of IRQCHIP (XICS / MPIC / APIC / VGIC / ...). The parameter-less version defaults to an arch specific default (APIC for x86, VGIC for arm, error on PPC). I'm not 100% sure yet whether we want to support models with multiple IRQCHIPs. If so, we need to return a device token from the KVM_CREATE_IRQCHIP ioctl. Maybe it makes more sense to model specific cases like this as separate type though with specific, explicit subdevice ids. Not sure. Other instantiation attributes we don't know that early on should be settable between the time frame of vm creation and first execution. An example for this are device addresses. Check out the threads "KVM: ARM: Rename KVM_SET_DEVICE_ADDRESS" and "KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl" for more information on this particular bit. Alex > >> * 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
