From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [v6 1/3] vfio: vfio_register_notifier: classify iommu notifier Date: Fri, 18 Nov 2016 10:55:53 -0700 Message-ID: <20161118105553.635ec83e@t450s.home> References: <1479466909-31765-1-git-send-email-jike.song@intel.com> <1479466909-31765-2-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]:57250 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753199AbcKRRzz (ORCPT ); Fri, 18 Nov 2016 12:55:55 -0500 In-Reply-To: <1479466909-31765-2-git-send-email-jike.song@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, 18 Nov 2016 19:01:47 +0800 Jike Song wrote: > Currently vfio_register_notifier assumes that there is only one > notifier chain, which is in vfio_iommu. However, the user might > also be interested in events other than vfio_iommu, for example, > vfio_group. Refactor vfio_{un}register_notifier implementation > to make it feasible. > > Cc: Alex Williamson > Cc: Kirti Wankhede > Cc: Paolo Bonzini > Cc: Xiao Guangrong > Signed-off-by: Jike Song > --- > drivers/vfio/vfio.c | 87 +++++++++++++++++++++++++++++++++++++++------------- > include/linux/vfio.h | 16 ++++++++-- > 2 files changed, 78 insertions(+), 25 deletions(-) > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c > index c815067..ec62bec 100644 > --- a/drivers/vfio/vfio.c > +++ b/drivers/vfio/vfio.c > @@ -2007,23 +2007,24 @@ int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, int npage) > } > EXPORT_SYMBOL(vfio_unpin_pages); > > -int vfio_register_notifier(struct device *dev, struct notifier_block *nb) > +static int vfio_register_iommu_notifier(struct vfio_group *group, > + unsigned long *events, > + struct notifier_block *nb) > { > struct vfio_container *container; > - struct vfio_group *group; > struct vfio_iommu_driver *driver; > int ret; > > - if (!dev || !nb) > - return -EINVAL; > + /* clear known events */ > + *events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP; > > - group = vfio_group_get_from_dev(dev); > - if (IS_ERR(group)) > - return PTR_ERR(group); > + /* refuse to register if still events remaining */ > + if (*events) > + return -EINVAL; > > ret = vfio_group_add_container_user(group); > if (ret) > - goto err_register_nb; > + return -EINVAL; > > container = group->container; > down_read(&container->group_lock); > @@ -2037,29 +2038,19 @@ int vfio_register_notifier(struct device *dev, struct notifier_block *nb) > up_read(&container->group_lock); > vfio_group_try_dissolve_container(group); > > -err_register_nb: > - vfio_group_put(group); > return ret; > } > -EXPORT_SYMBOL(vfio_register_notifier); > > -int vfio_unregister_notifier(struct device *dev, struct notifier_block *nb) > +static int vfio_unregister_iommu_notifier(struct vfio_group *group, > + struct notifier_block *nb) > { > struct vfio_container *container; > - struct vfio_group *group; > struct vfio_iommu_driver *driver; > int ret; > > - if (!dev || !nb) > - return -EINVAL; > - > - group = vfio_group_get_from_dev(dev); > - if (IS_ERR(group)) > - return PTR_ERR(group); > - > ret = vfio_group_add_container_user(group); > if (ret) > - goto err_unregister_nb; > + return -EINVAL; > > container = group->container; > down_read(&container->group_lock); > @@ -2074,7 +2065,59 @@ int vfio_unregister_notifier(struct device *dev, struct notifier_block *nb) > up_read(&container->group_lock); > vfio_group_try_dissolve_container(group); > > -err_unregister_nb: > + return ret; > +} > + > +int vfio_register_notifier(struct device *dev, vfio_notify_type_t type, > + unsigned long *events, > + struct notifier_block *nb) > +{ > + struct vfio_group *group; > + int ret; > + > + if (!dev || !nb) > + return -EINVAL; > + if (!events || *events == 0) > + return -EINVAL; nit, we could test all these within a single if() > + > + group = vfio_group_get_from_dev(dev); > + if (IS_ERR(group)) > + return PTR_ERR(group); > + > + switch (type) { > + case VFIO_IOMMU_NOTIFY: > + ret = vfio_register_iommu_notifier(group, events, nb); > + break; > + default: > + ret = EINVAL; -EINVAL ^ > + } > + > + vfio_group_put(group); > + return ret; > +} > +EXPORT_SYMBOL(vfio_register_notifier); > + > +int vfio_unregister_notifier(struct device *dev, vfio_notify_type_t type, > + struct notifier_block *nb) > +{ > + struct vfio_group *group; > + int ret; > + > + if (!dev || !nb) > + return -EINVAL; > + > + group = vfio_group_get_from_dev(dev); > + if (IS_ERR(group)) > + return PTR_ERR(group); > + > + switch (type) { > + case VFIO_IOMMU_NOTIFY: > + ret = vfio_unregister_iommu_notifier(group, nb); > + break; > + default: > + ret = -EINVAL; > + } > + > vfio_group_put(group); > return ret; > } > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index 15ff042..6f3ff31 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -107,14 +107,24 @@ extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn, > extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, > int npage); > > -#define VFIO_IOMMU_NOTIFY_DMA_UNMAP (1) > - > +typedef unsigned short vfio_notify_type_t; > extern int vfio_register_notifier(struct device *dev, > + vfio_notify_type_t type, > + unsigned long *required_events, > struct notifier_block *nb); > - > extern int vfio_unregister_notifier(struct device *dev, > + vfio_notify_type_t type, > struct notifier_block *nb); > > +/* each type has independent events */ > +enum vfio_notify_type { > + VFIO_IOMMU_NOTIFY = (__force vfio_notify_type_t)0, > +}; > + > +/* events for VFIO_IOMMU_NOTIFY */ > +#define VFIO_IOMMU_NOTIFY_DMA_UNMAP BIT(0) > + > + > /* > * Sub-module helpers > */