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

[Scott Wood <scottwood@freescale.com>]

  On 02/23/2013 09:04:33 AM, Marcelo Tosatti wrote:
> On Thu, Feb 21, 2013 at 08:00:25PM -0600, Scott Wood wrote:
> > On 02/21/2013 05:03:32 PM, Marcelo Tosatti wrote:
> > >You are not writing to the registers from the CPU point of view.
> >
> > That's exactly how KVM_DEV_MPIC_GRP_REGISTER is defined and
> > implemented on MPIC (with the exception of registers whose behavior
> > changes based on which specific vcpu you use to access them).
> > If/when we have a need to set/get state in a different manner,
> > that's a separate attribute group.
> 
> Can you describe usage of this register again?

It's used by QEMU to reflect memory space accesses to the kernel.  In  
practice, these are either debug accesses or MSIs.

> > >Also, it is necessary to provide proper locking of device attribute
> > >write versus vcpu device access. So far we have been focusing on
> > >having
> > >a lockless vcpu path.
> >
> > How is device access related to vcpus?  Existing irqchip code is not
> > lockless.
> 
> VCPUS access in-kernel devices.

Right, VCPUs (plural) not VCPU (singular).  Thus locking is needed.

> Yes, it is lockless (see RCU usage in virt/kvm/).

virt/kvm/kvm_main.c: /* kvm_io_bus_write - called under kvm->slots_lock  
*/
virt/kvm/ioapic.c: spin_lock(&ioapic->lock);
etc.

> > >Basically abstract 'device attributes' are too abstract.
> >
> > It's up to the device-specific documentation to make them not
> > abstract (I admit there are a few details missing in mpic.txt that
> > I've pointed out in this thread -- it is RFC v1 after all).  This
> > wouldn't be any different if we used separate ioctls for everything.
> > It's like saying abstract 'ioctl' is too abstract.
> 
> Perhaps a better way to put it is that its too permissive.

It's no more permissive than saying "go define an ioctl".  It is  
somewhat more constrained than that, in that it is an operation on a  
particular device, as opposed to any possible KVM action.

> > >However, your proposed interface deals with sucky capability,
> > >versioning
> > >and namespace conflicts we have now. Note these items can probably  
> be
> > >improved separately.
> >
> > Any particular proposals?
> 
> Namespace conflicts: Reserve ranges for each arch.

That only helps when the conflict is between different arches (as  
opposed to different devices), and it discourages sharing between  
arches.

-Scott