From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jike Song Subject: Re: [v3 4/5] vfio: implement APIs to set/put kvm to/from vfio group Date: Wed, 09 Nov 2016 11:07:31 +0800 Message-ID: <582292F3.5000209@intel.com> 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 Content-Transfer-Encoding: 7bit Cc: Alex Williamson , Paolo Bonzini , kwankhede@nvidia.com, cjia@nvidia.com, kevin.tian@intel.com, kvm@vger.kernel.org To: Xiao Guangrong Return-path: Received: from mga11.intel.com ([192.55.52.93]:1383 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751282AbcKIDON (ORCPT ); Tue, 8 Nov 2016 22:14:13 -0500 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 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 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