On 9 January 2013 14:58, Alexander Graf <ag...@suse.de> wrote: > > On 09.01.2013, at 15:48, Peter Maydell wrote: > >> On 9 January 2013 10:02, Alexander Graf <ag...@suse.de> wrote: >>> I think we should make thus at least potentially generic. In fact, I >>> wouldn't even >>> mind calling it DEV_REG with the same semantics as ONE_REG, just that it >>> also >>> gets a unique dev id that gets created during in-kernel device creation and >>> that >>> it's a vm ioctl. >> >> Well, we might want a DEV_REG, but you might as well just make ONE_REG >> OK as a VM ioctl, since there's no reason not to have not-per-cpu but not >> device registers. ONE_REG already supports dividing up the ID space so >> you can say which device or whatever is being used, because we had >> things like GIC registers in mind when we put that API together. > > ONE_REG's address space today doesn't include a field for the target device,
There's 16 bits of "register group type". > because that's already predetermined by the fd you run it on. Hence my > suggestion to add it as separate field, because then we keep the id space > mask-free. Er, it already has masks for the different parts of ID space. That's a feature, not a bug. >> However, this shouldn't be DEV_REG, because this isn't actually setting state >> in the irqchip device, it's configuring the kernel's part of the system model >> [compare wiring up irq routing, which also has a custom ioctl rather than a >> generic one]. As such it definitely needs to happen only before the VM is >> actually started and not during VM execution, unlike a DEV_REG which would >> presumably be generally valid. > > The same can be true for ONE_REG/SET_SREGS. On PPC we support setting > the PVR (CPU type) via SREGS because ONE_REG only came later. Setting > the CPU type only makes sense before you first start using a vcpu. After that > point it should be forbidden. > > So in a way, the irqchip memory address location is a device register, as it > has > all the properties that a register on that device would have. It is > > * per device > * one id for one purpose > * may or may not be writable during certain times There's a distinction between "things you need to do in machine setup" and "things you need to propagate for migration". It should be possible to do a migration by doing a complete copy of all ONE_REG (and DEV_REG if we have it) state from source to destination, so it needs to be always possible to write them. > We would also keep the gates open to get a new register id one day for a > second > address. Imagine an interrupt controller that splits its address mappings into > "PIC base registers" and "MSI target registers", though the MSI registers > logically > belong to the PIC. Then the ioctl as designed here gets stuck too. This is exactly what ARM's GIC has already. We have several memory regions (one for the distributor, one for the cpu interface), and map them with several calls to SET_DEVICE_ADDRESS. >> (b) we didn't need to tangle up and delay the KVM ARM work with a vague >> and unspecified desire for general configurability > > Yeah, that was the basic idea. Considering that the patch set hasn't been > going > in for another 2 months after that discussion indicates that this isn't too > much of > an issue though :). We might get there faster if people didn't keep nitpicking the APIs at the last minute :-) -- PMM -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html