On Thu, Jan 10, 2013 at 12:42:16PM +0100, Alexander Graf wrote: > > On 10.01.2013, at 06:09, Paul Mackerras wrote: > > > On Sat, Dec 15, 2012 at 01:06:13PM +1100, Benjamin Herrenschmidt wrote: [snip] > >> - 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. OK, that makes sense, but it presumably can't be done until the new kernel interface exists, right? > > > >> - 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). So, I propose KVM_CREATE_IRQCHIP_ARGS as the version of the ioctl that indicates the type of irqchip. The parameterless version already gives an 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. I think it depends on what you mean by IRQCHIP. If you mean the overall interrupt controller architecture, then no it doesn't make sense to have more than one. The overall architecture might well contain multiple identifiable pieces which could be considered IRQCHIPs. This is the case on x86 already, where they have two i8259-style PICs and one or more IOAPICs (and presumably emulated LAPICs as well). It's the case for the PAPR-style XICS controller, where we have a presentation controller (ICP) per CPU (like the x86 LAPIC), plus one or more source controllers (ICS) (like the x86 IOAPIC). Which meaning of IRQCHIP do you prefer? My series of patches uses KVM_CREATE_IRQCHIP_ARGS to indicate that the overall architecture is XICS (by creating the ICPs) and to create the source controllers (ICS), so it gets called multiple times. It would be possible to have KVM_CREATE_IRQCHIP_ARGS called just once with a parameter saying "XICS", and then create the source controllers implicitly as a side effect of the first KVM_IRQ_SET_SOURCES ioctl that refers to it. Would that be preferable? > 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. That ioctl looks like it should be generic, and would be useful for MPIC (OpenPIC) emulation. I don't need it for PAPR since the interrupt controllers are accessed through hcalls, not MMIO. 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
