On Wed, Jan 9, 2013 at 10:11 AM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote: > On 9 January 2013 14:58, Alexander Graf <agraf@xxxxxxx> wrote: >> >> On 09.01.2013, at 15:48, Peter Maydell wrote: >> >>> On 9 January 2013 10:02, Alexander Graf <agraf@xxxxxxx> 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 :). > well, now it's actually going in, and it seems like this is the last issue. Let's hold our breaths here for a second and let me write up a new device attribute API and send that out, and if you guys can possibly live with it, then please consider accepting it. -Christoffer -- 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
