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 _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm