Re: [v3 4/5] vfio: implement APIs to set/put kvm to/from vfio group

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 7 Nov 2016 19:10:37 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 07/11/2016 19:04, Alex Williamson wrote:
> >> > +struct kvm *vfio_group_get_kvm(struct vfio_group *group)
> >> > +{
> >> > +	struct kvm *kvm = NULL;  
> > Unnecessary initialization.
> >   
> >> > +
> >> > +	mutex_lock(&group->udata.lock);
> >> > +
> >> > +	kvm = group->udata.kvm;
> >> > +	if (kvm)
> >> > +		kvm_get_kvm(kvm);
> >> > +
> >> > +	mutex_unlock(&group->udata.lock);
> >> > +
> >> > +	return kvm;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(vfio_group_get_kvm);  
> >
> > How are kvm references acquired through vfio_group_get_kvm() ever
> > released?  
> 
> They are released with kvm_put_kvm, but it's done in the vendor driver
> so that VFIO core doesn't have a dependency on kvm.ko.

We could do a symbol_get() to avoid that so we could have a balanced
get/put through one interface.
 
> > Can the reference become invalid?  
> 
> No, this is guaranteed by virt/kvm/vfio.c + the udata.lock mutex (which
> probably should be renamed...).

The caller gets a reference to kvm, but there's no guarantee that the
association of that kvm reference to the group stays valid.  Once we're
outside of that mutex, we might as well consider that kvm:group
association stale.
 
> > The caller may still hold
> > a kvm references, but couldn't the group be detached from one kvm
> > instance and re-attached to another?  
> 
> Can this be handled by the vendor driver?  Does it get a callback when
> it's detached from a KVM instance?

The only release callback through vfio is when the user closes the
device, the code in this series is the full extent of vfio awareness of
kvm.  Thanks,

Alex