Am 09.01.2013 um 00:49 schrieb Christoffer Dall <c.dall@xxxxxxxxxxxxxxxxxxxxxx>:
> On Tue, Jan 8, 2013 at 6:29 PM, Scott Wood <scottwood@xxxxxxxxxxxxx> wrote:
>> On 01/08/2013 05:17:05 PM, Christoffer Dall wrote:
>>>
>>> On Tue, Jan 8, 2013 at 5:36 PM, Scott Wood <scottwood@xxxxxxxxxxxxx>
>>> wrote:
>>>> On 01/08/2013 12:41:30 PM, Christoffer Dall wrote:
>>>>> +struct kvm_device_address {
>>>>> + __u64 id;
>>>>> + __u64 addr;
>>>>> +};
>>>>
>>>>
>>>> What about this is really specific to addresses? Can't we set other
>>>> device
>>>> parameters this way?
>>>>
>>>> Sort of like a device equivalent of PPC's one-reg interface.
>>>>
>>> This has been discussed a number of times,
>>
>>
>> Sorry, this patch was just pointed out to me today. I googled the patch
>> title but couldn't find this discussion.
>>
>>
>
> I believe it was mainly discussed at the KVM Forum in person.
>
>>> and one or the other there
>>> is a need for userspace to tell KVM to present memory-mapped devices
>>> at a given address. It was also considered to make this specific to
>>> irqchip initialization, but irqchips are different and a lot of that
>>> code is x86-specific, so that approach was discarded.
>>>
>>> This *could* look something like this:
>>>
>>> struct kvm_device_param {
>>> u64 dev_id;
>>> u64 param_id;
>>> u64 value;
>>> };
>>>
>>> but that has less clear, or at least less specific, semantics.
>>
>>
>> Why is it less clear? You need to have device-specific documentation for
>> what "id" means, so why not also an enumeration of "param"s? Or just keep
>> it as is, and rename "address" to "value". Whether "dev" and "param" are
>> combined is orthogonal from whether it's used for non-address things.
>
> less clear in the sense that you have to look at more code to see what
> it does. I'm not saying that it's too unclear, at all, I'm actually
> fine with it, but to make my point, we can make an ioctl that's called
> do_something() that takes a struct with val0, val1, val2, val3, ...
>
>>
>> If you leave it as "address", either we'll have it being used for
>> non-address things regardless of the name ("Not a typewriter!"), or there'll
>> end up being yet more unnecessary ioctls, or device-specific things will end
>> up getting shoved into CPU interfaces such as one-reg. For example, on MPIC
>> we need to be able to specify the version of the chip to emulate in addition
>> to the address at which it lives.
>>
>> Also, why is it documented as an "arm" interface? Shouldn't it be a generic
>> interface, with other architectures currently not implementing any IDs?
>> What in the kvm_arch_vm_ioctl() wrapper is arm-specific?
>>
> As I remember the argument for keeping this the point was that there
> were other preferred methods for other archs to do this, and that ARM
> was the only platform that had this explicit need, but maybe I'm
> making this up.
>
> I'll let Peter and Alex respond this as well, and if they're fine with
> changing it to what I proposed above, then let's do that, and we can
> make it a non arm-specific interface.
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.
That way we wouldn't block our path to create two in-kernel irqchips one day.
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
