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

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

On 11/09/2016 10:28 AM, Jike Song wrote:
> On 11/08/2016 02:28 AM, Alex Williamson wrote:
>> 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,
>
> Hi Alex,
>
> Thanks for the comments, I'm composing a notifier chain in vfio-group,
> hopefully that can address current concerns.
>
> However, as for the vfio awareness of kvm, implementing notifiers doesn't
> seem better for that?  Do you think if somehow, we are able to figure out
> a programmatic method in qemu, to trigger intel vGPU related quirks, would
> still be a better choice?

I do not think so,,, communicating VFIO with KVM should be generic as it may
have more users in the future except KVMGT.

I think notification is worth to try - vendor driver can register its
callbacks into vfio-group which get called when KVM binds/unbinds with VFIO