[PATCH v5 01/12] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl

[Peter Maydell]

On 9 January 2013 14:58, Alexander Graf <agraf at suse.de> wrote:
>
> On 09.01.2013, at 15:48, Peter Maydell wrote:
>
>> On 9 January 2013 10:02, Alexander Graf <agraf at 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