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

[Paolo Bonzini <pbonzini@redhat.com>]

On 09/11/2016 14:06, Xiao Guangrong wrote:
> 
> 
> On 11/09/2016 08:49 PM, Jike Song wrote:
> 
>> +void vfio_group_attach_kvm(struct vfio_group *group, struct kvm *kvm,
>> +        void (*fn)(struct kvm *))
>> +{
>> +    mutex_lock(&group->udata.lock);
> 
> This lock is needed, please see below.

*not* needed I guess.

>> +
>> +    fn(kvm);
>> +    blocking_notifier_call_chain(&group->udata.notifier,
>> +                VFIO_GROUP_NOTIFY_ATTACH_KVM, kvm);
> 
> As this is a callback before KVM releases its last refcount, i do not
> think vendor driver need to get additional KVM refcount.

The *group* driver doesn't need it indeed.  The mdev vendor driver
however does, so it will use kvm_get_kvm under its own mutex.  That is:

- attach kvm

	mutex_lock(mdev_driver->lock);
	mdev_driver->kvm = opaque;
	kvm_get_kvm(mdev_driver->kvm);
	mutex_unlock(mdev_driver->lock);

- detach kvm

	mutex_lock(mdev_driver->lock);
	kvm_put_kvm(mdev_driver->kvm);
	WARN_ON(mdev_driver->kvm != opaque);
	mdev_driver->kvm = NULL;
	mutex_unlock(mdev_driver->lock);

- use kvm

	mutex_lock(mdev_driver->lock);
	kvm = mdev_driver->kvm;
	...
	mutex_unlock(mdev_driver->lock);

or if safe:

	mutex_lock(mdev_driver->lock);
	kvm = mdev_driver->kvm;
	kvm_get_kvm(kvm);
	mutex_unlock(mdev_driver->lock);
	...
	kvm_put_kvm(kvm);

Thanks,

Paolo