On 21.01.2013, at 01:08, Paul Mackerras wrote: > 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? Well, moving an x86 specific call into x86 specific code should be possible without kernel changes, no? :) I'm not sure how useful it is without having a new interface though. > >>> >>>> - 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. Works for me. I needs to get the type of irqchip as argument, maybe some more (like MPIC version, though that _could_ be handled through a different type too) > >> 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? Yeah, I'm always getting confused on all this too. We have 2 things we talk about here * overall architecture * individual devices hanging off of that architecture We need some link from "architecture" to "devices" because we need to be able to set MMIO offsets for the MPIC for example. On x86 and ARM today, this is implicit. The device IDs simply exist when we create a certain type of "system wide IRQCHIP". I'm not sure if that's a good idea or not. At least it needs to be properly documented and the linkage between "system wide type is X" -> "devices 1,2,3 exist" has to be clear. > > 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? I like the latter better, yes. In fact, instead of KVM_IRQ_SET_SOURCES you could use something like CREATE_DEVICE and create a source controller device, no? I wonder if that'd be any improvement over a SET_SOURCES ioctl. Hrm. >> 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. Yes. And it will be. We need it for in-kernel MPIC emulation too. Alex -- 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
