On 01/08/2013 05:49:40 PM, Christoffer Dall wrote:
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:
>> 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, ...
Such an IOCTL would add nothing other than trading the limited and
cumbersome ioctl namespace for something structured a bit differently
(which isn't such a bad thing...). A set device attribute ioctl would
constrain it to "take this number and convey it to the enumerated
device for the enumerated configuration purpose". There's already room
for device-specific semantics since you can have multiple address types.
Regarding the dev/param split, it looks like you're doing the split
anyway -- might as well make them separate struct fields rather than an
architecture-specific bitfield encoding.
> 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.
Well, at least PPC has this explicit need as well. :-)
Only the toplevel mechanism would be generic; it would be up to each
device to decide which (if any) configuration parameters it wants to
expose through it.
-Scott
--
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
