Re: [RFC PATCH 1/6] kvm: add device control API

[Scott Wood <scottwood@freescale.com>]

On 03/05/2013 08:48:33 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2013-02-13 at 23:49 -0600, Scott Wood wrote:
> > Currently, devices that are emulated inside KVM are configured in a
> > hardcoded manner based on an assumption that any given architecture
> > only has one way to do it.  If there's any need to access device  
> state,
> > it is done through inflexible one-purpose-only IOCTLs (e.g.
> > KVM_GET/SET_LAPIC).  Defining new IOCTLs for every little thing is
> > cumbersome and depletes a limited numberspace.
> >
> > This API provides a mechanism to instantiate a device of a certain
> > type, returning an ID that can be used to set/get attributes of the
> > device.  Attributes may include configuration parameters (e.g.
> > register base address), device state, operational commands, etc.  It
> > is similar to the ONE_REG API, except that it acts on devices rather
> > than vcpus.
> 
> Allright guys, let's take a break for a minute :-)
> 
> What you seem to be proposing is a whole new construct / API to create
> "device" objects with "attributes" as a way to avoid adding tons of
> ioctls to the VM.
> 
> Then you somewhat "coerce" behaviours (ie. methods) as side effects of
> setting some of those attributes, and create some kind of rigid API
> through which any kind of potential device "attribute" needs to be
> coerced through.

It's not *that* rigid, and the rigidity that is there reduces what has  
to be spelled out for each individual thing.

It also helps with things like strace, once "size" is added.  If we go  
the private ioctl route, it would need to be updated for every new  
command (or more likely, it never gets updated and the strace output is  
less useful).

> Essentially you are trying to re-invent encapsulation of kernel  
> objects
> manipulated by userspace, we already have several mechanisms for doing
> just that and you are trying to add yet a new one :-)
> 
> What about instead using existing mechanisms for doing just that:
> 
> Make your "create device" return an anonymous file descriptor !

Well, technically the API documentation doesn't say that it *isn't* a  
file descriptor. :-)

I don't necessarily hate the idea, it just seemed like overkill.  And  
if I went that way probably someone else would complain that it was.

> That gives you everything ... private ioctl's which can do whatever  
> the
> heck you want (attributes, methods, etc...), mmap, etc...

Including all the downsides of ioctls, plus dealing with the reference  
counting and destruction order...

> Guess what ? That's already what we do for various things like our
> in-kernel emulated iommu tables as far as I can remember.

Which file is this in?

-Scott