Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1515412 > unrolled thread
| Started by | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| First post | 2016-11-04 22:10 +0100 |
| Last post | 2016-11-07 07:50 +0100 |
| Articles | 20 on this page of 61 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH v11 00/22] Add Mediated device support Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:10 +0100
[PATCH v11 12/22] vfio: Add notifier callback to parent's ops structure of mdev Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 12/22] vfio: Add notifier callback to parent's ops structure of mdev Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 01:00 +0100
[PATCH v11 06/22] vfio iommu type1: Update arguments of vfio_lock_acct Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare() Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare() Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-08 11:00 +0100
Re: [PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare() Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 21:50 +0100
[PATCH v11 20/22] docs: Sysfs ABI for mediated device framework Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 16/22] vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare() Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 04/22] vfio: Common function to increment container_users Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 19/22] docs: Add Documentation for Mediated devices Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 13/22] vfio: Introduce common function to add capabilities Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-08 08:30 +0100
Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 21:50 +0100
Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 22:50 +0100
Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-09 03:30 +0100
[PATCH v11 03/22] vfio: Rearrange functions to get vfio_group from dev Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 14/22] vfio_pci: Update vfio_pci to use vfio_info_add_capability() Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 09/22] vfio iommu type1: Add task structure to vfio_dma Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 09/22] vfio iommu type1: Add task structure to vfio_dma Alex Williamson <alex.williamson@redhat.com> - 2016-11-07 22:10 +0100
Re: [PATCH v11 09/22] vfio iommu type1: Add task structure to vfio_dma Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 15:20 +0100
Re: [PATCH v11 09/22] vfio iommu type1: Add task structure to vfio_dma Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 17:50 +0100
[PATCH v11 02/22] vfio: VFIO based driver for Mediated devices Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 18/22] vfio: Define device_api strings Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 07/22] vfio iommu type1: Update argument of vaddr_get_pfn() Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 07/22] vfio iommu type1: Update argument of vaddr_get_pfn() Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-07 09:50 +0100
[PATCH v11 15/22] vfio: Introduce vfio_set_irqs_validate_and_prepare() Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 15/22] vfio: Introduce vfio_set_irqs_validate_and_prepare() Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-08 09:50 +0100
Re: [PATCH v11 15/22] vfio: Introduce vfio_set_irqs_validate_and_prepare() Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 21:30 +0100
Re: [PATCH v11 15/22] vfio: Introduce vfio_set_irqs_validate_and_prepare() Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-09 04:10 +0100
Re: [PATCH v11 15/22] vfio: Introduce vfio_set_irqs_validate_and_prepare() Alex Williamson <alex.williamson@redhat.com> - 2016-11-09 04:40 +0100
[PATCH v11 10/22] vfio iommu type1: Add support for mediated devices Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 10/22] vfio iommu type1: Add support for mediated devices Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 00:20 +0100
Re: [PATCH v11 10/22] vfio iommu type1: Add support for mediated devices Jike Song <jike.song@intel.com> - 2016-11-08 03:30 +0100
Re: [PATCH v11 10/22] vfio iommu type1: Add support for mediated devices Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 17:20 +0100
Re: [PATCH v11 10/22] vfio iommu type1: Add support for mediated devices Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 16:10 +0100
Re: [PATCH v11 10/22] vfio iommu type1: Add support for mediated devices Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 18:10 +0100
Re: [PATCH v11 10/22] vfio iommu type1: Add support for mediated devices Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-08 08:00 +0100
[PATCH v11 22/22] MAINTAINERS: Add entry VFIO based Mediated device drivers Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 08/22] vfio iommu type1: Add find_iommu_group() function Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
[PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 00:50 +0100
Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 17:30 +0100
Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 18:50 +0100
Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 21:10 +0100
Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 22:30 +0100
[PATCH v11 01/22] vfio: Mediated device Core driver Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
RE: [PATCH v11 01/22] vfio: Mediated device Core driver "Tian, Kevin" <kevin.tian@intel.com> - 2016-11-07 07:50 +0100
Re: [PATCH v11 01/22] vfio: Mediated device Core driver Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 22:20 +0100
[PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-04 22:20 +0100
Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops Alex Williamson <alex.williamson@redhat.com> - 2016-11-07 20:40 +0100
Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 15:00 +0100
Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 17:50 +0100
Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-08 20:00 +0100
Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops Alex Williamson <alex.williamson@redhat.com> - 2016-11-08 20:20 +0100
Re: [PATCH v11 00/22] Add Mediated device support Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-07 04:50 +0100
Re: [PATCH v11 00/22] Add Mediated device support Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-07 05:00 +0100
Re: [PATCH v11 00/22] Add Mediated device support Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-07 06:10 +0100
Re: [PATCH v11 00/22] Add Mediated device support Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-07 07:20 +0100
Re: [PATCH v11 00/22] Add Mediated device support Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-07 07:40 +0100
Re: [PATCH v11 00/22] Add Mediated device support Alexey Kardashevskiy <aik@ozlabs.ru> - 2016-11-07 07:50 +0100
Page 3 of 4 — ← Prev page 1 2 [3] 4 Next page →
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-04 22:20 +0100 |
| Subject | [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP |
| Message-ID | <szU42-71A-53@gated-at.bofh.it> |
| In reply to | #1515412 |
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 <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I5910d0024d6be87f3e8d3e0ca0eaeaaa0b17f271
---
drivers/vfio/vfio.c | 73 +++++++++++++++++++++++++++++++++++++++++
drivers/vfio/vfio_iommu_type1.c | 47 ++++++++++++++++++++------
include/linux/vfio.h | 11 +++++++
3 files changed, 121 insertions(+), 10 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 76d260e98930..4ed1a6a247c6 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1895,6 +1895,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 = -EINVAL;
+
+ 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 = -EINVAL;
+
+ 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 e511073446a0..c2d3a84c447b 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -37,6 +37,7 @@
#include <linux/vfio.h>
#include <linux/workqueue.h>
#include <linux/mdev.h>
+#include <linux/notifier.h>
#define DRIVER_VERSION "0.2"
#define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
@@ -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;
};
@@ -550,7 +552,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;
}
@@ -867,6 +870,11 @@ unlock:
/* Report how much was unmapped */
unmap->size = unmapped;
+ if (unmapped && iommu->external_domain)
+ blocking_notifier_call_chain(&iommu->notifier,
+ VFIO_IOMMU_NOTIFY_DMA_UNMAP,
+ unmap);
+
return ret;
}
@@ -1474,6 +1482,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
INIT_LIST_HEAD(&iommu->addr_space_list);
iommu->dma_list = RB_ROOT;
mutex_init(&iommu->lock);
+ BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier);
return iommu;
}
@@ -1610,16 +1619,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 ba1b64cb7d4b..dcda8fccefab 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -82,6 +82,10 @@ struct vfio_iommu_driver_ops {
unsigned long *user_pfn,
unsigned long *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);
@@ -139,6 +143,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,
unsigned long *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
*/
--
2.7.0
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2016-11-08 00:50 +0100 |
| Subject | Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP |
| Message-ID | <sB1PQ-1Bj-17@gated-at.bofh.it> |
| In reply to | #1515432 |
On Sat, 5 Nov 2016 02:40:45 +0530
Kirti Wankhede <kwankhede@nvidia.com> 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 <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: I5910d0024d6be87f3e8d3e0ca0eaeaaa0b17f271
> ---
> drivers/vfio/vfio.c | 73 +++++++++++++++++++++++++++++++++++++++++
> drivers/vfio/vfio_iommu_type1.c | 47 ++++++++++++++++++++------
> include/linux/vfio.h | 11 +++++++
> 3 files changed, 121 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 76d260e98930..4ed1a6a247c6 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1895,6 +1895,79 @@ err_unpin_pages:
> }
> EXPORT_SYMBOL(vfio_unpin_pages);
>
> +int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
Is the expectation here that this is a generic notifier for all
vfio->mdev signaling? That should probably be made clear in the mdev
API to avoid vendor drivers assuming their notifier callback only
occurs for unmaps, even if that's currently the case.
> +{
> + 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 = -EINVAL;
-ENOTTY again? And below.
> +
> + 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 = -EINVAL;
> +
> + up_read(&container->group_lock);
> + vfio_group_try_dissolve_container(group);
The concern any time we have an unregister like this is whether the
vendor driver does proper cleanup. Maybe we don't even need an
unregister, could we track this on the group such that releasing the
group automatically unregisters the notifier? Maybe a single nb
slot and -EBUSY if already set, cleared on release? Along those lines,
automatically unpinning anything would also be a nice feature (ie. if
an mdev device is unplugged while other devices are still in the
container), but then we'd need to track pinning per group and we already
have too much overhead in tracking pinning.
> +
> +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 e511073446a0..c2d3a84c447b 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -37,6 +37,7 @@
> #include <linux/vfio.h>
> #include <linux/workqueue.h>
> #include <linux/mdev.h>
> +#include <linux/notifier.h>
>
> #define DRIVER_VERSION "0.2"
> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
> @@ -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;
> };
> @@ -550,7 +552,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;
> }
> @@ -867,6 +870,11 @@ unlock:
> /* Report how much was unmapped */
> unmap->size = unmapped;
>
> + if (unmapped && iommu->external_domain)
> + blocking_notifier_call_chain(&iommu->notifier,
> + VFIO_IOMMU_NOTIFY_DMA_UNMAP,
> + unmap);
This is after the fact, there's already a gap here where pages are
unpinned and the mdev device is still running. The notifier needs to
happen prior to that and I suspect that we need to validate that we
have no remaining external pfn references within this vfio_dma block.
It seems like we need to root our pfn tracking in the vfio_dma so that
we can see that it's empty after the notifier chain and BUG_ON if not.
I would also add some enforcement that external pinning is only enabled
when vfio_iommu_type1 is configured for v2 semantics (ie. we only
support unmaps exactly matching previous maps).
> +
> return ret;
> }
>
> @@ -1474,6 +1482,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
> INIT_LIST_HEAD(&iommu->addr_space_list);
> iommu->dma_list = RB_ROOT;
> mutex_init(&iommu->lock);
> + BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier);
>
> return iommu;
> }
> @@ -1610,16 +1619,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 ba1b64cb7d4b..dcda8fccefab 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -82,6 +82,10 @@ struct vfio_iommu_driver_ops {
> unsigned long *user_pfn,
> unsigned long *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);
> @@ -139,6 +143,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,
> unsigned long *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
> */
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-08 17:30 +0100 |
| Subject | Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP |
| Message-ID | <sBhrA-3gP-17@gated-at.bofh.it> |
| In reply to | #1516681 |
On 11/8/2016 5:15 AM, Alex Williamson wrote:
> On Sat, 5 Nov 2016 02:40:45 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>
...
>>
>> +int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
>
> Is the expectation here that this is a generic notifier for all
> vfio->mdev signaling? That should probably be made clear in the mdev
> API to avoid vendor drivers assuming their notifier callback only
> occurs for unmaps, even if that's currently the case.
>
Ok. Adding comment about notifier callback in mdev_device which is part
of next patch.
...
>> 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;
>> }
>> @@ -867,6 +870,11 @@ unlock:
>> /* Report how much was unmapped */
>> unmap->size = unmapped;
>>
>> + if (unmapped && iommu->external_domain)
>> + blocking_notifier_call_chain(&iommu->notifier,
>> + VFIO_IOMMU_NOTIFY_DMA_UNMAP,
>> + unmap);
>
> This is after the fact, there's already a gap here where pages are
> unpinned and the mdev device is still running.
Oh, there is a bug here, now unpin_pages() take user_pfn as argument and
find vfio_dma. If its not found, it doesn't unpin pages. We have to call
this notifier before vfio_remove_dma(). But if we call this before
vfio_remove_dma() there will be deadlock since iommu->lock is already
held here and vfio_iommu_type1_unpin_pages() will also try to hold
iommu->lock.
If we want to call blocking_notifier_call_chain() before
vfio_remove_dma(), sequence should be:
unmapped += dma->size;
mutex_unlock(&iommu->lock);
if (iommu->external_domain)) {
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);
}
mutex_lock(&iommu->lock);
vfio_remove_dma(iommu, dma);
> The notifier needs to
> happen prior to that and I suspect that we need to validate that we
> have no remaining external pfn references within this vfio_dma block.
> It seems like we need to root our pfn tracking in the vfio_dma so that
> we can see that it's empty after the notifier chain and BUG_ON if not.
There is no way to find pfns from that iova range with current
implementation. We can have this validate if we go with linear array of
iova to track pfns.
> I would also add some enforcement that external pinning is only enabled
> when vfio_iommu_type1 is configured for v2 semantics (ie. we only
> support unmaps exactly matching previous maps).
>
Ok I'll add that check.
Thanks,
Kirti
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2016-11-08 18:50 +0100 |
| Subject | Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP |
| Message-ID | <sBiGZ-3WR-21@gated-at.bofh.it> |
| In reply to | #1517352 |
On Tue, 8 Nov 2016 21:56:29 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:
> On 11/8/2016 5:15 AM, Alex Williamson wrote:
> > On Sat, 5 Nov 2016 02:40:45 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >
> ...
> >>
> >> +int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
> >
> > Is the expectation here that this is a generic notifier for all
> > vfio->mdev signaling? That should probably be made clear in the mdev
> > API to avoid vendor drivers assuming their notifier callback only
> > occurs for unmaps, even if that's currently the case.
> >
>
> Ok. Adding comment about notifier callback in mdev_device which is part
> of next patch.
>
> ...
>
> >> 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;
> >> }
> >> @@ -867,6 +870,11 @@ unlock:
> >> /* Report how much was unmapped */
> >> unmap->size = unmapped;
> >>
> >> + if (unmapped && iommu->external_domain)
> >> + blocking_notifier_call_chain(&iommu->notifier,
> >> + VFIO_IOMMU_NOTIFY_DMA_UNMAP,
> >> + unmap);
> >
> > This is after the fact, there's already a gap here where pages are
> > unpinned and the mdev device is still running.
>
> Oh, there is a bug here, now unpin_pages() take user_pfn as argument and
> find vfio_dma. If its not found, it doesn't unpin pages. We have to call
> this notifier before vfio_remove_dma(). But if we call this before
> vfio_remove_dma() there will be deadlock since iommu->lock is already
> held here and vfio_iommu_type1_unpin_pages() will also try to hold
> iommu->lock.
> If we want to call blocking_notifier_call_chain() before
> vfio_remove_dma(), sequence should be:
>
> unmapped += dma->size;
> mutex_unlock(&iommu->lock);
> if (iommu->external_domain)) {
> 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);
> }
> mutex_lock(&iommu->lock);
> vfio_remove_dma(iommu, dma);
It seems like it would be worthwhile to have the rb-tree rooted in the
vfio-dma, then we only need to call the notifier if there are pages
pinned within that vfio-dma (ie. the rb-tree is not empty). We can
then release the lock call the notifier, re-acquire the lock, and
BUG_ON if the rb-tree still is not empty. We might get duplicate pfns
between separate vfio_dma structs, but as I mentioned in other replies,
that seems like an exception that we don't need to optimize for.
> > The notifier needs to
> > happen prior to that and I suspect that we need to validate that we
> > have no remaining external pfn references within this vfio_dma block.
> > It seems like we need to root our pfn tracking in the vfio_dma so that
> > we can see that it's empty after the notifier chain and BUG_ON if not.
>
> There is no way to find pfns from that iova range with current
> implementation. We can have this validate if we go with linear array of
> iova to track pfns.
Right, I was still hoping to avoid storing the pfn even with the
array/page-table approach though, ask the mm layer for the mapping
again. Is that too much overhead? Maybe the page table could store
the phys addr and we could use PAGE_MASK to store the reference count
so that each entry is still only 8bytes(?)
> > I would also add some enforcement that external pinning is only enabled
> > when vfio_iommu_type1 is configured for v2 semantics (ie. we only
> > support unmaps exactly matching previous maps).
> >
>
> Ok I'll add that check.
>
> Thanks,
> Kirti
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-08 21:10 +0100 |
| Subject | Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP |
| Message-ID | <sBkSt-5IO-7@gated-at.bofh.it> |
| In reply to | #1517430 |
On 11/8/2016 11:16 PM, Alex Williamson wrote:
> On Tue, 8 Nov 2016 21:56:29 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>
>> On 11/8/2016 5:15 AM, Alex Williamson wrote:
>>> On Sat, 5 Nov 2016 02:40:45 +0530
>>> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>>>
>> ...
>>>>
>>>> +int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
>>>
>>> Is the expectation here that this is a generic notifier for all
>>> vfio->mdev signaling? That should probably be made clear in the mdev
>>> API to avoid vendor drivers assuming their notifier callback only
>>> occurs for unmaps, even if that's currently the case.
>>>
>>
>> Ok. Adding comment about notifier callback in mdev_device which is part
>> of next patch.
>>
>> ...
>>
>>>> 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;
>>>> }
>>>> @@ -867,6 +870,11 @@ unlock:
>>>> /* Report how much was unmapped */
>>>> unmap->size = unmapped;
>>>>
>>>> + if (unmapped && iommu->external_domain)
>>>> + blocking_notifier_call_chain(&iommu->notifier,
>>>> + VFIO_IOMMU_NOTIFY_DMA_UNMAP,
>>>> + unmap);
>>>
>>> This is after the fact, there's already a gap here where pages are
>>> unpinned and the mdev device is still running.
>>
>> Oh, there is a bug here, now unpin_pages() take user_pfn as argument and
>> find vfio_dma. If its not found, it doesn't unpin pages. We have to call
>> this notifier before vfio_remove_dma(). But if we call this before
>> vfio_remove_dma() there will be deadlock since iommu->lock is already
>> held here and vfio_iommu_type1_unpin_pages() will also try to hold
>> iommu->lock.
>> If we want to call blocking_notifier_call_chain() before
>> vfio_remove_dma(), sequence should be:
>>
>> unmapped += dma->size;
>> mutex_unlock(&iommu->lock);
>> if (iommu->external_domain)) {
>> 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);
>> }
>> mutex_lock(&iommu->lock);
>> vfio_remove_dma(iommu, dma);
>
> It seems like it would be worthwhile to have the rb-tree rooted in the
> vfio-dma, then we only need to call the notifier if there are pages
> pinned within that vfio-dma (ie. the rb-tree is not empty). We can
> then release the lock call the notifier, re-acquire the lock, and
> BUG_ON if the rb-tree still is not empty. We might get duplicate pfns
> between separate vfio_dma structs, but as I mentioned in other replies,
> that seems like an exception that we don't need to optimize for.
>
If we don't optimize for the case where iova from different vfio_dma are
mapped to same pfn and we would not consider this case for page
accounting then:
- have rb tree of pinned iova, where key would be iova, in each vfio_dma
structure.
- iova tracking structure would have iova and ref_count only.
- page accounting would only count number of iova's in rb_tree, case
where different iova could map to same pfn would not be considered in
this implementation for now.
- vfio_unpin_pages() would have user_pfn and pfn as input, we would
validate that iova exist in rb tree and trust vendor driver that
corresponding pfn is correct, there is no validation of pfn. If want
validate pfn, call GUP, verify pfn and call put_pfn().
- In .release() or .detach_group() path, if there are entries in this rb
tree, call GUP again using that iova, get pfn and then call
put_pfn(pfn) for ref_count+1 times. This is because we are not keeping
pfn in our tracking logic.
Does this sound reasonable?
Thanks,
Kirti
>>> The notifier needs to
>>> happen prior to that and I suspect that we need to validate that we
>>> have no remaining external pfn references within this vfio_dma block.
>>> It seems like we need to root our pfn tracking in the vfio_dma so that
>>> we can see that it's empty after the notifier chain and BUG_ON if not.
>>
>> There is no way to find pfns from that iova range with current
>> implementation. We can have this validate if we go with linear array of
>> iova to track pfns.
>
> Right, I was still hoping to avoid storing the pfn even with the
> array/page-table approach though, ask the mm layer for the mapping
> again. Is that too much overhead? Maybe the page table could store
> the phys addr and we could use PAGE_MASK to store the reference count
> so that each entry is still only 8bytes(?)
>
>>> I would also add some enforcement that external pinning is only enabled
>>> when vfio_iommu_type1 is configured for v2 semantics (ie. we only
>>> support unmaps exactly matching previous maps).
>>>
>>
>> Ok I'll add that check.
>>
>> Thanks,
>> Kirti
>
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2016-11-08 22:30 +0100 |
| Subject | Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP |
| Message-ID | <sBm7U-6sU-15@gated-at.bofh.it> |
| In reply to | #1517538 |
On Wed, 9 Nov 2016 01:29:19 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:
> On 11/8/2016 11:16 PM, Alex Williamson wrote:
> > On Tue, 8 Nov 2016 21:56:29 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >
> >> On 11/8/2016 5:15 AM, Alex Williamson wrote:
> >>> On Sat, 5 Nov 2016 02:40:45 +0530
> >>> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >>>
> >> ...
> >>>>
> >>>> +int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
> >>>
> >>> Is the expectation here that this is a generic notifier for all
> >>> vfio->mdev signaling? That should probably be made clear in the mdev
> >>> API to avoid vendor drivers assuming their notifier callback only
> >>> occurs for unmaps, even if that's currently the case.
> >>>
> >>
> >> Ok. Adding comment about notifier callback in mdev_device which is part
> >> of next patch.
> >>
> >> ...
> >>
> >>>> 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;
> >>>> }
> >>>> @@ -867,6 +870,11 @@ unlock:
> >>>> /* Report how much was unmapped */
> >>>> unmap->size = unmapped;
> >>>>
> >>>> + if (unmapped && iommu->external_domain)
> >>>> + blocking_notifier_call_chain(&iommu->notifier,
> >>>> + VFIO_IOMMU_NOTIFY_DMA_UNMAP,
> >>>> + unmap);
> >>>
> >>> This is after the fact, there's already a gap here where pages are
> >>> unpinned and the mdev device is still running.
> >>
> >> Oh, there is a bug here, now unpin_pages() take user_pfn as argument and
> >> find vfio_dma. If its not found, it doesn't unpin pages. We have to call
> >> this notifier before vfio_remove_dma(). But if we call this before
> >> vfio_remove_dma() there will be deadlock since iommu->lock is already
> >> held here and vfio_iommu_type1_unpin_pages() will also try to hold
> >> iommu->lock.
> >> If we want to call blocking_notifier_call_chain() before
> >> vfio_remove_dma(), sequence should be:
> >>
> >> unmapped += dma->size;
> >> mutex_unlock(&iommu->lock);
> >> if (iommu->external_domain)) {
> >> 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);
> >> }
> >> mutex_lock(&iommu->lock);
> >> vfio_remove_dma(iommu, dma);
> >
> > It seems like it would be worthwhile to have the rb-tree rooted in the
> > vfio-dma, then we only need to call the notifier if there are pages
> > pinned within that vfio-dma (ie. the rb-tree is not empty). We can
> > then release the lock call the notifier, re-acquire the lock, and
> > BUG_ON if the rb-tree still is not empty. We might get duplicate pfns
> > between separate vfio_dma structs, but as I mentioned in other replies,
> > that seems like an exception that we don't need to optimize for.
> >
>
> If we don't optimize for the case where iova from different vfio_dma are
> mapped to same pfn and we would not consider this case for page
> accounting then:
Just to clarify, the current code (not handling mdevs) will pin and do
page accounting per iova, regardless of whether the iova translates to a
unique pfn. As long as we do no worse than that, I'm ok.
> - have rb tree of pinned iova, where key would be iova, in each vfio_dma
> structure.
> - iova tracking structure would have iova and ref_count only.
> - page accounting would only count number of iova's in rb_tree, case
> where different iova could map to same pfn would not be considered in
> this implementation for now.
> - vfio_unpin_pages() would have user_pfn and pfn as input, we would
> validate that iova exist in rb tree and trust vendor driver that
> corresponding pfn is correct, there is no validation of pfn. If want
> validate pfn, call GUP, verify pfn and call put_pfn().
> - In .release() or .detach_group() path, if there are entries in this rb
> tree, call GUP again using that iova, get pfn and then call
> put_pfn(pfn) for ref_count+1 times. This is because we are not keeping
> pfn in our tracking logic.
Wait a sec, if we detach a group from the container and it's not the
last group in the container (which would trigger a release), we can't
assume anything about which vfio_dma entries were associated with that
device. The vendor driver, through the release of the device(s) within
that group, needs to unpin. In a container release, we need to send a
notifier to the vendor driver(s) to cause an unpin. This is the only
mechanism we have to ensure that vendor drivers are not leaking
references. If during the release, after the notifier, if any
vfio_pfns remain, we need to BUG_ON, just like we need to do for any
other DMA_UNMAP.
Also, I'll say it again, I also don't like this API of passing around
potentially giant arrays, and especially the API of relying on the
vendor driver to tell us an arbitrary pfn to unpin. If we make the
assumption that vendor drivers do not pin lots and lots of memory,
perhaps we could use a struct vfio_pfn as:
struct vfio_pfn {
struct rb_node node;
dma_addr_t iova; /* key */
unsigned long pfn;
atomic_t ref_count;
};
This puts us at 44-bytes per pfn, which isn't great, but I think it
puts us in a better position with the API that we could make use of a
page-table or sparse array in the future that would eliminate the
rb_node and make the iova implicit in the location of the data
structure. That would leave only the pfn and ref_count, which could
potentially be combined into a single 8-byte field if we had per
vfio_dma (or higher) locking to avoid the atomic_t (and we're happy that
the reference count is always less than PAGE_SIZE, ie. we could fail
pinning if we get to that point).
That would allow for the unpin call to not provide the pfn, so could we
then look at whether we need the batching provided by the iova array at
all? I don't have a feel for the size of memory that gets pinned by
the vendor driver, the frequency of pinning, or whether usage of
hugepages for the guest is likely to translate into contiguous memory
requests through this API. What's your feeling? Thanks,
Alex
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-04 22:20 +0100 |
| Subject | [PATCH v11 01/22] vfio: Mediated device Core driver |
| Message-ID | <szU42-71A-49@gated-at.bofh.it> |
| In reply to | #1515412 |
Design for Mediated Device Driver:
Main purpose of this driver is to provide a common interface for mediated
device management that can be used by different drivers of different
devices.
This module provides a generic interface to create the device, add it to
mediated bus, add device to IOMMU group and then add it to vfio group.
Below is the high Level block diagram, with Nvidia, Intel and IBM devices
as example, since these are the devices which are going to actively use
this module as of now.
+---------------+
| |
| +-----------+ | mdev_register_driver() +--------------+
| | | +<------------------------+ __init() |
| | mdev | | | |
| | bus | +------------------------>+ |<-> VFIO user
| | driver | | probe()/remove() | vfio_mdev.ko | APIs
| | | | | |
| +-----------+ | +--------------+
| |
| MDEV CORE |
| MODULE |
| mdev.ko |
| +-----------+ | mdev_register_device() +--------------+
| | | +<------------------------+ |
| | | | | nvidia.ko |<-> physical
| | | +------------------------>+ | device
| | | | callback +--------------+
| | Physical | |
| | device | | mdev_register_device() +--------------+
| | interface | |<------------------------+ |
| | | | | i915.ko |<-> physical
| | | +------------------------>+ | device
| | | | callback +--------------+
| | | |
| | | | mdev_register_device() +--------------+
| | | +<------------------------+ |
| | | | | ccw_device.ko|<-> physical
| | | +------------------------>+ | device
| | | | callback +--------------+
| +-----------+ |
+---------------+
Core driver provides two types of registration interfaces:
1. Registration interface for mediated bus driver:
/**
* struct mdev_driver - Mediated device's driver
* @name: driver name
* @probe: called when new device created
* @remove:called when device removed
* @driver:device driver structure
*
**/
struct mdev_driver {
const char *name;
int (*probe) (struct device *dev);
void (*remove) (struct device *dev);
struct device_driver driver;
};
Mediated bus driver for mdev device should use this interface to register
and unregister with core driver respectively:
int mdev_register_driver(struct mdev_driver *drv, struct module *owner);
void mdev_unregister_driver(struct mdev_driver *drv);
Mediated bus driver is responsible to add/delete mediated devices to/from
VFIO group when devices are bound and unbound to the driver.
2. Physical device driver interface
This interface provides vendor driver the set APIs to manage physical
device related work in its driver. APIs are :
* dev_attr_groups: attributes of the parent device.
* mdev_attr_groups: attributes of the mediated device.
* supported_type_groups: attributes to define supported type. This is
mandatory field.
* create: to allocate basic resources in vendor driver for a mediated
device. This is mandatory to be provided by vendor driver.
* remove: to free resources in vendor driver when mediated device is
destroyed. This is mandatory to be provided by vendor driver.
* open: open callback of mediated device
* release: release callback of mediated device
* read : read emulation callback.
* write: write emulation callback.
* ioctl: ioctl callback.
* mmap: mmap emulation callback.
Drivers should use these interfaces to register and unregister device to
mdev core driver respectively:
extern int mdev_register_device(struct device *dev,
const struct parent_ops *ops);
extern void mdev_unregister_device(struct device *dev);
There are no locks to serialize above callbacks in mdev driver and
vfio_mdev driver. If required, vendor driver can have locks to serialize
above APIs in their driver.
Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I73a5084574270b14541c529461ea2f03c292d510
---
drivers/vfio/Kconfig | 1 +
drivers/vfio/Makefile | 1 +
drivers/vfio/mdev/Kconfig | 10 +
drivers/vfio/mdev/Makefile | 4 +
drivers/vfio/mdev/mdev_core.c | 388 +++++++++++++++++++++++++++++++++++++++
drivers/vfio/mdev/mdev_driver.c | 122 ++++++++++++
drivers/vfio/mdev/mdev_private.h | 41 +++++
drivers/vfio/mdev/mdev_sysfs.c | 286 +++++++++++++++++++++++++++++
include/linux/mdev.h | 167 +++++++++++++++++
9 files changed, 1020 insertions(+)
create mode 100644 drivers/vfio/mdev/Kconfig
create mode 100644 drivers/vfio/mdev/Makefile
create mode 100644 drivers/vfio/mdev/mdev_core.c
create mode 100644 drivers/vfio/mdev/mdev_driver.c
create mode 100644 drivers/vfio/mdev/mdev_private.h
create mode 100644 drivers/vfio/mdev/mdev_sysfs.c
create mode 100644 include/linux/mdev.h
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index da6e2ce77495..23eced02aaf6 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -48,4 +48,5 @@ menuconfig VFIO_NOIOMMU
source "drivers/vfio/pci/Kconfig"
source "drivers/vfio/platform/Kconfig"
+source "drivers/vfio/mdev/Kconfig"
source "virt/lib/Kconfig"
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 7b8a31f63fea..4a23c13b6be4 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
obj-$(CONFIG_VFIO_PCI) += pci/
obj-$(CONFIG_VFIO_PLATFORM) += platform/
+obj-$(CONFIG_VFIO_MDEV) += mdev/
diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
new file mode 100644
index 000000000000..303c14ce2847
--- /dev/null
+++ b/drivers/vfio/mdev/Kconfig
@@ -0,0 +1,10 @@
+
+config VFIO_MDEV
+ tristate "Mediated device driver framework"
+ depends on VFIO
+ default n
+ help
+ Provides a framework to virtualize devices.
+ See Documentation/vfio-mdev/vfio-mediated-device.txt for more details.
+
+ If you don't know what do here, say N.
diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
new file mode 100644
index 000000000000..31bc04801d94
--- /dev/null
+++ b/drivers/vfio/mdev/Makefile
@@ -0,0 +1,4 @@
+
+mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
+
+obj-$(CONFIG_VFIO_MDEV) += mdev.o
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
new file mode 100644
index 000000000000..54c59f325336
--- /dev/null
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -0,0 +1,388 @@
+/*
+ * Mediated device Core Driver
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ * Author: Neo Jia <cjia@nvidia.com>
+ * Kirti Wankhede <kwankhede@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/uuid.h>
+#include <linux/sysfs.h>
+#include <linux/mdev.h>
+
+#include "mdev_private.h"
+
+#define DRIVER_VERSION "0.1"
+#define DRIVER_AUTHOR "NVIDIA Corporation"
+#define DRIVER_DESC "Mediated device Core Driver"
+
+static LIST_HEAD(parent_list);
+static DEFINE_MUTEX(parent_list_lock);
+static struct class_compat *mdev_bus_compat_class;
+
+static int _find_mdev_device(struct device *dev, void *data)
+{
+ struct mdev_device *mdev;
+
+ if (!dev_is_mdev(dev))
+ return 0;
+
+ mdev = to_mdev_device(dev);
+
+ if (uuid_le_cmp(mdev->uuid, *(uuid_le *)data) == 0)
+ return 1;
+
+ return 0;
+}
+
+static bool mdev_device_exist(struct parent_device *parent, uuid_le uuid)
+{
+ struct device *dev;
+
+ dev = device_find_child(parent->dev, &uuid, _find_mdev_device);
+ if (dev) {
+ put_device(dev);
+ return true;
+ }
+
+ return false;
+}
+
+/* Should be called holding parent_list_lock */
+static struct parent_device *__find_parent_device(struct device *dev)
+{
+ struct parent_device *parent;
+
+ list_for_each_entry(parent, &parent_list, next) {
+ if (parent->dev == dev)
+ return parent;
+ }
+ return NULL;
+}
+
+static void mdev_release_parent(struct kref *kref)
+{
+ struct parent_device *parent = container_of(kref, struct parent_device,
+ ref);
+ struct device *dev = parent->dev;
+
+ kfree(parent);
+ put_device(dev);
+}
+
+static
+inline struct parent_device *mdev_get_parent(struct parent_device *parent)
+{
+ if (parent)
+ kref_get(&parent->ref);
+
+ return parent;
+}
+
+static inline void mdev_put_parent(struct parent_device *parent)
+{
+ if (parent)
+ kref_put(&parent->ref, mdev_release_parent);
+}
+
+static int mdev_device_create_ops(struct kobject *kobj,
+ struct mdev_device *mdev)
+{
+ struct parent_device *parent = mdev->parent;
+ int ret;
+
+ ret = parent->ops->create(kobj, mdev);
+ if (ret)
+ return ret;
+
+ ret = sysfs_create_groups(&mdev->dev.kobj,
+ parent->ops->mdev_attr_groups);
+ if (ret)
+ parent->ops->remove(mdev);
+
+ return ret;
+}
+
+/*
+ * mdev_device_remove_ops gets called from sysfs's 'remove' and when parent
+ * device is being unregistered from mdev device framework.
+ * - 'force_remove' is set to 'false' when called from sysfs's 'remove' which
+ * indicates that if the mdev device is active, used by VMM or userspace
+ * application, vendor driver could return error then don't remove the device.
+ * - 'force_remove' is set to 'true' when called from mdev_unregister_device()
+ * which indicate that parent device is being removed from mdev device
+ * framework so remove mdev device forcefully.
+ */
+static int mdev_device_remove_ops(struct mdev_device *mdev, bool force_remove)
+{
+ struct parent_device *parent = mdev->parent;
+ int ret;
+
+ /*
+ * Vendor driver can return error if VMM or userspace application is
+ * using this mdev device.
+ */
+ ret = parent->ops->remove(mdev);
+ if (ret && !force_remove)
+ return -EBUSY;
+
+ sysfs_remove_groups(&mdev->dev.kobj, parent->ops->mdev_attr_groups);
+ return 0;
+}
+
+static int mdev_device_remove_cb(struct device *dev, void *data)
+{
+ if (!dev_is_mdev(dev))
+ return 0;
+
+ return mdev_device_remove(dev, data ? *(bool *)data : true);
+}
+
+/*
+ * mdev_register_device : Register a device
+ * @dev: device structure representing parent device.
+ * @ops: Parent device operation structure to be registered.
+ *
+ * Add device to list of registered parent devices.
+ * Returns a negative value on error, otherwise 0.
+ */
+int mdev_register_device(struct device *dev, const struct parent_ops *ops)
+{
+ int ret;
+ struct parent_device *parent;
+
+ /* check for mandatory ops */
+ if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
+ return -EINVAL;
+
+ dev = get_device(dev);
+ if (!dev)
+ return -EINVAL;
+
+ mutex_lock(&parent_list_lock);
+
+ /* Check for duplicate */
+ parent = __find_parent_device(dev);
+ if (parent) {
+ ret = -EEXIST;
+ goto add_dev_err;
+ }
+
+ parent = kzalloc(sizeof(*parent), GFP_KERNEL);
+ if (!parent) {
+ ret = -ENOMEM;
+ goto add_dev_err;
+ }
+
+ kref_init(&parent->ref);
+ mutex_init(&parent->lock);
+
+ parent->dev = dev;
+ parent->ops = ops;
+
+ if (!mdev_bus_compat_class) {
+ mdev_bus_compat_class = class_compat_register("mdev_bus");
+ if (!mdev_bus_compat_class) {
+ ret = -ENOMEM;
+ goto add_dev_err;
+ }
+ }
+
+ ret = parent_create_sysfs_files(parent);
+ if (ret)
+ goto add_dev_err;
+
+ ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
+ if (ret)
+ dev_warn(dev, "Failed to create compatibility class link\n");
+
+ list_add(&parent->next, &parent_list);
+ mutex_unlock(&parent_list_lock);
+
+ dev_info(dev, "MDEV: Registered\n");
+ return 0;
+
+add_dev_err:
+ mutex_unlock(&parent_list_lock);
+ if (parent)
+ mdev_put_parent(parent);
+ else
+ put_device(dev);
+ return ret;
+}
+EXPORT_SYMBOL(mdev_register_device);
+
+/*
+ * mdev_unregister_device : Unregister a parent device
+ * @dev: device structure representing parent device.
+ *
+ * Remove device from list of registered parent devices. Give a chance to free
+ * existing mediated devices for given device.
+ */
+
+void mdev_unregister_device(struct device *dev)
+{
+ struct parent_device *parent;
+ bool force_remove = true;
+
+ mutex_lock(&parent_list_lock);
+ parent = __find_parent_device(dev);
+
+ if (!parent) {
+ mutex_unlock(&parent_list_lock);
+ return;
+ }
+ dev_info(dev, "MDEV: Unregistering\n");
+
+ list_del(&parent->next);
+ class_compat_remove_link(mdev_bus_compat_class, dev, NULL);
+
+ device_for_each_child(dev, (void *)&force_remove,
+ mdev_device_remove_cb);
+
+ parent_remove_sysfs_files(parent);
+
+ mutex_unlock(&parent_list_lock);
+ mdev_put_parent(parent);
+}
+EXPORT_SYMBOL(mdev_unregister_device);
+
+static void mdev_device_release(struct device *dev)
+{
+ struct mdev_device *mdev = to_mdev_device(dev);
+
+ dev_dbg(&mdev->dev, "MDEV: destroying\n");
+ kfree(mdev);
+}
+
+int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid)
+{
+ int ret;
+ struct mdev_device *mdev;
+ struct parent_device *parent;
+ struct mdev_type *type = to_mdev_type(kobj);
+
+ parent = mdev_get_parent(type->parent);
+ if (!parent)
+ return -EINVAL;
+
+ mutex_lock(&parent->lock);
+
+ /* Check for duplicate */
+ if (mdev_device_exist(parent, uuid)) {
+ ret = -EEXIST;
+ goto create_err;
+ }
+
+ mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
+ if (!mdev) {
+ ret = -ENOMEM;
+ goto create_err;
+ }
+
+ memcpy(&mdev->uuid, &uuid, sizeof(uuid_le));
+ mdev->parent = parent;
+ kref_init(&mdev->ref);
+
+ mdev->dev.parent = dev;
+ mdev->dev.bus = &mdev_bus_type;
+ mdev->dev.release = mdev_device_release;
+ dev_set_name(&mdev->dev, "%pUl", uuid.b);
+
+ ret = device_register(&mdev->dev);
+ if (ret) {
+ put_device(&mdev->dev);
+ goto create_err;
+ }
+
+ ret = mdev_device_create_ops(kobj, mdev);
+ if (ret)
+ goto create_failed;
+
+ ret = mdev_create_sysfs_files(&mdev->dev, type);
+ if (ret) {
+ mdev_device_remove_ops(mdev, true);
+ goto create_failed;
+ }
+
+ mdev->type_kobj = kobj;
+ dev_dbg(&mdev->dev, "MDEV: created\n");
+
+ mutex_unlock(&parent->lock);
+ return ret;
+
+create_failed:
+ device_unregister(&mdev->dev);
+
+create_err:
+ mutex_unlock(&parent->lock);
+ mdev_put_parent(parent);
+ return ret;
+}
+
+int mdev_device_remove(struct device *dev, bool force_remove)
+{
+ struct mdev_device *mdev;
+ struct parent_device *parent;
+ struct mdev_type *type;
+ int ret;
+
+ mdev = to_mdev_device(dev);
+ type = to_mdev_type(mdev->type_kobj);
+ parent = mdev->parent;
+ mutex_lock(&parent->lock);
+
+ ret = mdev_device_remove_ops(mdev, force_remove);
+ if (ret) {
+ mutex_unlock(&parent->lock);
+ return ret;
+ }
+
+ mdev_remove_sysfs_files(dev, type);
+ device_unregister(dev);
+ mutex_unlock(&parent->lock);
+ mdev_put_parent(parent);
+ return ret;
+}
+
+static int __init mdev_init(void)
+{
+ int ret;
+
+ ret = mdev_bus_register();
+ if (ret) {
+ pr_err("Failed to register mdev bus\n");
+ return ret;
+ }
+
+ /*
+ * Attempt to load known vfio_mdev. This gives us a working environment
+ * without the user needing to explicitly load vfio_mdev driver.
+ */
+ request_module_nowait("vfio_mdev");
+
+ return ret;
+}
+
+static void __exit mdev_exit(void)
+{
+ if (mdev_bus_compat_class)
+ class_compat_unregister(mdev_bus_compat_class);
+
+ mdev_bus_unregister();
+}
+
+module_init(mdev_init)
+module_exit(mdev_exit)
+
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c
new file mode 100644
index 000000000000..0b3250044a80
--- /dev/null
+++ b/drivers/vfio/mdev/mdev_driver.c
@@ -0,0 +1,122 @@
+/*
+ * MDEV driver
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ * Author: Neo Jia <cjia@nvidia.com>
+ * Kirti Wankhede <kwankhede@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/iommu.h>
+#include <linux/mdev.h>
+
+#include "mdev_private.h"
+
+static int mdev_attach_iommu(struct mdev_device *mdev)
+{
+ int ret;
+ struct iommu_group *group;
+
+ group = iommu_group_alloc();
+ if (IS_ERR(group))
+ return PTR_ERR(group);
+
+ ret = iommu_group_add_device(group, &mdev->dev);
+ if (ret)
+ goto attach_fail;
+
+ dev_info(&mdev->dev, "MDEV: group_id = %d\n",
+ iommu_group_id(group));
+attach_fail:
+ iommu_group_put(group);
+ return ret;
+}
+
+static void mdev_detach_iommu(struct mdev_device *mdev)
+{
+ iommu_group_remove_device(&mdev->dev);
+ dev_info(&mdev->dev, "MDEV: detaching iommu\n");
+}
+
+static int mdev_probe(struct device *dev)
+{
+ struct mdev_driver *drv = to_mdev_driver(dev->driver);
+ struct mdev_device *mdev = to_mdev_device(dev);
+ int ret;
+
+ ret = mdev_attach_iommu(mdev);
+ if (ret)
+ return ret;
+
+ if (drv && drv->probe)
+ ret = drv->probe(dev);
+
+ if (ret)
+ mdev_detach_iommu(mdev);
+
+ return ret;
+}
+
+static int mdev_remove(struct device *dev)
+{
+ struct mdev_driver *drv = to_mdev_driver(dev->driver);
+ struct mdev_device *mdev = to_mdev_device(dev);
+
+ if (drv && drv->remove)
+ drv->remove(dev);
+
+ mdev_detach_iommu(mdev);
+
+ return 0;
+}
+
+struct bus_type mdev_bus_type = {
+ .name = "mdev",
+ .probe = mdev_probe,
+ .remove = mdev_remove,
+};
+EXPORT_SYMBOL_GPL(mdev_bus_type);
+
+/*
+ * mdev_register_driver - register a new MDEV driver
+ * @drv: the driver to register
+ * @owner: module owner of driver to be registered
+ *
+ * Returns a negative value on error, otherwise 0.
+ */
+int mdev_register_driver(struct mdev_driver *drv, struct module *owner)
+{
+ /* initialize common driver fields */
+ drv->driver.name = drv->name;
+ drv->driver.bus = &mdev_bus_type;
+ drv->driver.owner = owner;
+
+ /* register with core */
+ return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL(mdev_register_driver);
+
+/*
+ * mdev_unregister_driver - unregister MDEV driver
+ * @drv: the driver to unregister
+ *
+ */
+void mdev_unregister_driver(struct mdev_driver *drv)
+{
+ driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL(mdev_unregister_driver);
+
+int mdev_bus_register(void)
+{
+ return bus_register(&mdev_bus_type);
+}
+
+void mdev_bus_unregister(void)
+{
+ bus_unregister(&mdev_bus_type);
+}
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
new file mode 100644
index 000000000000..000c93fcfdbd
--- /dev/null
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -0,0 +1,41 @@
+/*
+ * Mediated device interal definitions
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ * Author: Neo Jia <cjia@nvidia.com>
+ * Kirti Wankhede <kwankhede@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef MDEV_PRIVATE_H
+#define MDEV_PRIVATE_H
+
+int mdev_bus_register(void);
+void mdev_bus_unregister(void);
+
+struct mdev_type {
+ struct kobject kobj;
+ struct kobject *devices_kobj;
+ struct parent_device *parent;
+ struct list_head next;
+ struct attribute_group *group;
+};
+
+#define to_mdev_type_attr(_attr) \
+ container_of(_attr, struct mdev_type_attribute, attr)
+#define to_mdev_type(_kobj) \
+ container_of(_kobj, struct mdev_type, kobj)
+
+int parent_create_sysfs_files(struct parent_device *parent);
+void parent_remove_sysfs_files(struct parent_device *parent);
+
+int mdev_create_sysfs_files(struct device *dev, struct mdev_type *type);
+void mdev_remove_sysfs_files(struct device *dev, struct mdev_type *type);
+
+int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid);
+int mdev_device_remove(struct device *dev, bool force_remove);
+
+#endif /* MDEV_PRIVATE_H */
diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
new file mode 100644
index 000000000000..d808a41e4f68
--- /dev/null
+++ b/drivers/vfio/mdev/mdev_sysfs.c
@@ -0,0 +1,286 @@
+/*
+ * File attributes for Mediated devices
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ * Author: Neo Jia <cjia@nvidia.com>
+ * Kirti Wankhede <kwankhede@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/sysfs.h>
+#include <linux/ctype.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/uuid.h>
+#include <linux/mdev.h>
+
+#include "mdev_private.h"
+
+/* Static functions */
+
+static ssize_t mdev_type_attr_show(struct kobject *kobj,
+ struct attribute *__attr, char *buf)
+{
+ struct mdev_type_attribute *attr = to_mdev_type_attr(__attr);
+ struct mdev_type *type = to_mdev_type(kobj);
+ ssize_t ret = -EIO;
+
+ if (attr->show)
+ ret = attr->show(kobj, type->parent->dev, buf);
+ return ret;
+}
+
+static ssize_t mdev_type_attr_store(struct kobject *kobj,
+ struct attribute *__attr,
+ const char *buf, size_t count)
+{
+ struct mdev_type_attribute *attr = to_mdev_type_attr(__attr);
+ struct mdev_type *type = to_mdev_type(kobj);
+ ssize_t ret = -EIO;
+
+ if (attr->store)
+ ret = attr->store(&type->kobj, type->parent->dev, buf, count);
+ return ret;
+}
+
+static const struct sysfs_ops mdev_type_sysfs_ops = {
+ .show = mdev_type_attr_show,
+ .store = mdev_type_attr_store,
+};
+
+static ssize_t create_store(struct kobject *kobj, struct device *dev,
+ const char *buf, size_t count)
+{
+ char *str;
+ uuid_le uuid;
+ int ret;
+
+ if ((count < UUID_STRING_LEN) || (count > UUID_STRING_LEN + 1))
+ return -EINVAL;
+
+ str = kstrndup(buf, count, GFP_KERNEL);
+ if (!str)
+ return -ENOMEM;
+
+ ret = uuid_le_to_bin(str, &uuid);
+ kfree(str);
+ if (ret)
+ return ret;
+
+ ret = mdev_device_create(kobj, dev, uuid);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+MDEV_TYPE_ATTR_WO(create);
+
+static void mdev_type_release(struct kobject *kobj)
+{
+ struct mdev_type *type = to_mdev_type(kobj);
+
+ pr_debug("Releasing group %s\n", kobj->name);
+ kfree(type);
+}
+
+static struct kobj_type mdev_type_ktype = {
+ .sysfs_ops = &mdev_type_sysfs_ops,
+ .release = mdev_type_release,
+};
+
+struct mdev_type *add_mdev_supported_type(struct parent_device *parent,
+ struct attribute_group *group)
+{
+ struct mdev_type *type;
+ int ret;
+
+ if (!group->name) {
+ pr_err("%s: Type name empty!\n", __func__);
+ return ERR_PTR(-EINVAL);
+ }
+
+ type = kzalloc(sizeof(*type), GFP_KERNEL);
+ if (!type)
+ return ERR_PTR(-ENOMEM);
+
+ type->kobj.kset = parent->mdev_types_kset;
+
+ ret = kobject_init_and_add(&type->kobj, &mdev_type_ktype, NULL,
+ "%s-%s", dev_driver_string(parent->dev),
+ group->name);
+ if (ret) {
+ kfree(type);
+ return ERR_PTR(ret);
+ }
+
+ ret = sysfs_create_file(&type->kobj, &mdev_type_attr_create.attr);
+ if (ret)
+ goto attr_create_failed;
+
+ type->devices_kobj = kobject_create_and_add("devices", &type->kobj);
+ if (!type->devices_kobj) {
+ ret = -ENOMEM;
+ goto attr_devices_failed;
+ }
+
+ ret = sysfs_create_files(&type->kobj,
+ (const struct attribute **)group->attrs);
+ if (ret) {
+ ret = -ENOMEM;
+ goto attrs_failed;
+ }
+
+ type->group = group;
+ type->parent = parent;
+ return type;
+
+attrs_failed:
+ kobject_put(type->devices_kobj);
+attr_devices_failed:
+ sysfs_remove_file(&type->kobj, &mdev_type_attr_create.attr);
+attr_create_failed:
+ kobject_del(&type->kobj);
+ kobject_put(&type->kobj);
+ return ERR_PTR(ret);
+}
+
+static void remove_mdev_supported_type(struct mdev_type *type)
+{
+ sysfs_remove_files(&type->kobj,
+ (const struct attribute **)type->group->attrs);
+ kobject_put(type->devices_kobj);
+ sysfs_remove_file(&type->kobj, &mdev_type_attr_create.attr);
+ kobject_del(&type->kobj);
+ kobject_put(&type->kobj);
+}
+
+static int add_mdev_supported_type_groups(struct parent_device *parent)
+{
+ int i;
+
+ for (i = 0; parent->ops->supported_type_groups[i]; i++) {
+ struct mdev_type *type;
+
+ type = add_mdev_supported_type(parent,
+ parent->ops->supported_type_groups[i]);
+ if (IS_ERR(type)) {
+ struct mdev_type *ltype, *tmp;
+
+ list_for_each_entry_safe(ltype, tmp, &parent->type_list,
+ next) {
+ list_del(<ype->next);
+ remove_mdev_supported_type(ltype);
+ }
+ return PTR_ERR(type);
+ }
+ list_add(&type->next, &parent->type_list);
+ }
+ return 0;
+}
+
+/* mdev sysfs Functions */
+void parent_remove_sysfs_files(struct parent_device *parent)
+{
+ struct mdev_type *type, *tmp;
+
+ list_for_each_entry_safe(type, tmp, &parent->type_list, next) {
+ list_del(&type->next);
+ remove_mdev_supported_type(type);
+ }
+
+ sysfs_remove_groups(&parent->dev->kobj, parent->ops->dev_attr_groups);
+ kset_unregister(parent->mdev_types_kset);
+}
+
+int parent_create_sysfs_files(struct parent_device *parent)
+{
+ int ret;
+
+ parent->mdev_types_kset = kset_create_and_add("mdev_supported_types",
+ NULL, &parent->dev->kobj);
+
+ if (!parent->mdev_types_kset)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&parent->type_list);
+
+ ret = sysfs_create_groups(&parent->dev->kobj,
+ parent->ops->dev_attr_groups);
+ if (ret)
+ goto create_err;
+
+ ret = add_mdev_supported_type_groups(parent);
+ if (ret)
+ sysfs_remove_groups(&parent->dev->kobj,
+ parent->ops->dev_attr_groups);
+ else
+ return ret;
+
+create_err:
+ kset_unregister(parent->mdev_types_kset);
+ return ret;
+}
+
+static ssize_t remove_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ unsigned long val;
+
+ if (kstrtoul(buf, 0, &val) < 0)
+ return -EINVAL;
+
+ if (val && device_remove_file_self(dev, attr)) {
+ int ret;
+
+ ret = mdev_device_remove(dev, false);
+ if (ret) {
+ device_create_file(dev, attr);
+ return ret;
+ }
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_WO(remove);
+
+static const struct attribute *mdev_device_attrs[] = {
+ &dev_attr_remove.attr,
+ NULL,
+};
+
+int mdev_create_sysfs_files(struct device *dev, struct mdev_type *type)
+{
+ int ret;
+
+ ret = sysfs_create_files(&dev->kobj, mdev_device_attrs);
+ if (ret)
+ return ret;
+
+ ret = sysfs_create_link(type->devices_kobj, &dev->kobj, dev_name(dev));
+ if (ret)
+ goto device_link_failed;
+
+ ret = sysfs_create_link(&dev->kobj, &type->kobj, "mdev_type");
+ if (ret)
+ goto type_link_failed;
+
+ return ret;
+
+type_link_failed:
+ sysfs_remove_link(type->devices_kobj, dev_name(dev));
+device_link_failed:
+ sysfs_remove_files(&dev->kobj, mdev_device_attrs);
+ return ret;
+}
+
+void mdev_remove_sysfs_files(struct device *dev, struct mdev_type *type)
+{
+ sysfs_remove_link(&dev->kobj, "mdev_type");
+ sysfs_remove_link(type->devices_kobj, dev_name(dev));
+ sysfs_remove_files(&dev->kobj, mdev_device_attrs);
+}
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
new file mode 100644
index 000000000000..0352febc1944
--- /dev/null
+++ b/include/linux/mdev.h
@@ -0,0 +1,167 @@
+/*
+ * Mediated device definition
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ * Author: Neo Jia <cjia@nvidia.com>
+ * Kirti Wankhede <kwankhede@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef MDEV_H
+#define MDEV_H
+
+/* Parent Device */
+struct parent_device {
+ struct device *dev;
+ const struct parent_ops *ops;
+
+ /* internal */
+ struct kref ref;
+ struct mutex lock;
+ struct list_head next;
+ struct kset *mdev_types_kset;
+ struct list_head type_list;
+};
+
+/* Mediated device */
+struct mdev_device {
+ struct device dev;
+ struct parent_device *parent;
+ uuid_le uuid;
+ void *driver_data;
+
+ /* internal */
+ struct kref ref;
+ struct list_head next;
+ struct kobject *type_kobj;
+};
+
+
+/**
+ * struct parent_ops - Structure to be registered for each parent device to
+ * register the device to mdev module.
+ *
+ * @owner: The module owner.
+ * @dev_attr_groups: Attributes of the parent device.
+ * @mdev_attr_groups: Attributes of the mediated device.
+ * @supported_type_groups: Attributes to define supported types. It is mandatory
+ * to provide supported types.
+ * @create: Called to allocate basic resources in parent device's
+ * driver for a particular mediated device. It is
+ * mandatory to provide create ops.
+ * @kobj: kobject of type for which 'create' is called.
+ * @mdev: mdev_device structure on of mediated device
+ * that is being created
+ * Returns integer: success (0) or error (< 0)
+ * @remove: Called to free resources in parent device's driver for a
+ * a mediated device. It is mandatory to provide 'remove'
+ * ops.
+ * @mdev: mdev_device device structure which is being
+ * destroyed
+ * Returns integer: success (0) or error (< 0)
+ * @open: Open mediated device.
+ * @mdev: mediated device.
+ * Returns integer: success (0) or error (< 0)
+ * @release: release mediated device
+ * @mdev: mediated device.
+ * @read: Read emulation callback
+ * @mdev: mediated device structure
+ * @buf: read buffer
+ * @count: number of bytes to read
+ * @ppos: address.
+ * Retuns number on bytes read on success or error.
+ * @write: Write emulation callback
+ * @mdev: mediated device structure
+ * @buf: write buffer
+ * @count: number of bytes to be written
+ * @ppos: address.
+ * Retuns number on bytes written on success or error.
+ * @ioctl: IOCTL callback
+ * @mdev: mediated device structure
+ * @cmd: mediated device structure
+ * @arg: mediated device structure
+ * @mmap: mmap callback
+ * Parent device that support mediated device should be registered with mdev
+ * module with parent_ops structure.
+ **/
+
+struct parent_ops {
+ struct module *owner;
+ const struct attribute_group **dev_attr_groups;
+ const struct attribute_group **mdev_attr_groups;
+ struct attribute_group **supported_type_groups;
+
+ int (*create)(struct kobject *kobj, struct mdev_device *mdev);
+ int (*remove)(struct mdev_device *mdev);
+ int (*open)(struct mdev_device *mdev);
+ void (*release)(struct mdev_device *mdev);
+ ssize_t (*read)(struct mdev_device *mdev, char __user *buf,
+ size_t count, loff_t *ppos);
+ ssize_t (*write)(struct mdev_device *mdev, const char __user *buf,
+ size_t count, loff_t *ppos);
+ ssize_t (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
+ unsigned long arg);
+ int (*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
+};
+
+/* interface for exporting mdev supported type attributes */
+struct mdev_type_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct kobject *kobj, struct device *dev, char *buf);
+ ssize_t (*store)(struct kobject *kobj, struct device *dev,
+ const char *buf, size_t count);
+};
+
+#define MDEV_TYPE_ATTR(_name, _mode, _show, _store) \
+struct mdev_type_attribute mdev_type_attr_##_name = \
+ __ATTR(_name, _mode, _show, _store)
+#define MDEV_TYPE_ATTR_RW(_name) \
+ struct mdev_type_attribute mdev_type_attr_##_name = __ATTR_RW(_name)
+#define MDEV_TYPE_ATTR_RO(_name) \
+ struct mdev_type_attribute mdev_type_attr_##_name = __ATTR_RO(_name)
+#define MDEV_TYPE_ATTR_WO(_name) \
+ struct mdev_type_attribute mdev_type_attr_##_name = __ATTR_WO(_name)
+
+/**
+ * struct mdev_driver - Mediated device driver
+ * @name: driver name
+ * @probe: called when new device created
+ * @remove: called when device removed
+ * @driver: device driver structure
+ *
+ **/
+struct mdev_driver {
+ const char *name;
+ int (*probe)(struct device *dev);
+ void (*remove)(struct device *dev);
+ struct device_driver driver;
+};
+
+#define to_mdev_driver(drv) container_of(drv, struct mdev_driver, driver)
+#define to_mdev_device(dev) container_of(dev, struct mdev_device, dev)
+
+static inline void *mdev_get_drvdata(struct mdev_device *mdev)
+{
+ return mdev->driver_data;
+}
+
+static inline void mdev_set_drvdata(struct mdev_device *mdev, void *data)
+{
+ mdev->driver_data = data;
+}
+
+extern struct bus_type mdev_bus_type;
+
+#define dev_is_mdev(d) ((d)->bus == &mdev_bus_type)
+
+extern int mdev_register_device(struct device *dev,
+ const struct parent_ops *ops);
+extern void mdev_unregister_device(struct device *dev);
+
+extern int mdev_register_driver(struct mdev_driver *drv, struct module *owner);
+extern void mdev_unregister_driver(struct mdev_driver *drv);
+
+#endif /* MDEV_H */
--
2.7.0
[toc] | [prev] | [next] | [standalone]
| From | "Tian, Kevin" <kevin.tian@intel.com> |
|---|---|
| Date | 2016-11-07 07:50 +0100 |
| Subject | RE: [PATCH v11 01/22] vfio: Mediated device Core driver |
| Message-ID | <sALUK-7ND-11@gated-at.bofh.it> |
| In reply to | #1515433 |
> From: Kirti Wankhede [mailto:kwankhede@nvidia.com] > Sent: Saturday, November 05, 2016 5:11 AM > [...] > > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> > Signed-off-by: Neo Jia <cjia@nvidia.com> > Change-Id: I73a5084574270b14541c529461ea2f03c292d510 Jike has given his reviewed-by for some patches in v10. Please include his reviewed-by in new version, which represent acknowledgements from our side and also be informative to other reviewers. Thanks Kevin
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-08 22:20 +0100 |
| Subject | Re: [PATCH v11 01/22] vfio: Mediated device Core driver |
| Message-ID | <sBlYe-6oS-17@gated-at.bofh.it> |
| In reply to | #1515433 |
On 11/8/2016 2:55 PM, Dong Jia Shi wrote:
> * Kirti Wankhede <kwankhede@nvidia.com> [2016-11-05 02:40:35 +0530]:
>
> Hi Kirti,
>
> [...]
>> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
>> index da6e2ce77495..23eced02aaf6 100644
>> --- a/drivers/vfio/Kconfig
>> +++ b/drivers/vfio/Kconfig
>> @@ -48,4 +48,5 @@ menuconfig VFIO_NOIOMMU
>>
>> source "drivers/vfio/pci/Kconfig"
>> source "drivers/vfio/platform/Kconfig"
>> +source "drivers/vfio/mdev/Kconfig"
>> source "virt/lib/Kconfig"
>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>> index 7b8a31f63fea..4a23c13b6be4 100644
>> --- a/drivers/vfio/Makefile
>> +++ b/drivers/vfio/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
>> obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
>> obj-$(CONFIG_VFIO_PCI) += pci/
>> obj-$(CONFIG_VFIO_PLATFORM) += platform/
>> +obj-$(CONFIG_VFIO_MDEV) += mdev/
>> diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
>> new file mode 100644
>> index 000000000000..303c14ce2847
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/Kconfig
>> @@ -0,0 +1,10 @@
>> +
>> +config VFIO_MDEV
>> + tristate "Mediated device driver framework"
>> + depends on VFIO
>> + default n
>> + help
>> + Provides a framework to virtualize devices.
>> + See Documentation/vfio-mdev/vfio-mediated-device.txt for more details.
> We don't have this doc at this point of time.
>
Yes, but I have this doc in this patch series.
>> +
>> + If you don't know what do here, say N.
>> diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
>> new file mode 100644
>> index 000000000000..31bc04801d94
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/Makefile
>> @@ -0,0 +1,4 @@
>> +
>> +mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
>> +
>> +obj-$(CONFIG_VFIO_MDEV) += mdev.o
>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>> new file mode 100644
>> index 000000000000..54c59f325336
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/mdev_core.c
> [...]
>
>> +
>> +/*
>> + * mdev_device_remove_ops gets called from sysfs's 'remove' and when parent
>> + * device is being unregistered from mdev device framework.
>> + * - 'force_remove' is set to 'false' when called from sysfs's 'remove' which
>> + * indicates that if the mdev device is active, used by VMM or userspace
>> + * application, vendor driver could return error then don't remove the device.
>> + * - 'force_remove' is set to 'true' when called from mdev_unregister_device()
>> + * which indicate that parent device is being removed from mdev device
>> + * framework so remove mdev device forcefully.
>> + */
>> +static int mdev_device_remove_ops(struct mdev_device *mdev, bool force_remove)
> ?
> s/force_remove/force/
>
>> +{
>> + struct parent_device *parent = mdev->parent;
>> + int ret;
>> +
>> + /*
>> + * Vendor driver can return error if VMM or userspace application is
>> + * using this mdev device.
>> + */
>> + ret = parent->ops->remove(mdev);
>> + if (ret && !force_remove)
>> + return -EBUSY;
>> +
>> + sysfs_remove_groups(&mdev->dev.kobj, parent->ops->mdev_attr_groups);
>> + return 0;
>> +}
>> +
>> +static int mdev_device_remove_cb(struct device *dev, void *data)
>> +{
>> + if (!dev_is_mdev(dev))
>> + return 0;
>> +
>> + return mdev_device_remove(dev, data ? *(bool *)data : true);
>> +}
>> +
>> +/*
>> + * mdev_register_device : Register a device
>> + * @dev: device structure representing parent device.
>> + * @ops: Parent device operation structure to be registered.
>> + *
>> + * Add device to list of registered parent devices.
>> + * Returns a negative value on error, otherwise 0.
>> + */
>> +int mdev_register_device(struct device *dev, const struct parent_ops *ops)
>> +{
>> + int ret;
>> + struct parent_device *parent;
>> +
>> + /* check for mandatory ops */
>> + if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
>> + return -EINVAL;
>> +
>> + dev = get_device(dev);
>> + if (!dev)
>> + return -EINVAL;
>> +
>> + mutex_lock(&parent_list_lock);
>> +
>> + /* Check for duplicate */
>> + parent = __find_parent_device(dev);
>> + if (parent) {
>> + ret = -EEXIST;
>> + goto add_dev_err;
>> + }
>> +
>> + parent = kzalloc(sizeof(*parent), GFP_KERNEL);
>> + if (!parent) {
>> + ret = -ENOMEM;
>> + goto add_dev_err;
>> + }
>> +
>> + kref_init(&parent->ref);
>> + mutex_init(&parent->lock);
>> +
>> + parent->dev = dev;
>> + parent->ops = ops;
>> +
>> + if (!mdev_bus_compat_class) {
>> + mdev_bus_compat_class = class_compat_register("mdev_bus");
>> + if (!mdev_bus_compat_class) {
>> + ret = -ENOMEM;
>> + goto add_dev_err;
>> + }
>> + }
>> +
>> + ret = parent_create_sysfs_files(parent);
>> + if (ret)
>> + goto add_dev_err;
>> +
>> + ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
>> + if (ret)
>> + dev_warn(dev, "Failed to create compatibility class link\n");
>> +
>> + list_add(&parent->next, &parent_list);
>> + mutex_unlock(&parent_list_lock);
>> +
>> + dev_info(dev, "MDEV: Registered\n");
>> + return 0;
>> +
>> +add_dev_err:
>> + mutex_unlock(&parent_list_lock);
>> + if (parent)
>> + mdev_put_parent(parent);
> Why do this? I don't find the place that you call mdev_get_parent above.
>
kref_init(&parent->ref);
Above increments the ref_count, so mdev_put_parent() should be called if
anything fails.
>> + else
>> + put_device(dev);
> Shouldn't we always do this?
>
When mdev_put_parent() is called, its release function do this. So if
mdev_put_parent() is called, we don't need this.
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(mdev_register_device);
>> +
>> +/*
>> + * mdev_unregister_device : Unregister a parent device
>> + * @dev: device structure representing parent device.
>> + *
>> + * Remove device from list of registered parent devices. Give a chance to free
>> + * existing mediated devices for given device.
>> + */
>> +
>> +void mdev_unregister_device(struct device *dev)
>> +{
>> + struct parent_device *parent;
>> + bool force_remove = true;
>> +
>> + mutex_lock(&parent_list_lock);
>> + parent = __find_parent_device(dev);
>> +
>> + if (!parent) {
>> + mutex_unlock(&parent_list_lock);
>> + return;
>> + }
>> + dev_info(dev, "MDEV: Unregistering\n");
>> +
>> + list_del(&parent->next);
>> + class_compat_remove_link(mdev_bus_compat_class, dev, NULL);
>> +
>> + device_for_each_child(dev, (void *)&force_remove,
>> + mdev_device_remove_cb);
>> +
>> + parent_remove_sysfs_files(parent);
>> +
>> + mutex_unlock(&parent_list_lock);
>> + mdev_put_parent(parent);
> We also need to call put_device(dev) here since we have a get_device
> during registration, no?
> Or I must miss something...
>
As explained above.
>> +}
>> +EXPORT_SYMBOL(mdev_unregister_device);
>> +
> [...]
>
>> +static int __init mdev_init(void)
>> +{
>> + int ret;
>> +
>> + ret = mdev_bus_register();
>> + if (ret) {
>> + pr_err("Failed to register mdev bus\n");
>> + return ret;
>> + }
>> +
>> + /*
>> + * Attempt to load known vfio_mdev. This gives us a working environment
>> + * without the user needing to explicitly load vfio_mdev driver.
>> + */
>> + request_module_nowait("vfio_mdev");
> We don't have this module yet.
>
Yes, this module is added in 02/22 patch. I'll move only this call to
02/22 patch in my next update.
But ideally it should not fails anything if this vfio_mdev module is not
found.
>> +
>> + return ret;
>> +}
>> +
>> +static void __exit mdev_exit(void)
>> +{
>> + if (mdev_bus_compat_class)
>> + class_compat_unregister(mdev_bus_compat_class);
>> +
>> + mdev_bus_unregister();
>> +}
>> +
>> +module_init(mdev_init)
>> +module_exit(mdev_exit)
>> +
>> +MODULE_VERSION(DRIVER_VERSION);
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>> diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c
>> new file mode 100644
>> index 000000000000..0b3250044a80
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/mdev_driver.c
>> @@ -0,0 +1,122 @@
>> +/*
>> + * MDEV driver
>> + *
>> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
>> + * Author: Neo Jia <cjia@nvidia.com>
>> + * Kirti Wankhede <kwankhede@nvidia.com>
> Don't know if you care much for this:
> There is a TAB before your name. :>
>
Oh ok, Thanks for pointing that, I'll remove TAB.
I'll also take care of all the nits you suggested below.
Thanks,
Kirti
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/iommu.h>
>> +#include <linux/mdev.h>
>> +
>> +#include "mdev_private.h"
>> +
>> +static int mdev_attach_iommu(struct mdev_device *mdev)
>> +{
>> + int ret;
>> + struct iommu_group *group;
>> +
>> + group = iommu_group_alloc();
>> + if (IS_ERR(group))
>> + return PTR_ERR(group);
>> +
>> + ret = iommu_group_add_device(group, &mdev->dev);
>> + if (ret)
>> + goto attach_fail;
>> +
>> + dev_info(&mdev->dev, "MDEV: group_id = %d\n",
>> + iommu_group_id(group));
> nit: strange indentation.
> The above two lines could be safely merge into one line.
>
>> +attach_fail:
>> + iommu_group_put(group);
>> + return ret;
>> +}
>> +
>> +static void mdev_detach_iommu(struct mdev_device *mdev)
>> +{
>> + iommu_group_remove_device(&mdev->dev);
>> + dev_info(&mdev->dev, "MDEV: detaching iommu\n");
>> +}
>> +
>> +static int mdev_probe(struct device *dev)
>> +{
>> + struct mdev_driver *drv = to_mdev_driver(dev->driver);
>> + struct mdev_device *mdev = to_mdev_device(dev);
>> + int ret;
>> +
>> + ret = mdev_attach_iommu(mdev);
>> + if (ret)
>> + return ret;
>> +
>> + if (drv && drv->probe)
>> + ret = drv->probe(dev);
>> +
>> + if (ret)
>> + mdev_detach_iommu(mdev);
> ?
> if (drv && drv->probe) {
> ret = drv->probe(dev);
> if (ret)
> mdev_detach_iommu(mdev);
> }
>
>> +
>> + return ret;
>> +}
>> +
>> +static int mdev_remove(struct device *dev)
>> +{
>> + struct mdev_driver *drv = to_mdev_driver(dev->driver);
>> + struct mdev_device *mdev = to_mdev_device(dev);
>> +
>> + if (drv && drv->remove)
>> + drv->remove(dev);
>> +
>> + mdev_detach_iommu(mdev);
>> +
>> + return 0;
>> +}
>> +
>> +struct bus_type mdev_bus_type = {
>> + .name = "mdev",
>> + .probe = mdev_probe,
>> + .remove = mdev_remove,
>> +};
>> +EXPORT_SYMBOL_GPL(mdev_bus_type);
>> +
>> +/*
> Is this a kernel-doc comment?
> It should be started with:
> /**
>
>> + * mdev_register_driver - register a new MDEV driver
>> + * @drv: the driver to register
>> + * @owner: module owner of driver to be registered
>> + *
>> + * Returns a negative value on error, otherwise 0.
>> + */
>> +int mdev_register_driver(struct mdev_driver *drv, struct module *owner)
>> +{
>> + /* initialize common driver fields */
>> + drv->driver.name = drv->name;
>> + drv->driver.bus = &mdev_bus_type;
>> + drv->driver.owner = owner;
>> +
>> + /* register with core */
>> + return driver_register(&drv->driver);
>> +}
>> +EXPORT_SYMBOL(mdev_register_driver);
>> +
>> +/*
>> + * mdev_unregister_driver - unregister MDEV driver
>> + * @drv: the driver to unregister
>> + *
> Empty line.
>
>> + */
>> +void mdev_unregister_driver(struct mdev_driver *drv)
>> +{
>> + driver_unregister(&drv->driver);
>> +}
>> +EXPORT_SYMBOL(mdev_unregister_driver);
>> +
>> +int mdev_bus_register(void)
>> +{
>> + return bus_register(&mdev_bus_type);
>> +}
>> +
>> +void mdev_bus_unregister(void)
>> +{
>> + bus_unregister(&mdev_bus_type);
>> +}
> [...]
>
>> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
>> +/* Mediated device */
>> +struct mdev_device {
>> + struct device dev;
>> + struct parent_device *parent;
>> + uuid_le uuid;
>> + void *driver_data;
>> +
>> + /* internal */
>> + struct kref ref;
>> + struct list_head next;
>> + struct kobject *type_kobj;
>> +};
>> +
>> +
> Empty line.
>
>> +/**
>> + * struct parent_ops - Structure to be registered for each parent device to
>> + * register the device to mdev module.
>> + *
> [...]
>
>> + * @mmap: mmap callback
> No need a piece of description for arguments of the mmap callback?
>
>> + * Parent device that support mediated device should be registered with mdev
>> + * module with parent_ops structure.
>> + **/
>> +
>> +struct parent_ops {
>> + struct module *owner;
>> + const struct attribute_group **dev_attr_groups;
>> + const struct attribute_group **mdev_attr_groups;
>> + struct attribute_group **supported_type_groups;
>> +
>> + int (*create)(struct kobject *kobj, struct mdev_device *mdev);
>> + int (*remove)(struct mdev_device *mdev);
>> + int (*open)(struct mdev_device *mdev);
>> + void (*release)(struct mdev_device *mdev);
>> + ssize_t (*read)(struct mdev_device *mdev, char __user *buf,
>> + size_t count, loff_t *ppos);
>> + ssize_t (*write)(struct mdev_device *mdev, const char __user *buf,
>> + size_t count, loff_t *ppos);
>> + ssize_t (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
>> + unsigned long arg);
>> + int (*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
>> +};
>> +
> [...]
>
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-04 22:20 +0100 |
| Subject | [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops |
| Message-ID | <szU42-71A-59@gated-at.bofh.it> |
| In reply to | #1515412 |
Added two new callback functions to struct vfio_iommu_driver_ops. Backend
IOMMU module that supports pining and unpinning pages for mdev devices
should provide these functions.
Added APIs for pining and unpining pages to VFIO module. These calls back
into backend iommu module to actually pin and unpin pages.
Renamed static functions in vfio_type1_iommu.c to resolve conflicts
Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: Ia7417723aaae86bec2959ad9ae6c2915ddd340e0
---
drivers/vfio/vfio.c | 96 +++++++++++++++++++++++++++++++++++++++++
drivers/vfio/vfio_iommu_type1.c | 20 ++++-----
include/linux/vfio.h | 14 +++++-
3 files changed, 119 insertions(+), 11 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 2e83bdf007fe..76d260e98930 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1799,6 +1799,102 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
}
EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
+
+/*
+ * Pin a set of guest PFNs and return their associated host PFNs for local
+ * domain only.
+ * @dev [in] : device
+ * @user_pfn [in]: array of user/guest PFNs
+ * @npage [in]: count of array elements
+ * @prot [in] : protection flags
+ * @phys_pfn[out] : array of host PFNs
+ * Return error or number of pages pinned.
+ */
+int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
+ int npage, int prot, unsigned long *phys_pfn)
+{
+ struct vfio_container *container;
+ struct vfio_group *group;
+ struct vfio_iommu_driver *driver;
+ int ret;
+
+ if (!dev || !user_pfn || !phys_pfn)
+ 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_pin_pages;
+
+ container = group->container;
+ down_read(&container->group_lock);
+
+ driver = container->iommu_driver;
+ if (likely(driver && driver->ops->pin_pages))
+ ret = driver->ops->pin_pages(container->iommu_data, user_pfn,
+ npage, prot, phys_pfn);
+ else
+ ret = -EINVAL;
+
+ up_read(&container->group_lock);
+ vfio_group_try_dissolve_container(group);
+
+err_pin_pages:
+ vfio_group_put(group);
+ return ret;
+
+}
+EXPORT_SYMBOL(vfio_pin_pages);
+
+/*
+ * Unpin set of host PFNs for local domain only.
+ * @dev [in] : device
+ * @user_pfn [in]: array of user/guest PFNs to be unpinned
+ * @pfn [in] : array of host PFNs to be unpinned.
+ * @npage [in] :count of elements in array, that is number of pages.
+ * Return error or number of pages unpinned.
+ */
+int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
+ unsigned long *pfn, int npage)
+{
+ struct vfio_container *container;
+ struct vfio_group *group;
+ struct vfio_iommu_driver *driver;
+ int ret;
+
+ if (!dev || !pfn)
+ 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_unpin_pages;
+
+ container = group->container;
+ down_read(&container->group_lock);
+
+ driver = container->iommu_driver;
+ if (likely(driver && driver->ops->unpin_pages))
+ ret = driver->ops->unpin_pages(container->iommu_data, user_pfn,
+ pfn, npage);
+ else
+ ret = -EINVAL;
+
+ up_read(&container->group_lock);
+ vfio_group_try_dissolve_container(group);
+
+err_unpin_pages:
+ vfio_group_put(group);
+ return ret;
+}
+EXPORT_SYMBOL(vfio_unpin_pages);
+
/**
* Module/class support
*/
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2ba19424e4a1..7fb87f008e0a 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -259,8 +259,8 @@ static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
* the iommu can only map chunks of consecutive pfns anyway, so get the
* first page and all consecutive pages with the same locking.
*/
-static long vfio_pin_pages(unsigned long vaddr, long npage,
- int prot, unsigned long *pfn_base)
+static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
+ int prot, unsigned long *pfn_base)
{
unsigned long limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
bool lock_cap = capable(CAP_IPC_LOCK);
@@ -318,8 +318,8 @@ static long vfio_pin_pages(unsigned long vaddr, long npage,
return i;
}
-static long vfio_unpin_pages(unsigned long pfn, long npage,
- int prot, bool do_accounting)
+static long __vfio_unpin_pages_remote(unsigned long pfn, long npage,
+ int prot, bool do_accounting)
{
unsigned long unlocked = 0;
long i;
@@ -382,9 +382,9 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
if (WARN_ON(!unmapped))
break;
- unlocked += vfio_unpin_pages(phys >> PAGE_SHIFT,
- unmapped >> PAGE_SHIFT,
- dma->prot, false);
+ unlocked += __vfio_unpin_pages_remote(phys >> PAGE_SHIFT,
+ unmapped >> PAGE_SHIFT,
+ dma->prot, false);
iova += unmapped;
cond_resched();
@@ -613,8 +613,8 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
while (size) {
/* Pin a contiguous chunk of memory */
- npage = vfio_pin_pages(vaddr + dma->size,
- size >> PAGE_SHIFT, prot, &pfn);
+ npage = __vfio_pin_pages_remote(vaddr + dma->size,
+ size >> PAGE_SHIFT, prot, &pfn);
if (npage <= 0) {
WARN_ON(!npage);
ret = (int)npage;
@@ -624,7 +624,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
/* Map it! */
ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage, prot);
if (ret) {
- vfio_unpin_pages(pfn, npage, prot, true);
+ __vfio_unpin_pages_remote(pfn, npage, prot, true);
break;
}
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 0ecae0b1cd34..ba1b64cb7d4b 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -75,7 +75,13 @@ struct vfio_iommu_driver_ops {
struct iommu_group *group);
void (*detach_group)(void *iommu_data,
struct iommu_group *group);
-
+ int (*pin_pages)(void *iommu_data, unsigned long *user_pfn,
+ int npage, int prot,
+ unsigned long *phys_pfn);
+ int (*unpin_pages)(void *iommu_data,
+ unsigned long *user_pfn,
+ unsigned long *pfn,
+ int npage);
};
extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
@@ -127,6 +133,12 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
}
#endif /* CONFIG_EEH */
+extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
+ int npage, int prot, unsigned long *phys_pfn);
+
+extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
+ unsigned long *pfn, int npage);
+
/*
* IRQfd - generic
*/
--
2.7.0
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2016-11-07 20:40 +0100 |
| Subject | Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops |
| Message-ID | <sAXVU-7y3-5@gated-at.bofh.it> |
| In reply to | #1515434 |
On Sat, 5 Nov 2016 02:40:39 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:
> Added two new callback functions to struct vfio_iommu_driver_ops. Backend
> IOMMU module that supports pining and unpinning pages for mdev devices
> should provide these functions.
> Added APIs for pining and unpining pages to VFIO module. These calls back
> into backend iommu module to actually pin and unpin pages.
> Renamed static functions in vfio_type1_iommu.c to resolve conflicts
>
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: Ia7417723aaae86bec2959ad9ae6c2915ddd340e0
> ---
> drivers/vfio/vfio.c | 96 +++++++++++++++++++++++++++++++++++++++++
> drivers/vfio/vfio_iommu_type1.c | 20 ++++-----
> include/linux/vfio.h | 14 +++++-
> 3 files changed, 119 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 2e83bdf007fe..76d260e98930 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1799,6 +1799,102 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
> }
> EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
>
> +
> +/*
> + * Pin a set of guest PFNs and return their associated host PFNs for local
> + * domain only.
> + * @dev [in] : device
> + * @user_pfn [in]: array of user/guest PFNs
> + * @npage [in]: count of array elements
> + * @prot [in] : protection flags
> + * @phys_pfn[out] : array of host PFNs
> + * Return error or number of pages pinned.
> + */
> +int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
> + int npage, int prot, unsigned long *phys_pfn)
> +{
> + struct vfio_container *container;
> + struct vfio_group *group;
> + struct vfio_iommu_driver *driver;
> + int ret;
> +
> + if (!dev || !user_pfn || !phys_pfn)
> + 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_pin_pages;
> +
> + container = group->container;
> + down_read(&container->group_lock);
> +
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->pin_pages))
> + ret = driver->ops->pin_pages(container->iommu_data, user_pfn,
> + npage, prot, phys_pfn);
> + else
> + ret = -EINVAL;
-ENOTTY might be a more appropriate error return here and below since
we're not signaling invalid argument, we're signaling lack of support.
> +
> + up_read(&container->group_lock);
> + vfio_group_try_dissolve_container(group);
> +
> +err_pin_pages:
> + vfio_group_put(group);
> + return ret;
> +
Unnecessary extra line
> +}
> +EXPORT_SYMBOL(vfio_pin_pages);
> +
> +/*
> + * Unpin set of host PFNs for local domain only.
> + * @dev [in] : device
> + * @user_pfn [in]: array of user/guest PFNs to be unpinned
> + * @pfn [in] : array of host PFNs to be unpinned.
> + * @npage [in] :count of elements in array, that is number of pages.
> + * Return error or number of pages unpinned.
> + */
> +int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
> + unsigned long *pfn, int npage)
> +{
> + struct vfio_container *container;
> + struct vfio_group *group;
> + struct vfio_iommu_driver *driver;
> + int ret;
> +
> + if (!dev || !pfn)
> + 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_unpin_pages;
> +
> + container = group->container;
> + down_read(&container->group_lock);
> +
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->unpin_pages))
> + ret = driver->ops->unpin_pages(container->iommu_data, user_pfn,
> + pfn, npage);
> + else
> + ret = -EINVAL;
> +
> + up_read(&container->group_lock);
> + vfio_group_try_dissolve_container(group);
> +
> +err_unpin_pages:
> + vfio_group_put(group);
> + return ret;
> +}
> +EXPORT_SYMBOL(vfio_unpin_pages);
> +
> /**
> * Module/class support
> */
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 2ba19424e4a1..7fb87f008e0a 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -259,8 +259,8 @@ static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
> * the iommu can only map chunks of consecutive pfns anyway, so get the
> * first page and all consecutive pages with the same locking.
> */
> -static long vfio_pin_pages(unsigned long vaddr, long npage,
> - int prot, unsigned long *pfn_base)
> +static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
> + int prot, unsigned long *pfn_base)
nit, what is the additional underscore prefix intended to imply?
Appending _remote is sufficient to avoid the symbol conflict.
> {
> unsigned long limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> bool lock_cap = capable(CAP_IPC_LOCK);
> @@ -318,8 +318,8 @@ static long vfio_pin_pages(unsigned long vaddr, long npage,
> return i;
> }
>
> -static long vfio_unpin_pages(unsigned long pfn, long npage,
> - int prot, bool do_accounting)
> +static long __vfio_unpin_pages_remote(unsigned long pfn, long npage,
> + int prot, bool do_accounting)
> {
> unsigned long unlocked = 0;
> long i;
> @@ -382,9 +382,9 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
> if (WARN_ON(!unmapped))
> break;
>
> - unlocked += vfio_unpin_pages(phys >> PAGE_SHIFT,
> - unmapped >> PAGE_SHIFT,
> - dma->prot, false);
> + unlocked += __vfio_unpin_pages_remote(phys >> PAGE_SHIFT,
> + unmapped >> PAGE_SHIFT,
> + dma->prot, false);
> iova += unmapped;
>
> cond_resched();
> @@ -613,8 +613,8 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
>
> while (size) {
> /* Pin a contiguous chunk of memory */
> - npage = vfio_pin_pages(vaddr + dma->size,
> - size >> PAGE_SHIFT, prot, &pfn);
> + npage = __vfio_pin_pages_remote(vaddr + dma->size,
> + size >> PAGE_SHIFT, prot, &pfn);
> if (npage <= 0) {
> WARN_ON(!npage);
> ret = (int)npage;
> @@ -624,7 +624,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
> /* Map it! */
> ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage, prot);
> if (ret) {
> - vfio_unpin_pages(pfn, npage, prot, true);
> + __vfio_unpin_pages_remote(pfn, npage, prot, true);
> break;
> }
>
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 0ecae0b1cd34..ba1b64cb7d4b 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -75,7 +75,13 @@ struct vfio_iommu_driver_ops {
> struct iommu_group *group);
> void (*detach_group)(void *iommu_data,
> struct iommu_group *group);
> -
> + int (*pin_pages)(void *iommu_data, unsigned long *user_pfn,
> + int npage, int prot,
> + unsigned long *phys_pfn);
> + int (*unpin_pages)(void *iommu_data,
Are we changing from long to int here simply because of the absurdity
in passing in more than a 2^31 entry array, that would already consume
more than 16GB itself?
> + unsigned long *user_pfn,
> + unsigned long *pfn,
nit, use phys_pfn so as to match the pin function?
> + int npage);
> };
>
> extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
> @@ -127,6 +133,12 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
> }
> #endif /* CONFIG_EEH */
>
> +extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
> + int npage, int prot, unsigned long *phys_pfn);
> +
> +extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
> + unsigned long *pfn, int npage);
> +
> /*
> * IRQfd - generic
> */
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-08 15:00 +0100 |
| Subject | Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops |
| Message-ID | <sBf6r-1Bm-39@gated-at.bofh.it> |
| In reply to | #1516536 |
On 11/8/2016 1:06 AM, Alex Williamson wrote:
> On Sat, 5 Nov 2016 02:40:39 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>
...
>> +int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
>> + int npage, int prot, unsigned long *phys_pfn)
>> +{
>> + struct vfio_container *container;
>> + struct vfio_group *group;
>> + struct vfio_iommu_driver *driver;
>> + int ret;
>> +
>> + if (!dev || !user_pfn || !phys_pfn)
>> + 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_pin_pages;
>> +
>> + container = group->container;
>> + down_read(&container->group_lock);
>> +
>> + driver = container->iommu_driver;
>> + if (likely(driver && driver->ops->pin_pages))
>> + ret = driver->ops->pin_pages(container->iommu_data, user_pfn,
>> + npage, prot, phys_pfn);
>> + else
>> + ret = -EINVAL;
>
> -ENOTTY might be a more appropriate error return here and below since
> we're not signaling invalid argument, we're signaling lack of support.
>
Used -EINVAL in sync with other driver->ops like read, write and mmap.
Changing it to -ENOTTY as you suggested above since these ops are optional.
...
>> -static long vfio_pin_pages(unsigned long vaddr, long npage,
>> - int prot, unsigned long *pfn_base)
>> +static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
>> + int prot, unsigned long *pfn_base)
>
> nit, what is the additional underscore prefix intended to imply?
> Appending _remote is sufficient to avoid the symbol conflict.
>
This function name changed in review process from start, we started with
changing to __vfio_pin_pages and then added _remote to it later. We can
remove '__' from it. Updating.
...
>> -
>> + int (*pin_pages)(void *iommu_data, unsigned long *user_pfn,
>> + int npage, int prot,
>> + unsigned long *phys_pfn);
>> + int (*unpin_pages)(void *iommu_data,
>
> Are we changing from long to int here simply because of the absurdity
> in passing in more than a 2^31 entry array, that would already consume
> more than 16GB itself?
>
These are on demand pin/unpin request, will that request go beyond 16GB
limit? For Nvidia vGPU solution, pin request will not go beyond this limit.
>> + unsigned long *user_pfn,
>> + unsigned long *pfn,
>
> nit, use phys_pfn so as to match the pin function?
>
Ok.
>> + int npage);
>> };
>>
>> extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
>> @@ -127,6 +133,12 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>> }
>> #endif /* CONFIG_EEH */
>>
>> +extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
>> + int npage, int prot, unsigned long *phys_pfn);
>> +
>> +extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
>> + unsigned long *pfn, int npage);
>> +
>> /*
>> * IRQfd - generic
>> */
>
Thanks,
Kirti
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2016-11-08 17:50 +0100 |
| Subject | Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops |
| Message-ID | <sBhKW-3nj-33@gated-at.bofh.it> |
| In reply to | #1517210 |
On Tue, 8 Nov 2016 19:25:35 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:
> On 11/8/2016 1:06 AM, Alex Williamson wrote:
> > On Sat, 5 Nov 2016 02:40:39 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >
> ...
> >> +int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
> >> + int npage, int prot, unsigned long *phys_pfn)
> >> +{
> >> + struct vfio_container *container;
> >> + struct vfio_group *group;
> >> + struct vfio_iommu_driver *driver;
> >> + int ret;
> >> +
> >> + if (!dev || !user_pfn || !phys_pfn)
> >> + 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_pin_pages;
> >> +
> >> + container = group->container;
> >> + down_read(&container->group_lock);
> >> +
> >> + driver = container->iommu_driver;
> >> + if (likely(driver && driver->ops->pin_pages))
> >> + ret = driver->ops->pin_pages(container->iommu_data, user_pfn,
> >> + npage, prot, phys_pfn);
> >> + else
> >> + ret = -EINVAL;
> >
> > -ENOTTY might be a more appropriate error return here and below since
> > we're not signaling invalid argument, we're signaling lack of support.
> >
>
> Used -EINVAL in sync with other driver->ops like read, write and mmap.
> Changing it to -ENOTTY as you suggested above since these ops are optional.
TBH, I'm not sure which is better, but it's nice to try to
differentiate different error paths with different errno values.
> ...
>
> >> -static long vfio_pin_pages(unsigned long vaddr, long npage,
> >> - int prot, unsigned long *pfn_base)
> >> +static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
> >> + int prot, unsigned long *pfn_base)
> >
> > nit, what is the additional underscore prefix intended to imply?
> > Appending _remote is sufficient to avoid the symbol conflict.
> >
>
> This function name changed in review process from start, we started with
> changing to __vfio_pin_pages and then added _remote to it later. We can
> remove '__' from it. Updating.
>
> ...
>
> >> -
> >> + int (*pin_pages)(void *iommu_data, unsigned long *user_pfn,
> >> + int npage, int prot,
> >> + unsigned long *phys_pfn);
> >> + int (*unpin_pages)(void *iommu_data,
> >
> > Are we changing from long to int here simply because of the absurdity
> > in passing in more than a 2^31 entry array, that would already consume
> > more than 16GB itself?
> >
>
> These are on demand pin/unpin request, will that request go beyond 16GB
> limit? For Nvidia vGPU solution, pin request will not go beyond this limit.
16G is simply the size of the user_pfn or phys_pfn arrays at a maximal
int32_t npage value, the interface actually allows mapping up to 8TB
per call, but at that point we have 16GB of input, 16GB of output, and
80GB of vfio_pfns created. So I don't really have a problem changing
form long to int given lack of scalability in the API in general, but
it does make me second guess the API itself. Thanks,
Alex
> >> + unsigned long *user_pfn,
> >> + unsigned long *pfn,
> >
> > nit, use phys_pfn so as to match the pin function?
> >
>
> Ok.
>
> >> + int npage);
> >> };
> >>
> >> extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
> >> @@ -127,6 +133,12 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
> >> }
> >> #endif /* CONFIG_EEH */
> >>
> >> +extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
> >> + int npage, int prot, unsigned long *phys_pfn);
> >> +
> >> +extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
> >> + unsigned long *pfn, int npage);
> >> +
> >> /*
> >> * IRQfd - generic
> >> */
> >
>
> Thanks,
> Kirti
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-08 20:00 +0100 |
| Subject | Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops |
| Message-ID | <sBjMK-4Jv-47@gated-at.bofh.it> |
| In reply to | #1517380 |
On 11/8/2016 10:09 PM, Alex Williamson wrote: > On Tue, 8 Nov 2016 19:25:35 +0530 > Kirti Wankhede <kwankhede@nvidia.com> wrote: > ... >>>> - >>>> + int (*pin_pages)(void *iommu_data, unsigned long *user_pfn, >>>> + int npage, int prot, >>>> + unsigned long *phys_pfn); >>>> + int (*unpin_pages)(void *iommu_data, >>> >>> Are we changing from long to int here simply because of the absurdity >>> in passing in more than a 2^31 entry array, that would already consume >>> more than 16GB itself? >>> >> >> These are on demand pin/unpin request, will that request go beyond 16GB >> limit? For Nvidia vGPU solution, pin request will not go beyond this limit. > > 16G is simply the size of the user_pfn or phys_pfn arrays at a maximal > int32_t npage value, the interface actually allows mapping up to 8TB > per call, but at that point we have 16GB of input, 16GB of output, and > 80GB of vfio_pfns created. So I don't really have a problem changing > form long to int given lack of scalability in the API in general, but > it does make me second guess the API itself. Thanks, > Changing to 'long', in future we might enhance this API without changing it signature. Thanks, Kirti
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2016-11-08 20:20 +0100 |
| Subject | Re: [PATCH v11 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops |
| Message-ID | <sBk66-57F-15@gated-at.bofh.it> |
| In reply to | #1517486 |
On Wed, 9 Nov 2016 00:17:53 +0530 Kirti Wankhede <kwankhede@nvidia.com> wrote: > On 11/8/2016 10:09 PM, Alex Williamson wrote: > > On Tue, 8 Nov 2016 19:25:35 +0530 > > Kirti Wankhede <kwankhede@nvidia.com> wrote: > > > ... > > >>>> - > >>>> + int (*pin_pages)(void *iommu_data, unsigned long *user_pfn, > >>>> + int npage, int prot, > >>>> + unsigned long *phys_pfn); > >>>> + int (*unpin_pages)(void *iommu_data, > >>> > >>> Are we changing from long to int here simply because of the absurdity > >>> in passing in more than a 2^31 entry array, that would already consume > >>> more than 16GB itself? > >>> > >> > >> These are on demand pin/unpin request, will that request go beyond 16GB > >> limit? For Nvidia vGPU solution, pin request will not go beyond this limit. > > > > 16G is simply the size of the user_pfn or phys_pfn arrays at a maximal > > int32_t npage value, the interface actually allows mapping up to 8TB > > per call, but at that point we have 16GB of input, 16GB of output, and > > 80GB of vfio_pfns created. So I don't really have a problem changing > > form long to int given lack of scalability in the API in general, but > > it does make me second guess the API itself. Thanks, > > > > Changing to 'long', in future we might enhance this API without changing > it signature. I think the pfn arrays are more of a problem long term than whether we can only map 2^31 pfns in one call. I particularly dislike that the caller provides both the iova and pfn arrays for unpinning. Being an in-kernel driver, we should trust it, but it makes the interface difficult to use and seems like it indicates that our tracking data structures aren't architected the way they should be. Upstream, this API will need to be flexible and change over time, it's only the downstream distros that may lock in a kABI. Not breaking them should be a consideration, but also needs to be weighted against long term upstream goals. Thanks, Alex
[toc] | [prev] | [next] | [standalone]
| From | Alexey Kardashevskiy <aik@ozlabs.ru> |
|---|---|
| Date | 2016-11-07 04:50 +0100 |
| Message-ID | <sAJ6y-5Y3-3@gated-at.bofh.it> |
| In reply to | #1515412 |
On 05/11/16 08:10, Kirti Wankhede wrote: > This series adds Mediated device support to Linux host kernel. Purpose > of this series is to provide a common interface for mediated device > management that can be used by different devices. This series introduces > Mdev core module that creates and manages mediated devices, VFIO based > driver for mediated devices that are created by mdev core module and > update VFIO type1 IOMMU module to support pinning & unpinning for mediated > devices. > > What changed in v11? > mdev core: > Register mdev_bus class when first device is registed to avoid panic if > any vendor driver and mdev driver are selected as built-in but vendor > driver loads first and then mdev module. > vfio_mdev: > Added notifier callback function to mdev parent's ops so that notifer > is registered from vfio_mdev module during device open and unregistered > it from device close call. This is a optional callback. Some drivers > using mdev framework might not pin or unpin pages, for example the > sample mtty driver that simulates serial port. Vendor driver who need to > pin/unpin pages should provide this callback. Otherwise pin request > would fail. > vfio_iommu_type1: > Updated to keep track of who (task and address space) mapped iova range. > During DMA_UNMAP, same task who mapped it or other task who shares same > address space is allowed to unmap, otherwise unmap fails. > QEMU maps few iova ranges initially, then fork threads and from the child > thread calls DMA_UNMAP on previously mapped iova. Since child shares same > address space, DMA_UNMAP is successful. > Address space keeps track of pages pinned (pfn_list) by external user / > mdev devices. This pfn_list is used to verify pfn during unpin_request, > re-accounting of pages when direct device assigned in hot-unplugged and > mdev device is present in same container. > When the container is released, all mapped iova from all tasks are > unmapped and removed. > > Tested by assigning below combinations of devices to a single VM: > - GPU pass through only > - vGPU device only > - One GPU pass through and one vGPU device > - Linux VM hot plug and unplug vGPU device while GPU pass through device > exist > - Linux VM hot plug and unplug GPU pass through device while vGPU device > exist > > Patch series tested with linux-next upto commit 14970f204b19 @Fri Oct 28 > Resolved against conflicting change: Patch 11/22 does not apply if the patchset is applied on top of 14970f204b19. > 05692d7005a3 vfio/pci: Fix integer overflows, bitmask check > > > Kirti Wankhede (22): > vfio: Mediated device Core driver > vfio: VFIO based driver for Mediated devices > vfio: Rearrange functions to get vfio_group from dev > vfio: Common function to increment container_users > vfio iommu: Added pin and unpin callback functions to > vfio_iommu_driver_ops > vfio iommu type1: Update arguments of vfio_lock_acct > vfio iommu type1: Update argument of vaddr_get_pfn() > vfio iommu type1: Add find_iommu_group() function > vfio iommu type1: Add task structure to vfio_dma > vfio iommu type1: Add support for mediated devices > vfio iommu: Add blocking notifier to notify DMA_UNMAP > vfio: Add notifier callback to parent's ops structure of mdev > vfio: Introduce common function to add capabilities > vfio_pci: Update vfio_pci to use vfio_info_add_capability() > vfio: Introduce vfio_set_irqs_validate_and_prepare() > vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare() > vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare() > vfio: Define device_api strings > docs: Add Documentation for Mediated devices > docs: Sysfs ABI for mediated device framework > docs: Sample driver to demonstrate how to use Mediated device > framework. > MAINTAINERS: Add entry VFIO based Mediated device drivers > > Documentation/ABI/testing/sysfs-bus-vfio-mdev | 111 ++ > Documentation/vfio-mediated-device.txt | 399 +++++++ > MAINTAINERS | 9 + > drivers/vfio/Kconfig | 1 + > drivers/vfio/Makefile | 1 + > drivers/vfio/mdev/Kconfig | 17 + > drivers/vfio/mdev/Makefile | 5 + > drivers/vfio/mdev/mdev_core.c | 388 +++++++ > drivers/vfio/mdev/mdev_driver.c | 122 ++ > drivers/vfio/mdev/mdev_private.h | 41 + > drivers/vfio/mdev/mdev_sysfs.c | 286 +++++ > drivers/vfio/mdev/vfio_mdev.c | 167 +++ > drivers/vfio/pci/vfio_pci.c | 83 +- > drivers/vfio/platform/vfio_platform_common.c | 31 +- > drivers/vfio/vfio.c | 334 +++++- > drivers/vfio/vfio_iommu_type1.c | 831 ++++++++++++-- > include/linux/mdev.h | 176 +++ > include/linux/vfio.h | 32 +- > include/uapi/linux/vfio.h | 10 + > samples/vfio-mdev/Makefile | 13 + > samples/vfio-mdev/mtty.c | 1503 +++++++++++++++++++++++++ > 21 files changed, 4342 insertions(+), 218 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-bus-vfio-mdev > create mode 100644 Documentation/vfio-mediated-device.txt > create mode 100644 drivers/vfio/mdev/Kconfig > create mode 100644 drivers/vfio/mdev/Makefile > create mode 100644 drivers/vfio/mdev/mdev_core.c > create mode 100644 drivers/vfio/mdev/mdev_driver.c > create mode 100644 drivers/vfio/mdev/mdev_private.h > create mode 100644 drivers/vfio/mdev/mdev_sysfs.c > create mode 100644 drivers/vfio/mdev/vfio_mdev.c > create mode 100644 include/linux/mdev.h > create mode 100644 samples/vfio-mdev/Makefile > create mode 100644 samples/vfio-mdev/mtty.c > -- Alexey
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-07 05:00 +0100 |
| Message-ID | <sAJgj-61l-3@gated-at.bofh.it> |
| In reply to | #1515837 |
On 11/7/2016 9:00 AM, Alexey Kardashevskiy wrote: > On 05/11/16 08:10, Kirti Wankhede wrote: >> This series adds Mediated device support to Linux host kernel. Purpose >> of this series is to provide a common interface for mediated device >> management that can be used by different devices. This series introduces >> Mdev core module that creates and manages mediated devices, VFIO based >> driver for mediated devices that are created by mdev core module and >> update VFIO type1 IOMMU module to support pinning & unpinning for mediated >> devices. >> >> What changed in v11? >> mdev core: >> Register mdev_bus class when first device is registed to avoid panic if >> any vendor driver and mdev driver are selected as built-in but vendor >> driver loads first and then mdev module. >> vfio_mdev: >> Added notifier callback function to mdev parent's ops so that notifer >> is registered from vfio_mdev module during device open and unregistered >> it from device close call. This is a optional callback. Some drivers >> using mdev framework might not pin or unpin pages, for example the >> sample mtty driver that simulates serial port. Vendor driver who need to >> pin/unpin pages should provide this callback. Otherwise pin request >> would fail. >> vfio_iommu_type1: >> Updated to keep track of who (task and address space) mapped iova range. >> During DMA_UNMAP, same task who mapped it or other task who shares same >> address space is allowed to unmap, otherwise unmap fails. >> QEMU maps few iova ranges initially, then fork threads and from the child >> thread calls DMA_UNMAP on previously mapped iova. Since child shares same >> address space, DMA_UNMAP is successful. >> Address space keeps track of pages pinned (pfn_list) by external user / >> mdev devices. This pfn_list is used to verify pfn during unpin_request, >> re-accounting of pages when direct device assigned in hot-unplugged and >> mdev device is present in same container. >> When the container is released, all mapped iova from all tasks are >> unmapped and removed. >> >> Tested by assigning below combinations of devices to a single VM: >> - GPU pass through only >> - vGPU device only >> - One GPU pass through and one vGPU device >> - Linux VM hot plug and unplug vGPU device while GPU pass through device >> exist >> - Linux VM hot plug and unplug GPU pass through device while vGPU device >> exist >> >> Patch series tested with linux-next upto commit 14970f204b19 @Fri Oct 28 >> Resolved against conflicting change: > > Patch 11/22 does not apply if the patchset is applied on top of 14970f204b19. > > Pull commit 05692d7005a3 (below) commit. > >> 05692d7005a3 vfio/pci: Fix integer overflows, bitmask check >> Thanks, Kirti >> >> Kirti Wankhede (22): >> vfio: Mediated device Core driver >> vfio: VFIO based driver for Mediated devices >> vfio: Rearrange functions to get vfio_group from dev >> vfio: Common function to increment container_users >> vfio iommu: Added pin and unpin callback functions to >> vfio_iommu_driver_ops >> vfio iommu type1: Update arguments of vfio_lock_acct >> vfio iommu type1: Update argument of vaddr_get_pfn() >> vfio iommu type1: Add find_iommu_group() function >> vfio iommu type1: Add task structure to vfio_dma >> vfio iommu type1: Add support for mediated devices >> vfio iommu: Add blocking notifier to notify DMA_UNMAP >> vfio: Add notifier callback to parent's ops structure of mdev >> vfio: Introduce common function to add capabilities >> vfio_pci: Update vfio_pci to use vfio_info_add_capability() >> vfio: Introduce vfio_set_irqs_validate_and_prepare() >> vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare() >> vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare() >> vfio: Define device_api strings >> docs: Add Documentation for Mediated devices >> docs: Sysfs ABI for mediated device framework >> docs: Sample driver to demonstrate how to use Mediated device >> framework. >> MAINTAINERS: Add entry VFIO based Mediated device drivers >> >> Documentation/ABI/testing/sysfs-bus-vfio-mdev | 111 ++ >> Documentation/vfio-mediated-device.txt | 399 +++++++ >> MAINTAINERS | 9 + >> drivers/vfio/Kconfig | 1 + >> drivers/vfio/Makefile | 1 + >> drivers/vfio/mdev/Kconfig | 17 + >> drivers/vfio/mdev/Makefile | 5 + >> drivers/vfio/mdev/mdev_core.c | 388 +++++++ >> drivers/vfio/mdev/mdev_driver.c | 122 ++ >> drivers/vfio/mdev/mdev_private.h | 41 + >> drivers/vfio/mdev/mdev_sysfs.c | 286 +++++ >> drivers/vfio/mdev/vfio_mdev.c | 167 +++ >> drivers/vfio/pci/vfio_pci.c | 83 +- >> drivers/vfio/platform/vfio_platform_common.c | 31 +- >> drivers/vfio/vfio.c | 334 +++++- >> drivers/vfio/vfio_iommu_type1.c | 831 ++++++++++++-- >> include/linux/mdev.h | 176 +++ >> include/linux/vfio.h | 32 +- >> include/uapi/linux/vfio.h | 10 + >> samples/vfio-mdev/Makefile | 13 + >> samples/vfio-mdev/mtty.c | 1503 +++++++++++++++++++++++++ >> 21 files changed, 4342 insertions(+), 218 deletions(-) >> create mode 100644 Documentation/ABI/testing/sysfs-bus-vfio-mdev >> create mode 100644 Documentation/vfio-mediated-device.txt >> create mode 100644 drivers/vfio/mdev/Kconfig >> create mode 100644 drivers/vfio/mdev/Makefile >> create mode 100644 drivers/vfio/mdev/mdev_core.c >> create mode 100644 drivers/vfio/mdev/mdev_driver.c >> create mode 100644 drivers/vfio/mdev/mdev_private.h >> create mode 100644 drivers/vfio/mdev/mdev_sysfs.c >> create mode 100644 drivers/vfio/mdev/vfio_mdev.c >> create mode 100644 include/linux/mdev.h >> create mode 100644 samples/vfio-mdev/Makefile >> create mode 100644 samples/vfio-mdev/mtty.c >> > >
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-07 06:10 +0100 |
| Message-ID | <sAKlX-6Vb-3@gated-at.bofh.it> |
| In reply to | #1515839 |
Verified that this patch series gets applied cleanly to Linux-next upto: bc33b0ca11e3 Linux 4.9-rc4 Also tested this patch series with above kernel. Thanks, Kirti On 11/7/2016 9:29 AM, Kirti Wankhede wrote: > > > On 11/7/2016 9:00 AM, Alexey Kardashevskiy wrote: >> On 05/11/16 08:10, Kirti Wankhede wrote: >>> This series adds Mediated device support to Linux host kernel. Purpose >>> of this series is to provide a common interface for mediated device >>> management that can be used by different devices. This series introduces >>> Mdev core module that creates and manages mediated devices, VFIO based >>> driver for mediated devices that are created by mdev core module and >>> update VFIO type1 IOMMU module to support pinning & unpinning for mediated >>> devices. >>> >>> What changed in v11? >>> mdev core: >>> Register mdev_bus class when first device is registed to avoid panic if >>> any vendor driver and mdev driver are selected as built-in but vendor >>> driver loads first and then mdev module. >>> vfio_mdev: >>> Added notifier callback function to mdev parent's ops so that notifer >>> is registered from vfio_mdev module during device open and unregistered >>> it from device close call. This is a optional callback. Some drivers >>> using mdev framework might not pin or unpin pages, for example the >>> sample mtty driver that simulates serial port. Vendor driver who need to >>> pin/unpin pages should provide this callback. Otherwise pin request >>> would fail. >>> vfio_iommu_type1: >>> Updated to keep track of who (task and address space) mapped iova range. >>> During DMA_UNMAP, same task who mapped it or other task who shares same >>> address space is allowed to unmap, otherwise unmap fails. >>> QEMU maps few iova ranges initially, then fork threads and from the child >>> thread calls DMA_UNMAP on previously mapped iova. Since child shares same >>> address space, DMA_UNMAP is successful. >>> Address space keeps track of pages pinned (pfn_list) by external user / >>> mdev devices. This pfn_list is used to verify pfn during unpin_request, >>> re-accounting of pages when direct device assigned in hot-unplugged and >>> mdev device is present in same container. >>> When the container is released, all mapped iova from all tasks are >>> unmapped and removed. >>> >>> Tested by assigning below combinations of devices to a single VM: >>> - GPU pass through only >>> - vGPU device only >>> - One GPU pass through and one vGPU device >>> - Linux VM hot plug and unplug vGPU device while GPU pass through device >>> exist >>> - Linux VM hot plug and unplug GPU pass through device while vGPU device >>> exist >>> >>> Patch series tested with linux-next upto commit 14970f204b19 @Fri Oct 28 >>> Resolved against conflicting change: >> >> Patch 11/22 does not apply if the patchset is applied on top of 14970f204b19. >> >> > > Pull commit 05692d7005a3 (below) commit. > >> >>> 05692d7005a3 vfio/pci: Fix integer overflows, bitmask check >>> > > Thanks, > Kirti > >>> >>> Kirti Wankhede (22): >>> vfio: Mediated device Core driver >>> vfio: VFIO based driver for Mediated devices >>> vfio: Rearrange functions to get vfio_group from dev >>> vfio: Common function to increment container_users >>> vfio iommu: Added pin and unpin callback functions to >>> vfio_iommu_driver_ops >>> vfio iommu type1: Update arguments of vfio_lock_acct >>> vfio iommu type1: Update argument of vaddr_get_pfn() >>> vfio iommu type1: Add find_iommu_group() function >>> vfio iommu type1: Add task structure to vfio_dma >>> vfio iommu type1: Add support for mediated devices >>> vfio iommu: Add blocking notifier to notify DMA_UNMAP >>> vfio: Add notifier callback to parent's ops structure of mdev >>> vfio: Introduce common function to add capabilities >>> vfio_pci: Update vfio_pci to use vfio_info_add_capability() >>> vfio: Introduce vfio_set_irqs_validate_and_prepare() >>> vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare() >>> vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare() >>> vfio: Define device_api strings >>> docs: Add Documentation for Mediated devices >>> docs: Sysfs ABI for mediated device framework >>> docs: Sample driver to demonstrate how to use Mediated device >>> framework. >>> MAINTAINERS: Add entry VFIO based Mediated device drivers >>> >>> Documentation/ABI/testing/sysfs-bus-vfio-mdev | 111 ++ >>> Documentation/vfio-mediated-device.txt | 399 +++++++ >>> MAINTAINERS | 9 + >>> drivers/vfio/Kconfig | 1 + >>> drivers/vfio/Makefile | 1 + >>> drivers/vfio/mdev/Kconfig | 17 + >>> drivers/vfio/mdev/Makefile | 5 + >>> drivers/vfio/mdev/mdev_core.c | 388 +++++++ >>> drivers/vfio/mdev/mdev_driver.c | 122 ++ >>> drivers/vfio/mdev/mdev_private.h | 41 + >>> drivers/vfio/mdev/mdev_sysfs.c | 286 +++++ >>> drivers/vfio/mdev/vfio_mdev.c | 167 +++ >>> drivers/vfio/pci/vfio_pci.c | 83 +- >>> drivers/vfio/platform/vfio_platform_common.c | 31 +- >>> drivers/vfio/vfio.c | 334 +++++- >>> drivers/vfio/vfio_iommu_type1.c | 831 ++++++++++++-- >>> include/linux/mdev.h | 176 +++ >>> include/linux/vfio.h | 32 +- >>> include/uapi/linux/vfio.h | 10 + >>> samples/vfio-mdev/Makefile | 13 + >>> samples/vfio-mdev/mtty.c | 1503 +++++++++++++++++++++++++ >>> 21 files changed, 4342 insertions(+), 218 deletions(-) >>> create mode 100644 Documentation/ABI/testing/sysfs-bus-vfio-mdev >>> create mode 100644 Documentation/vfio-mediated-device.txt >>> create mode 100644 drivers/vfio/mdev/Kconfig >>> create mode 100644 drivers/vfio/mdev/Makefile >>> create mode 100644 drivers/vfio/mdev/mdev_core.c >>> create mode 100644 drivers/vfio/mdev/mdev_driver.c >>> create mode 100644 drivers/vfio/mdev/mdev_private.h >>> create mode 100644 drivers/vfio/mdev/mdev_sysfs.c >>> create mode 100644 drivers/vfio/mdev/vfio_mdev.c >>> create mode 100644 include/linux/mdev.h >>> create mode 100644 samples/vfio-mdev/Makefile >>> create mode 100644 samples/vfio-mdev/mtty.c >>> >> >>
[toc] | [prev] | [next] | [standalone]
| From | Alexey Kardashevskiy <aik@ozlabs.ru> |
|---|---|
| Date | 2016-11-07 07:20 +0100 |
| Message-ID | <sALrI-7Ef-9@gated-at.bofh.it> |
| In reply to | #1515839 |
On 07/11/16 14:59, Kirti Wankhede wrote: > > > On 11/7/2016 9:00 AM, Alexey Kardashevskiy wrote: >> On 05/11/16 08:10, Kirti Wankhede wrote: >>> This series adds Mediated device support to Linux host kernel. Purpose >>> of this series is to provide a common interface for mediated device >>> management that can be used by different devices. This series introduces >>> Mdev core module that creates and manages mediated devices, VFIO based >>> driver for mediated devices that are created by mdev core module and >>> update VFIO type1 IOMMU module to support pinning & unpinning for mediated >>> devices. >>> >>> What changed in v11? >>> mdev core: >>> Register mdev_bus class when first device is registed to avoid panic if >>> any vendor driver and mdev driver are selected as built-in but vendor >>> driver loads first and then mdev module. >>> vfio_mdev: >>> Added notifier callback function to mdev parent's ops so that notifer >>> is registered from vfio_mdev module during device open and unregistered >>> it from device close call. This is a optional callback. Some drivers >>> using mdev framework might not pin or unpin pages, for example the >>> sample mtty driver that simulates serial port. Vendor driver who need to >>> pin/unpin pages should provide this callback. Otherwise pin request >>> would fail. >>> vfio_iommu_type1: >>> Updated to keep track of who (task and address space) mapped iova range. >>> During DMA_UNMAP, same task who mapped it or other task who shares same >>> address space is allowed to unmap, otherwise unmap fails. >>> QEMU maps few iova ranges initially, then fork threads and from the child >>> thread calls DMA_UNMAP on previously mapped iova. Since child shares same >>> address space, DMA_UNMAP is successful. >>> Address space keeps track of pages pinned (pfn_list) by external user / >>> mdev devices. This pfn_list is used to verify pfn during unpin_request, >>> re-accounting of pages when direct device assigned in hot-unplugged and >>> mdev device is present in same container. >>> When the container is released, all mapped iova from all tasks are >>> unmapped and removed. >>> >>> Tested by assigning below combinations of devices to a single VM: >>> - GPU pass through only >>> - vGPU device only >>> - One GPU pass through and one vGPU device >>> - Linux VM hot plug and unplug vGPU device while GPU pass through device >>> exist >>> - Linux VM hot plug and unplug GPU pass through device while vGPU device >>> exist >>> >>> Patch series tested with linux-next upto commit 14970f204b19 @Fri Oct 28 >>> Resolved against conflicting change: >> >> Patch 11/22 does not apply if the patchset is applied on top of 14970f204b19. >> >> > > Pull commit 05692d7005a3 (below) commit. Nah, it was my fault, thunderbird's plugin saved mails in wrong order, false alarm, sorry. Speaking of order, 11/22 is better be squashed into 02/22 (as it does not seem to make sense alone; it also does not depend on something between 02/22 and 11/22); and both 01/22 and 02/22 better be moved where "docs" patches start as they do not makes sense earlier anyway, or do they? > >> >>> 05692d7005a3 vfio/pci: Fix integer overflows, bitmask check >>> > > Thanks, > Kirti > >>> >>> Kirti Wankhede (22): >>> vfio: Mediated device Core driver >>> vfio: VFIO based driver for Mediated devices >>> vfio: Rearrange functions to get vfio_group from dev >>> vfio: Common function to increment container_users >>> vfio iommu: Added pin and unpin callback functions to >>> vfio_iommu_driver_ops >>> vfio iommu type1: Update arguments of vfio_lock_acct >>> vfio iommu type1: Update argument of vaddr_get_pfn() >>> vfio iommu type1: Add find_iommu_group() function >>> vfio iommu type1: Add task structure to vfio_dma >>> vfio iommu type1: Add support for mediated devices >>> vfio iommu: Add blocking notifier to notify DMA_UNMAP >>> vfio: Add notifier callback to parent's ops structure of mdev >>> vfio: Introduce common function to add capabilities >>> vfio_pci: Update vfio_pci to use vfio_info_add_capability() >>> vfio: Introduce vfio_set_irqs_validate_and_prepare() >>> vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare() >>> vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare() >>> vfio: Define device_api strings >>> docs: Add Documentation for Mediated devices >>> docs: Sysfs ABI for mediated device framework >>> docs: Sample driver to demonstrate how to use Mediated device >>> framework. >>> MAINTAINERS: Add entry VFIO based Mediated device drivers >>> >>> Documentation/ABI/testing/sysfs-bus-vfio-mdev | 111 ++ >>> Documentation/vfio-mediated-device.txt | 399 +++++++ >>> MAINTAINERS | 9 + >>> drivers/vfio/Kconfig | 1 + >>> drivers/vfio/Makefile | 1 + >>> drivers/vfio/mdev/Kconfig | 17 + >>> drivers/vfio/mdev/Makefile | 5 + >>> drivers/vfio/mdev/mdev_core.c | 388 +++++++ >>> drivers/vfio/mdev/mdev_driver.c | 122 ++ >>> drivers/vfio/mdev/mdev_private.h | 41 + >>> drivers/vfio/mdev/mdev_sysfs.c | 286 +++++ >>> drivers/vfio/mdev/vfio_mdev.c | 167 +++ >>> drivers/vfio/pci/vfio_pci.c | 83 +- >>> drivers/vfio/platform/vfio_platform_common.c | 31 +- >>> drivers/vfio/vfio.c | 334 +++++- >>> drivers/vfio/vfio_iommu_type1.c | 831 ++++++++++++-- >>> include/linux/mdev.h | 176 +++ >>> include/linux/vfio.h | 32 +- >>> include/uapi/linux/vfio.h | 10 + >>> samples/vfio-mdev/Makefile | 13 + >>> samples/vfio-mdev/mtty.c | 1503 +++++++++++++++++++++++++ >>> 21 files changed, 4342 insertions(+), 218 deletions(-) >>> create mode 100644 Documentation/ABI/testing/sysfs-bus-vfio-mdev >>> create mode 100644 Documentation/vfio-mediated-device.txt >>> create mode 100644 drivers/vfio/mdev/Kconfig >>> create mode 100644 drivers/vfio/mdev/Makefile >>> create mode 100644 drivers/vfio/mdev/mdev_core.c >>> create mode 100644 drivers/vfio/mdev/mdev_driver.c >>> create mode 100644 drivers/vfio/mdev/mdev_private.h >>> create mode 100644 drivers/vfio/mdev/mdev_sysfs.c >>> create mode 100644 drivers/vfio/mdev/vfio_mdev.c >>> create mode 100644 include/linux/mdev.h >>> create mode 100644 samples/vfio-mdev/Makefile >>> create mode 100644 samples/vfio-mdev/mtty.c >>> >> >> -- Alexey
[toc] | [prev] | [next] | [standalone]
| From | Kirti Wankhede <kwankhede@nvidia.com> |
|---|---|
| Date | 2016-11-07 07:40 +0100 |
| Message-ID | <sALL4-7Kp-23@gated-at.bofh.it> |
| In reply to | #1515859 |
On 11/7/2016 11:45 AM, Alexey Kardashevskiy wrote: > On 07/11/16 14:59, Kirti Wankhede wrote: >> >> >> On 11/7/2016 9:00 AM, Alexey Kardashevskiy wrote: >>> On 05/11/16 08:10, Kirti Wankhede wrote: ... >>>> >>>> Patch series tested with linux-next upto commit 14970f204b19 @Fri Oct 28 >>>> Resolved against conflicting change: >>> >>> Patch 11/22 does not apply if the patchset is applied on top of 14970f204b19. >>> >>> >> >> Pull commit 05692d7005a3 (below) commit. > > Nah, it was my fault, thunderbird's plugin saved mails in wrong order, > false alarm, sorry. > > Speaking of order, 11/22 is better be squashed into 02/22 (as it does not > seem to make sense alone; it also does not depend on something between > 02/22 and 11/22); and both 01/22 and 02/22 better be moved where "docs" > patches start as they do not makes sense earlier anyway, or do they? > > There are dependencies in the patch series: Patch 10/22 depends on patch 01/22 and patch 03/22 to 09/22 patch. Patch 02/22 depends on patch 01/22. Patch 11/22 doesn't make sense without patch 10/22. Patch 12/22 depends on patch 11/22. Patch 13/22 to 18/22 are independent, pulling common code in functions. But patch 21/22 uses these APIs in sample driver, so has to be before that. Patch 19/22 adds basic documentation. Patch 21/22 depends on 19/22 patch because that adds sample driver and its explanation to documentation. Thanks, Kirti
[toc] | [prev] | [next] | [standalone]
Page 3 of 4 — ← Prev page 1 2 [3] 4 Next page →
Back to top | Article view | linux.kernel
csiph-web