From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [v3 4/5] vfio: implement APIs to set/put kvm to/from vfio group Date: Wed, 9 Nov 2016 10:52:53 +0800 Message-ID: References: <1477895706-22824-1-git-send-email-jike.song@intel.com> <1477895706-22824-5-git-send-email-jike.song@intel.com> <20161107110412.5db26fd4@t450s.home> <20161107112834.2aa971df@t450s.home> <582289EA.9000101@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Paolo Bonzini , kwankhede@nvidia.com, cjia@nvidia.com, kevin.tian@intel.com, kvm@vger.kernel.org To: Jike Song , Alex Williamson Return-path: Received: from mga11.intel.com ([192.55.52.93]:55547 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753026AbcKIDA3 (ORCPT ); Tue, 8 Nov 2016 22:00:29 -0500 In-Reply-To: <582289EA.9000101@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 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