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:52 AM, Xiao Guangrong wrote:
> 
> 
> 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
> 

Certainly it's worthy, I agree :-)

Just being anxious about how could the kvm awareness in vfio be avoided ...


--
Thanks,
Jike