Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965498AbcKNX6s (ORCPT ); Mon, 14 Nov 2016 18:58:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36042 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752354AbcKNX6q (ORCPT ); Mon, 14 Nov 2016 18:58:46 -0500 Date: Mon, 14 Nov 2016 16:58:44 -0700 From: Alex Williamson To: Kirti Wankhede Cc: , , , , , , , , Subject: Re: [PATCH v12 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP Message-ID: <20161114165844.52537988@t450s.home> In-Reply-To: <1479138156-28905-12-git-send-email-kwankhede@nvidia.com> References: <1479138156-28905-1-git-send-email-kwankhede@nvidia.com> <1479138156-28905-12-git-send-email-kwankhede@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 14 Nov 2016 23:58:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8135 Lines: 257 On Mon, 14 Nov 2016 21:12:25 +0530 Kirti Wankhede wrote: > Added blocking notifier to IOMMU TYPE1 driver to notify vendor drivers > about DMA_UNMAP. > Exported two APIs vfio_register_notifier() and vfio_unregister_notifier(). > Notifier should be registered, if external user wants to use > vfio_pin_pages()/vfio_unpin_pages() APIs to pin/unpin pages. > Vendor driver should use VFIO_IOMMU_NOTIFY_DMA_UNMAP action to invalidate > mappings. > > Signed-off-by: Kirti Wankhede > Signed-off-by: Neo Jia > Change-Id: I5910d0024d6be87f3e8d3e0ca0eaeaaa0b17f271 > --- > drivers/vfio/vfio.c | 73 +++++++++++++++++++++++++++++++++++++++++ > drivers/vfio/vfio_iommu_type1.c | 60 ++++++++++++++++++++++++++------- > include/linux/vfio.h | 11 +++++++ > 3 files changed, 132 insertions(+), 12 deletions(-) > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c > index 7dcfbca2016a..388a3cbddbd9 100644 > --- a/drivers/vfio/vfio.c > +++ b/drivers/vfio/vfio.c > @@ -1902,6 +1902,79 @@ err_unpin_pages: > } > EXPORT_SYMBOL(vfio_unpin_pages); > > +int vfio_register_notifier(struct device *dev, struct notifier_block *nb) > +{ > + struct vfio_container *container; > + struct vfio_group *group; > + struct vfio_iommu_driver *driver; > + ssize_t 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_register_nb; > + > + container = group->container; > + down_read(&container->group_lock); > + > + driver = container->iommu_driver; > + if (likely(driver && driver->ops->register_notifier)) > + ret = driver->ops->register_notifier(container->iommu_data, nb); > + else > + ret = -ENOTTY; > + > + 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) > +{ > + struct vfio_container *container; > + struct vfio_group *group; > + struct vfio_iommu_driver *driver; > + ssize_t 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; > + > + container = group->container; > + down_read(&container->group_lock); > + > + driver = container->iommu_driver; > + if (likely(driver && driver->ops->unregister_notifier)) > + ret = driver->ops->unregister_notifier(container->iommu_data, > + nb); > + else > + ret = -ENOTTY; > + > + up_read(&container->group_lock); > + vfio_group_try_dissolve_container(group); > + > +err_unregister_nb: > + vfio_group_put(group); > + return ret; > +} > +EXPORT_SYMBOL(vfio_unregister_notifier); > + > /** > * Module/class support > */ > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 2697d874dd35..7e034e7c5ea5 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -38,6 +38,7 @@ > #include > #include > #include > +#include > > #define DRIVER_VERSION "0.2" > #define DRIVER_AUTHOR "Alex Williamson " > @@ -60,6 +61,7 @@ struct vfio_iommu { > struct vfio_domain *external_domain; /* domain for external user */ > struct mutex lock; > struct rb_root dma_list; > + struct blocking_notifier_head notifier; > bool v2; > bool nesting; > }; > @@ -568,7 +570,8 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > > mutex_lock(&iommu->lock); > > - if (!iommu->external_domain) { > + /* Fail if notifier list is empty */ > + if ((!iommu->external_domain) || (!iommu->notifier.head)) { > ret = -EINVAL; > goto pin_done; > } > @@ -854,15 +857,29 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu, > if (dma->task->mm != current->mm) > break; > unmapped += dma->size; > + > + mutex_unlock(&iommu->lock); > + if (iommu->external_domain && !RB_EMPTY_ROOT(&dma->pfn_list)) { > + struct vfio_iommu_type1_dma_unmap nb_unmap; > + > + nb_unmap.iova = dma->iova; > + nb_unmap.size = dma->size; > + blocking_notifier_call_chain(&iommu->notifier, > + VFIO_IOMMU_NOTIFY_DMA_UNMAP, > + &nb_unmap); > + > + if (WARN_ON(!RB_EMPTY_ROOT(&dma->pfn_list))) > + goto unmap_exit; > + } > + mutex_lock(&iommu->lock); Why are the mutexes pushed out so far, we are referencing way too much stuff outside of the mutex here. The notifier head has its own semaphore, so we should be able to squeeze the mutex opening to just around the notifier call, in which case we only worry about the iommu itself going way. > vfio_remove_dma(iommu, dma); > } > > unlock: > mutex_unlock(&iommu->lock); > - > +unmap_exit: > /* Report how much was unmapped */ > unmap->size = unmapped; > - Stray line deletions. > return ret; > } > > @@ -1430,6 +1447,7 @@ static void *vfio_iommu_type1_open(unsigned long arg) > INIT_LIST_HEAD(&iommu->domain_list); > iommu->dma_list = RB_ROOT; > mutex_init(&iommu->lock); > + BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier); > > return iommu; > } > @@ -1565,16 +1583,34 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, > return -ENOTTY; > } > > +static int vfio_iommu_type1_register_notifier(void *iommu_data, > + struct notifier_block *nb) > +{ > + struct vfio_iommu *iommu = iommu_data; > + > + return blocking_notifier_chain_register(&iommu->notifier, nb); > +} > + > +static int vfio_iommu_type1_unregister_notifier(void *iommu_data, > + struct notifier_block *nb) > +{ > + struct vfio_iommu *iommu = iommu_data; > + > + return blocking_notifier_chain_unregister(&iommu->notifier, nb); > +} > + > static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = { > - .name = "vfio-iommu-type1", > - .owner = THIS_MODULE, > - .open = vfio_iommu_type1_open, > - .release = vfio_iommu_type1_release, > - .ioctl = vfio_iommu_type1_ioctl, > - .attach_group = vfio_iommu_type1_attach_group, > - .detach_group = vfio_iommu_type1_detach_group, > - .pin_pages = vfio_iommu_type1_pin_pages, > - .unpin_pages = vfio_iommu_type1_unpin_pages, > + .name = "vfio-iommu-type1", > + .owner = THIS_MODULE, > + .open = vfio_iommu_type1_open, > + .release = vfio_iommu_type1_release, > + .ioctl = vfio_iommu_type1_ioctl, > + .attach_group = vfio_iommu_type1_attach_group, > + .detach_group = vfio_iommu_type1_detach_group, > + .pin_pages = vfio_iommu_type1_pin_pages, > + .unpin_pages = vfio_iommu_type1_unpin_pages, > + .register_notifier = vfio_iommu_type1_register_notifier, > + .unregister_notifier = vfio_iommu_type1_unregister_notifier, > }; > > static int __init vfio_iommu_type1_init(void) > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index 86f507d0f585..123e089388e9 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -80,6 +80,10 @@ struct vfio_iommu_driver_ops { > unsigned long *phys_pfn); > int (*unpin_pages)(void *iommu_data, > unsigned long *user_pfn, int npage); > + int (*register_notifier)(void *iommu_data, > + struct notifier_block *nb); > + int (*unregister_notifier)(void *iommu_data, > + struct notifier_block *nb); > }; > > extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops); > @@ -137,6 +141,13 @@ 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 > + > +extern int vfio_register_notifier(struct device *dev, > + struct notifier_block *nb); > + > +extern int vfio_unregister_notifier(struct device *dev, > + struct notifier_block *nb); > /* > * IRQfd - generic > */