From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [v3 4/5] vfio: implement APIs to set/put kvm to/from vfio group Date: Mon, 7 Nov 2016 11:04:12 -0700 Message-ID: <20161107110412.5db26fd4@t450s.home> References: <1477895706-22824-1-git-send-email-jike.song@intel.com> <1477895706-22824-5-git-send-email-jike.song@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: pbonzini@redhat.com, guangrong.xiao@linux.intel.com, kwankhede@nvidia.com, cjia@nvidia.com, kevin.tian@intel.com, kvm@vger.kernel.org To: Jike Song Return-path: Received: from mx1.redhat.com ([209.132.183.28]:54498 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932572AbcKGSFB (ORCPT ); Mon, 7 Nov 2016 13:05:01 -0500 In-Reply-To: <1477895706-22824-5-git-send-email-jike.song@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, 31 Oct 2016 14:35:05 +0800 Jike Song wrote: Patch title is "set/put" but there is no "put". > A vfio_group may be or may not be attached to a KVM instance, > if it is, the user of vfio_group might also want to know which > KVM instance it is attached to, to utilize features provided > by KVM. In VFIO there are already external APIs for KVM to > get/put the vfio_group, by extending that, KVM can set or clear > itself to/from the vfio_group, for external users to use. > > Signed-off-by: Jike Song > --- > drivers/vfio/vfio.c | 30 ++++++++++++++++++++++++++++++ > include/linux/vfio.h | 4 ++++ > 2 files changed, 34 insertions(+) > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c > index e3e58e3..41611cc 100644 > --- a/drivers/vfio/vfio.c > +++ b/drivers/vfio/vfio.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > > #define DRIVER_VERSION "0.3" > #define DRIVER_AUTHOR "Alex Williamson " > @@ -86,6 +87,10 @@ struct vfio_group { > struct mutex unbound_lock; > atomic_t opened; > bool noiommu; > + struct { > + struct kvm *kvm; > + struct mutex lock; > + } udata; > }; > > struct vfio_device { > @@ -333,6 +338,7 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group) > mutex_init(&group->device_lock); > INIT_LIST_HEAD(&group->unbound_list); > mutex_init(&group->unbound_lock); > + mutex_init(&group->udata.lock); > atomic_set(&group->container_users, 0); > atomic_set(&group->opened, 0); > group->iommu_group = iommu_group; > @@ -1739,6 +1745,30 @@ long vfio_external_check_extension(struct vfio_group *group, unsigned long arg) > } > EXPORT_SYMBOL_GPL(vfio_external_check_extension); > > +void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) > +{ > + mutex_lock(&group->udata.lock); > + group->udata.kvm = kvm; > + mutex_unlock(&group->udata.lock); > +} > +EXPORT_SYMBOL_GPL(vfio_group_set_kvm); > + > +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? Can the reference become invalid? The caller may still hold a kvm references, but couldn't the group be detached from one kvm instance and re-attached to another? This seems like an ad-hoc reference that doesn't impose any usage semantics on the caller or release mechanism. Thanks, Alex > /** > * Sub-module support > */ > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index ad9b857..3abd690 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -95,6 +95,10 @@ extern long vfio_external_check_extension(struct vfio_group *group, > extern struct vfio_group *vfio_group_get_from_dev(struct device *dev); > extern void vfio_group_put(struct vfio_group *group); > > +struct kvm; > +extern void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm); > +extern struct kvm *vfio_group_get_kvm(struct vfio_group *group); > + > /* > * Sub-module helpers > */