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

[Jike Song <jike.song@intel.com>]

On 11/09/2016 10:00 PM, Xiao Guangrong wrote:
> On 11/09/2016 09:31 PM, Paolo Bonzini wrote:
>> 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.
> 
> Yes, indeed. Sorry for the typo. :(
> 
>>
>>>> +
>>>> +    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:
> 
> Yes, own mutex is definitely can work.:) It is vendor driver internal
> operation and it depends on the internal implementation.
> 
> My idea is that we can make sure the KVM instance is valid before calling
> DETACH callback. So if we can properly release all the resource associated
> with the kvm instance in this callback, it is okay without additional kvm
> ref.

Yes, the mutex in vfio_group is actually pointless, now I understand and agree.
Thanks Guangrong and Polao :)

--
Thanks,
Jike