Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1515412 > unrolled thread

[PATCH v11 00/22] Add Mediated device support

Started byKirti Wankhede <kwankhede@nvidia.com>
First post2016-11-04 22:10 +0100
Last post2016-11-07 07:50 +0100
Articles 20 on this page of 61 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [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 1 of 4  [1] 2 3 4  Next page →


#1515412 — [PATCH v11 00/22] Add Mediated device support

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:10 +0100
Subject[PATCH v11 00/22] Add Mediated device support
Message-ID<szTUm-6Y7-25@gated-at.bofh.it>
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:
  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

-- 
2.7.0

[toc] | [next] | [standalone]


#1515413 — [PATCH v11 12/22] vfio: Add notifier callback to parent's ops structure of mdev

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 12/22] vfio: Add notifier callback to parent's ops structure of mdev
Message-ID<szU41-71A-1@gated-at.bofh.it>
In reply to#1515412
Add a notifier calback to parent's ops structure of mdev device so that per
device notifer for vfio module is registered through vfio_mdev module.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: Iafa6f1721aecdd6e50eb93b153b5621e6d29b637
---
 drivers/vfio/mdev/vfio_mdev.c | 19 +++++++++++++++++++
 include/linux/mdev.h          |  9 +++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
index bb534d19e321..2b7c24aa9e46 100644
--- a/drivers/vfio/mdev/vfio_mdev.c
+++ b/drivers/vfio/mdev/vfio_mdev.c
@@ -24,6 +24,15 @@
 #define DRIVER_AUTHOR   "NVIDIA Corporation"
 #define DRIVER_DESC     "VFIO based driver for Mediated device"
 
+static int vfio_mdev_notifier(struct notifier_block *nb, unsigned long action,
+			      void *data)
+{
+	struct mdev_device *mdev = container_of(nb, struct mdev_device, nb);
+	struct parent_device *parent = mdev->parent;
+
+	return parent->ops->notifier(mdev, action, data);
+}
+
 static int vfio_mdev_open(void *device_data)
 {
 	struct mdev_device *mdev = device_data;
@@ -40,6 +49,11 @@ static int vfio_mdev_open(void *device_data)
 	if (ret)
 		module_put(THIS_MODULE);
 
+	if (likely(parent->ops->notifier)) {
+		mdev->nb.notifier_call = vfio_mdev_notifier;
+		if (vfio_register_notifier(&mdev->dev, &mdev->nb))
+			pr_err("Failed to register notifier for mdev\n");
+	}
 	return ret;
 }
 
@@ -48,6 +62,11 @@ static void vfio_mdev_release(void *device_data)
 	struct mdev_device *mdev = device_data;
 	struct parent_device *parent = mdev->parent;
 
+	if (likely(parent->ops->notifier)) {
+		if (vfio_unregister_notifier(&mdev->dev, &mdev->nb))
+			pr_err("Failed to unregister notifier for mdev\n");
+	}
+
 	if (likely(parent->ops->release))
 		parent->ops->release(mdev);
 
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 0352febc1944..2999ef0ddaed 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -37,6 +37,7 @@ struct mdev_device {
 	struct kref		ref;
 	struct list_head	next;
 	struct kobject		*type_kobj;
+	struct notifier_block	nb;
 };
 
 
@@ -84,6 +85,12 @@ struct mdev_device {
  *			@cmd: mediated device structure
  *			@arg: mediated device structure
  * @mmap:		mmap callback
+ *			@mdev: mediated device structure
+ *			@vma: vma structure
+ * @notifer:		Notifier callback
+ *			@mdev: mediated device structure
+ *			@action: Action for which notifier is called
+ *			@data: Data associated with the notifier
  * Parent device that support mediated device should be registered with mdev
  * module with parent_ops structure.
  **/
@@ -105,6 +112,8 @@ struct parent_ops {
 	ssize_t (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
 			 unsigned long arg);
 	int	(*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
+	int	(*notifier)(struct mdev_device *mdev, unsigned long action,
+			    void *data);
 };
 
 /* interface for exporting mdev supported type attributes */
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1516688 — Re: [PATCH v11 12/22] vfio: Add notifier callback to parent's ops structure of mdev

FromAlex Williamson <alex.williamson@redhat.com>
Date2016-11-08 01:00 +0100
SubjectRe: [PATCH v11 12/22] vfio: Add notifier callback to parent's ops structure of mdev
Message-ID<sB1Zw-1Es-9@gated-at.bofh.it>
In reply to#1515413
On Sat, 5 Nov 2016 02:40:46 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> Add a notifier calback to parent's ops structure of mdev device so that per
> device notifer for vfio module is registered through vfio_mdev module.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: Iafa6f1721aecdd6e50eb93b153b5621e6d29b637
> ---
>  drivers/vfio/mdev/vfio_mdev.c | 19 +++++++++++++++++++
>  include/linux/mdev.h          |  9 +++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
> index bb534d19e321..2b7c24aa9e46 100644
> --- a/drivers/vfio/mdev/vfio_mdev.c
> +++ b/drivers/vfio/mdev/vfio_mdev.c
> @@ -24,6 +24,15 @@
>  #define DRIVER_AUTHOR   "NVIDIA Corporation"
>  #define DRIVER_DESC     "VFIO based driver for Mediated device"
>  
> +static int vfio_mdev_notifier(struct notifier_block *nb, unsigned long action,
> +			      void *data)
> +{
> +	struct mdev_device *mdev = container_of(nb, struct mdev_device, nb);
> +	struct parent_device *parent = mdev->parent;
> +
> +	return parent->ops->notifier(mdev, action, data);
> +}
> +
>  static int vfio_mdev_open(void *device_data)
>  {
>  	struct mdev_device *mdev = device_data;
> @@ -40,6 +49,11 @@ static int vfio_mdev_open(void *device_data)
>  	if (ret)
>  		module_put(THIS_MODULE);
>  
> +	if (likely(parent->ops->notifier)) {
> +		mdev->nb.notifier_call = vfio_mdev_notifier;
> +		if (vfio_register_notifier(&mdev->dev, &mdev->nb))
> +			pr_err("Failed to register notifier for mdev\n");
> +	}
>  	return ret;
>  }
>  
> @@ -48,6 +62,11 @@ static void vfio_mdev_release(void *device_data)
>  	struct mdev_device *mdev = device_data;
>  	struct parent_device *parent = mdev->parent;
>  
> +	if (likely(parent->ops->notifier)) {
> +		if (vfio_unregister_notifier(&mdev->dev, &mdev->nb))
> +			pr_err("Failed to unregister notifier for mdev\n");
> +	}
> +

Ok, I guess this is sufficient to automatically handle the unregister
at the mdev layer.  No need for my comments on the previous other
than the ordering of when the callback is called.  Thanks,

Alex

>  	if (likely(parent->ops->release))
>  		parent->ops->release(mdev);
>  
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index 0352febc1944..2999ef0ddaed 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -37,6 +37,7 @@ struct mdev_device {
>  	struct kref		ref;
>  	struct list_head	next;
>  	struct kobject		*type_kobj;
> +	struct notifier_block	nb;
>  };
>  
>  
> @@ -84,6 +85,12 @@ struct mdev_device {
>   *			@cmd: mediated device structure
>   *			@arg: mediated device structure
>   * @mmap:		mmap callback
> + *			@mdev: mediated device structure
> + *			@vma: vma structure
> + * @notifer:		Notifier callback
> + *			@mdev: mediated device structure
> + *			@action: Action for which notifier is called
> + *			@data: Data associated with the notifier
>   * Parent device that support mediated device should be registered with mdev
>   * module with parent_ops structure.
>   **/
> @@ -105,6 +112,8 @@ struct parent_ops {
>  	ssize_t (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
>  			 unsigned long arg);
>  	int	(*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
> +	int	(*notifier)(struct mdev_device *mdev, unsigned long action,
> +			    void *data);
>  };
>  
>  /* interface for exporting mdev supported type attributes */

[toc] | [prev] | [next] | [standalone]


#1515414 — [PATCH v11 06/22] vfio iommu type1: Update arguments of vfio_lock_acct

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 06/22] vfio iommu type1: Update arguments of vfio_lock_acct
Message-ID<szU41-71A-3@gated-at.bofh.it>
In reply to#1515412
Updated arguments of vfio_lock_acct to take mm structure as input argument

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I5d3673cc9d3786bb436b395d5f74537f1a36da80
---
 drivers/vfio/vfio_iommu_type1.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 7fb87f008e0a..02b302d0b7de 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -150,17 +150,16 @@ static void vfio_lock_acct_bg(struct work_struct *work)
 	kfree(vwork);
 }
 
-static void vfio_lock_acct(long npage)
+static void vfio_lock_acct(struct mm_struct *mm, long npage)
 {
 	struct vwork *vwork;
-	struct mm_struct *mm;
 
-	if (!current->mm || !npage)
+	if (!mm || !npage)
 		return; /* process exited or nothing to do */
 
-	if (down_write_trylock(&current->mm->mmap_sem)) {
-		current->mm->locked_vm += npage;
-		up_write(&current->mm->mmap_sem);
+	if (down_write_trylock(&mm->mmap_sem)) {
+		mm->locked_vm += npage;
+		up_write(&mm->mmap_sem);
 		return;
 	}
 
@@ -172,8 +171,7 @@ static void vfio_lock_acct(long npage)
 	vwork = kmalloc(sizeof(struct vwork), GFP_KERNEL);
 	if (!vwork)
 		return;
-	mm = get_task_mm(current);
-	if (!mm) {
+	if (!mmget_not_zero(mm)) {
 		kfree(vwork);
 		return;
 	}
@@ -285,7 +283,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
 
 	if (unlikely(disable_hugepages)) {
 		if (!rsvd)
-			vfio_lock_acct(1);
+			vfio_lock_acct(current->mm, 1);
 		return 1;
 	}
 
@@ -313,7 +311,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
 	}
 
 	if (!rsvd)
-		vfio_lock_acct(i);
+		vfio_lock_acct(current->mm, i);
 
 	return i;
 }
@@ -328,7 +326,7 @@ static long __vfio_unpin_pages_remote(unsigned long pfn, long npage,
 		unlocked += put_pfn(pfn++, prot);
 
 	if (do_accounting)
-		vfio_lock_acct(-unlocked);
+		vfio_lock_acct(current->mm, -unlocked);
 
 	return unlocked;
 }
@@ -390,7 +388,7 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
 		cond_resched();
 	}
 
-	vfio_lock_acct(-unlocked);
+	vfio_lock_acct(current->mm, -unlocked);
 }
 
 static void vfio_remove_dma(struct vfio_iommu *iommu, struct vfio_dma *dma)
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1515415 — [PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare()

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare()
Message-ID<szU41-71A-13@gated-at.bofh.it>
In reply to#1515412
Updated vfio_platform_common.c file to use
vfio_set_irqs_validate_and_prepare()

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: Id87cd6b78ae901610b39bf957974baa6f40cd7b0
---
 drivers/vfio/platform/vfio_platform_common.c | 31 +++++++---------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index d78142830754..4c27f4be3c3d 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -364,36 +364,21 @@ static long vfio_platform_ioctl(void *device_data,
 		struct vfio_irq_set hdr;
 		u8 *data = NULL;
 		int ret = 0;
+		size_t data_size = 0;
 
 		minsz = offsetofend(struct vfio_irq_set, count);
 
 		if (copy_from_user(&hdr, (void __user *)arg, minsz))
 			return -EFAULT;
 
-		if (hdr.argsz < minsz)
-			return -EINVAL;
-
-		if (hdr.index >= vdev->num_irqs)
-			return -EINVAL;
-
-		if (hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
-				  VFIO_IRQ_SET_ACTION_TYPE_MASK))
-			return -EINVAL;
-
-		if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) {
-			size_t size;
-
-			if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL)
-				size = sizeof(uint8_t);
-			else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD)
-				size = sizeof(int32_t);
-			else
-				return -EINVAL;
-
-			if (hdr.argsz - minsz < size)
-				return -EINVAL;
+		ret = vfio_set_irqs_validate_and_prepare(&hdr, vdev->num_irqs,
+						 vdev->num_irqs, &data_size);
+		if (ret)
+			return ret;
 
-			data = memdup_user((void __user *)(arg + minsz), size);
+		if (data_size) {
+			data = memdup_user((void __user *)(arg + minsz),
+					    data_size);
 			if (IS_ERR(data))
 				return PTR_ERR(data);
 		}
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1517008 — Re: [PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare()

FromAlexey Kardashevskiy <aik@ozlabs.ru>
Date2016-11-08 11:00 +0100
SubjectRe: [PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare()
Message-ID<sBbma-7Et-19@gated-at.bofh.it>
In reply to#1515415
On 05/11/16 08:10, Kirti Wankhede wrote:
> Updated vfio_platform_common.c file to use
> vfio_set_irqs_validate_and_prepare()
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: Id87cd6b78ae901610b39bf957974baa6f40cd7b0
> ---
>  drivers/vfio/platform/vfio_platform_common.c | 31 +++++++---------------------
>  1 file changed, 8 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index d78142830754..4c27f4be3c3d 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -364,36 +364,21 @@ static long vfio_platform_ioctl(void *device_data,
>  		struct vfio_irq_set hdr;
>  		u8 *data = NULL;
>  		int ret = 0;
> +		size_t data_size = 0;
>  
>  		minsz = offsetofend(struct vfio_irq_set, count);
>  
>  		if (copy_from_user(&hdr, (void __user *)arg, minsz))
>  			return -EFAULT;
>  
> -		if (hdr.argsz < minsz)
> -			return -EINVAL;
> -
> -		if (hdr.index >= vdev->num_irqs)
> -			return -EINVAL;
> -
> -		if (hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
> -				  VFIO_IRQ_SET_ACTION_TYPE_MASK))
> -			return -EINVAL;
> -
> -		if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) {
> -			size_t size;
> -
> -			if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL)
> -				size = sizeof(uint8_t);
> -			else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD)
> -				size = sizeof(int32_t);
> -			else
> -				return -EINVAL;
> -
> -			if (hdr.argsz - minsz < size)
> -				return -EINVAL;
> +		ret = vfio_set_irqs_validate_and_prepare(&hdr, vdev->num_irqs,
> +						 vdev->num_irqs, &data_size);

The patch does not change this but I am still curious:

is not the second vdev->num_irqs supposed to be one of
VFIO_PCI_INTX_IRQ_INDEX..VFIO_PCI_NUM_IRQS, not the actual number of
interrupt vectors (as in vfio-pci)?




> +		if (ret)
> +			return ret;
>  
> -			data = memdup_user((void __user *)(arg + minsz), size);
> +		if (data_size) {
> +			data = memdup_user((void __user *)(arg + minsz),
> +					    data_size);
>  			if (IS_ERR(data))
>  				return PTR_ERR(data);
>  		}
> 


-- 
Alexey

[toc] | [prev] | [next] | [standalone]


#1517569 — Re: [PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare()

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-08 21:50 +0100
SubjectRe: [PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare()
Message-ID<sBlvb-5XM-11@gated-at.bofh.it>
In reply to#1517008

On 11/8/2016 2:22 PM, Alexey Kardashevskiy wrote:
> On 05/11/16 08:10, Kirti Wankhede wrote:
>> Updated vfio_platform_common.c file to use
>> vfio_set_irqs_validate_and_prepare()
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>> Change-Id: Id87cd6b78ae901610b39bf957974baa6f40cd7b0
>> ---
>>  drivers/vfio/platform/vfio_platform_common.c | 31 +++++++---------------------
>>  1 file changed, 8 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
>> index d78142830754..4c27f4be3c3d 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -364,36 +364,21 @@ static long vfio_platform_ioctl(void *device_data,
>>  		struct vfio_irq_set hdr;
>>  		u8 *data = NULL;
>>  		int ret = 0;
>> +		size_t data_size = 0;
>>  
>>  		minsz = offsetofend(struct vfio_irq_set, count);
>>  
>>  		if (copy_from_user(&hdr, (void __user *)arg, minsz))
>>  			return -EFAULT;
>>  
>> -		if (hdr.argsz < minsz)
>> -			return -EINVAL;
>> -
>> -		if (hdr.index >= vdev->num_irqs)
>> -			return -EINVAL;
>> -
>> -		if (hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
>> -				  VFIO_IRQ_SET_ACTION_TYPE_MASK))
>> -			return -EINVAL;
>> -
>> -		if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) {
>> -			size_t size;
>> -
>> -			if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL)
>> -				size = sizeof(uint8_t);
>> -			else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD)
>> -				size = sizeof(int32_t);
>> -			else
>> -				return -EINVAL;
>> -
>> -			if (hdr.argsz - minsz < size)
>> -				return -EINVAL;
>> +		ret = vfio_set_irqs_validate_and_prepare(&hdr, vdev->num_irqs,
>> +						 vdev->num_irqs, &data_size);
> 
> The patch does not change this but I am still curious:
> 
> is not the second vdev->num_irqs supposed to be one of
> VFIO_PCI_INTX_IRQ_INDEX..VFIO_PCI_NUM_IRQS, not the actual number of
> interrupt vectors (as in vfio-pci)?
> 
> 

Those are PCI specific. I don't think those counts are applicable here.

If you see the prototype, second argument and third argument have
different meaning.

int vfio_set_irqs_validate_and_prepare(struct vfio_irq_set *hdr, int
num_irqs, int max_irq_type, size_t *data_size)

- num_irqs are number of irqs caller want to setup and
- max_irq_type is the one which is return to user in
VFIO_DEVICE_GET_INFO ioctl's info.num_irqs.

For platform these two are same.

Thanks,
Kirti

> 
> 
>> +		if (ret)
>> +			return ret;
>>  
>> -			data = memdup_user((void __user *)(arg + minsz), size);
>> +		if (data_size) {
>> +			data = memdup_user((void __user *)(arg + minsz),
>> +					    data_size);
>>  			if (IS_ERR(data))
>>  				return PTR_ERR(data);
>>  		}
>>
> 
> 

[toc] | [prev] | [next] | [standalone]


#1515416 — [PATCH v11 20/22] docs: Sysfs ABI for mediated device framework

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 20/22] docs: Sysfs ABI for mediated device framework
Message-ID<szU41-71A-7@gated-at.bofh.it>
In reply to#1515412
Added details of sysfs ABI for mediated device framework

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: Icb0fd4ed58a2fa793fbcb1c3d5009a4403c1f3ac
---
 Documentation/ABI/testing/sysfs-bus-vfio-mdev | 111 ++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-vfio-mdev

diff --git a/Documentation/ABI/testing/sysfs-bus-vfio-mdev b/Documentation/ABI/testing/sysfs-bus-vfio-mdev
new file mode 100644
index 000000000000..452dbe39270e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-vfio-mdev
@@ -0,0 +1,111 @@
+What:           /sys/.../<device>/mdev_supported_types/
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+                This directory contains list of directories of currently
+		supported mediated device types and their details for
+		<device>. Supported type attributes are defined by the
+		vendor driver who registers with Mediated device framework.
+		Each supported type is a directory whose name is created
+		by adding the device driver string as a prefix to the
+		string provided by the vendor driver.
+
+What:           /sys/.../<device>/mdev_supported_types/<type-id>/
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+                This directory gives details of supported type, like name,
+		description, available_instances, device_api etc.
+		'device_api' and 'available_instances' are mandatory
+		attributes to be provided by vendor driver. 'name',
+		'description' and other vendor driver specific attributes
+		are optional.
+
+What:           /sys/.../mdev_supported_types/<type-id>/create
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		Writing UUID to this file will create mediated device of
+		type <type-id> for parent device <device>. This is a
+		write-only file.
+		For example:
+		# echo "83b8f4f2-509f-382f-3c1e-e6bfe0fa1001" >	\
+		       /sys/devices/foo/mdev_supported_types/foo-1/create
+
+What:           /sys/.../mdev_supported_types/<type-id>/devices/
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		This directory contains symbolic links pointing to mdev
+		devices sysfs entries which are created of this <type-id>.
+
+What:           /sys/.../mdev_supported_types/<type-id>/available_instances
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		Reading this attribute will show the number of mediated
+		devices of type <type-id> that can be created. This is a
+		readonly file.
+Users:
+		Userspace applications interested in creating mediated
+		device of that type. Userspace application should check
+		the number of available instances could be created before
+		creating mediated device of this type.
+
+What:           /sys/.../mdev_supported_types/<type-id>/device_api
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		Reading this attribute will show VFIO device API supported
+		by this type. For example, "vfio-pci" for a PCI device,
+		"vfio-platform" for platform device.
+
+What:           /sys/.../mdev_supported_types/<type-id>/name
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		Reading this attribute will show human readable name of the
+		mediated device that will get created of type <type-id>.
+		This is optional attribute. For example: "Grid M60-0Q"
+Users:
+		Userspace applications interested in knowing the name of
+		a particular <type-id> that can help in understanding the
+		type of mediated device.
+
+What:           /sys/.../mdev_supported_types/<type-id>/description
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		Reading this attribute will show description of the type of
+		mediated device that will get created of type <type-id>.
+		This is optional attribute. For example:
+		"2 heads, 512M FB, 2560x1600 maximum resolution"
+Users:
+		Userspace applications interested in knowing the details of
+		a particular <type-id> that can help in understanding the
+		features provided by that type of mediated device.
+
+What:           /sys/.../<device>/<UUID>/
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		This directory represents device directory of mediated
+		device. It contains all the attributes related to mediated
+		device.
+
+What:           /sys/.../<device>/<UUID>/mdev_type
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		This is symbolic link pointing to supported type, <type-id>
+		directory of which this mediated device is created.
+
+What:           /sys/.../<device>/<UUID>/remove
+Date:           October 2016
+Contact:        Kirti Wankhede <kwankhede@nvidia.com>
+Description:
+		Writing '1' to this file destroys the mediated device. The
+		vendor driver can fail the remove() callback if that device
+		is active and the vendor driver doesn't support hot unplug.
+		Example:
+		# echo 1 > /sys/bus/mdev/devices/<UUID>/remove
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1515417 — [PATCH v11 16/22] vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare()

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 16/22] vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare()
Message-ID<szU41-71A-19@gated-at.bofh.it>
In reply to#1515412
Updated vfio_pci.c file to use vfio_set_irqs_validate_and_prepare()

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I9f3daba89d8dba5cb5b01a8cff420412f30686c7
---
 drivers/vfio/pci/vfio_pci.c | 34 +++++++---------------------------
 1 file changed, 7 insertions(+), 27 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 03b5434f4d5b..dcd7c2a99618 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -818,45 +818,25 @@ static long vfio_pci_ioctl(void *device_data,
 
 	} else if (cmd == VFIO_DEVICE_SET_IRQS) {
 		struct vfio_irq_set hdr;
-		size_t size;
 		u8 *data = NULL;
 		int max, ret = 0;
+		size_t data_size = 0;
 
 		minsz = offsetofend(struct vfio_irq_set, count);
 
 		if (copy_from_user(&hdr, (void __user *)arg, minsz))
 			return -EFAULT;
 
-		if (hdr.argsz < minsz || hdr.index >= VFIO_PCI_NUM_IRQS ||
-		    hdr.count >= (U32_MAX - hdr.start) ||
-		    hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
-				  VFIO_IRQ_SET_ACTION_TYPE_MASK))
-			return -EINVAL;
-
 		max = vfio_pci_get_irq_count(vdev, hdr.index);
-		if (hdr.start >= max || hdr.start + hdr.count > max)
-			return -EINVAL;
 
-		switch (hdr.flags & VFIO_IRQ_SET_DATA_TYPE_MASK) {
-		case VFIO_IRQ_SET_DATA_NONE:
-			size = 0;
-			break;
-		case VFIO_IRQ_SET_DATA_BOOL:
-			size = sizeof(uint8_t);
-			break;
-		case VFIO_IRQ_SET_DATA_EVENTFD:
-			size = sizeof(int32_t);
-			break;
-		default:
-			return -EINVAL;
-		}
-
-		if (size) {
-			if (hdr.argsz - minsz < hdr.count * size)
-				return -EINVAL;
+		ret = vfio_set_irqs_validate_and_prepare(&hdr, max,
+						 VFIO_PCI_NUM_IRQS, &data_size);
+		if (ret)
+			return ret;
 
+		if (data_size) {
 			data = memdup_user((void __user *)(arg + minsz),
-					   hdr.count * size);
+					    data_size);
 			if (IS_ERR(data))
 				return PTR_ERR(data);
 		}
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1515418 — [PATCH v11 04/22] vfio: Common function to increment container_users

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 04/22] vfio: Common function to increment container_users
Message-ID<szU41-71A-9@gated-at.bofh.it>
In reply to#1515412
This change rearrange functions to have common function to increment
container_users

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I8bdeb352bc8439b107ffd519480fd4dc238677f2
---
 drivers/vfio/vfio.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 23bc86c1d05d..2e83bdf007fe 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1385,6 +1385,23 @@ static bool vfio_group_viable(struct vfio_group *group)
 					 group, vfio_dev_viable) == 0);
 }
 
+static int vfio_group_add_container_user(struct vfio_group *group)
+{
+	if (!atomic_inc_not_zero(&group->container_users))
+		return -EINVAL;
+
+	if (group->noiommu) {
+		atomic_dec(&group->container_users);
+		return -EPERM;
+	}
+	if (!group->container->iommu_driver || !vfio_group_viable(group)) {
+		atomic_dec(&group->container_users);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static const struct file_operations vfio_device_fops;
 
 static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
@@ -1694,23 +1711,14 @@ static const struct file_operations vfio_device_fops = {
 struct vfio_group *vfio_group_get_external_user(struct file *filep)
 {
 	struct vfio_group *group = filep->private_data;
+	int ret;
 
 	if (filep->f_op != &vfio_group_fops)
 		return ERR_PTR(-EINVAL);
 
-	if (!atomic_inc_not_zero(&group->container_users))
-		return ERR_PTR(-EINVAL);
-
-	if (group->noiommu) {
-		atomic_dec(&group->container_users);
-		return ERR_PTR(-EPERM);
-	}
-
-	if (!group->container->iommu_driver ||
-			!vfio_group_viable(group)) {
-		atomic_dec(&group->container_users);
-		return ERR_PTR(-EINVAL);
-	}
+	ret = vfio_group_add_container_user(group);
+	if (ret)
+		return ERR_PTR(ret);
 
 	vfio_group_get(group);
 
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1515419 — [PATCH v11 19/22] docs: Add Documentation for Mediated devices

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 19/22] docs: Add Documentation for Mediated devices
Message-ID<szU41-71A-21@gated-at.bofh.it>
In reply to#1515412
Add file Documentation/vfio-mediated-device.txt that include details of
mediated device framework.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I137dd646442936090d92008b115908b7b2c7bc5d
---
 Documentation/vfio-mediated-device.txt | 298 +++++++++++++++++++++++++++++++++
 1 file changed, 298 insertions(+)
 create mode 100644 Documentation/vfio-mediated-device.txt

diff --git a/Documentation/vfio-mediated-device.txt b/Documentation/vfio-mediated-device.txt
new file mode 100644
index 000000000000..d61e95aec961
--- /dev/null
+++ b/Documentation/vfio-mediated-device.txt
@@ -0,0 +1,298 @@
+/*
+ * VFIO 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.
+ */
+
+Virtual Function I/O (VFIO) Mediated devices[1]
+===============================================
+
+The number of use cases for virtualizing DMA devices that do not have built-in
+SR_IOV capability is increasing. Previously, to virtualize such devices,
+developers had to create their own management interfaces and APIs, and then
+integrate them with user space software. To simplify integration with user space
+software, we have identified common requirements and a unified management
+interface for such devices.
+
+The VFIO driver framework provides unified APIs for direct device access. It is
+an IOMMU/device-agnostic framework for exposing direct device access to user
+space in a secure, IOMMU-protected environment. This framework is used for
+multiple devices, such as GPUs, network adapters, and compute accelerators. With
+direct device access, virtual machines or user space applications have direct
+access to the physical device. This framework is reused for mediated devices.
+
+The mediated core driver provides a common interface for mediated device
+management that can be used by drivers of different devices. This module
+provides a generic interface to perform these operations:
+
+* Create and destroy a mediated device
+* Add a mediated device to and remove it from a mediated bus driver
+* Add a mediated device to and remove it from an IOMMU group
+
+The mediated core driver also provides an interface to register a bus driver.
+For example, the mediated VFIO mdev driver is designed for mediated devices and
+supports VFIO APIs. The mediated bus driver adds a mediated device to and
+removes it from a VFIO group.
+
+The following high-level block diagram shows the main components and interfaces
+in the VFIO mediated driver framework. The diagram shows NVIDIA, Intel, and IBM
+devices as examples, as these devices are the first devices to use this module.
+
+     +---------------+
+     |               |
+     | +-----------+ |  mdev_register_driver() +--------------+
+     | |           | +<------------------------+              |
+     | |  mdev     | |                         |              |
+     | |  bus      | +------------------------>+ vfio_mdev.ko |<-> VFIO user
+     | |  driver   | |     probe()/remove()    |              |    APIs
+     | |           | |                         +--------------+
+     | +-----------+ |
+     |               |
+     |  MDEV CORE    |
+     |   MODULE      |
+     |   mdev.ko     |
+     | +-----------+ |  mdev_register_device() +--------------+
+     | |           | +<------------------------+              |
+     | |           | |                         |  nvidia.ko   |<-> physical
+     | |           | +------------------------>+              |    device
+     | |           | |        callbacks        +--------------+
+     | | Physical  | |
+     | |  device   | |  mdev_register_device() +--------------+
+     | | interface | |<------------------------+              |
+     | |           | |                         |  i915.ko     |<-> physical
+     | |           | +------------------------>+              |    device
+     | |           | |        callbacks        +--------------+
+     | |           | |
+     | |           | |  mdev_register_device() +--------------+
+     | |           | +<------------------------+              |
+     | |           | |                         | ccw_device.ko|<-> physical
+     | |           | +------------------------>+              |    device
+     | |           | |        callbacks        +--------------+
+     | +-----------+ |
+     +---------------+
+
+
+Registration Interfaces
+=======================
+
+The mediated core driver provides the following types of registration
+interfaces:
+
+* Registration interface for a mediated bus driver
+* Physical device driver interface
+
+Registration Interface for a Mediated Bus Driver
+------------------------------------------------
+
+The registration interface for a mediated bus driver provides the following
+structure to represent a mediated device's driver:
+
+     /*
+      * struct mdev_driver [2] - 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;
+     };
+
+A mediated bus driver for mdev should use this structure in the function calls
+to register and unregister itself with the core driver:
+
+* Register:
+
+  extern int  mdev_register_driver(struct mdev_driver *drv,
+				   struct module *owner);
+
+* Unregister:
+
+  extern void mdev_unregister_driver(struct mdev_driver *drv);
+
+The mediated bus driver is responsible for adding mediated devices to the VFIO
+group when devices are bound to the driver and removing mediated devices from
+the VFIO when devices are unbound from the driver.
+
+
+Physical Device Driver Interface
+--------------------------------
+
+The physical device driver interface provides the parent_ops[3] structure to
+define the APIs to manage work in the mediated core driver that is related to
+the physical device.
+
+The structures in the parent_ops structure are as follows:
+
+* dev_attr_groups: attributes of the parent device
+* mdev_attr_groups: attributes of the mediated device
+* supported_config: attributes to define supported configurations
+
+The functions in the parent_ops structure are as follows:
+
+* create: allocate basic resources in a driver for a mediated device
+* remove: free resources in a driver when a mediated device is destroyed
+
+The callbacks in the parent_ops structure are as follows:
+
+* open: open callback of mediated device
+* close: close callback of mediated device
+* ioctl: ioctl callback of mediated device
+* read : read emulation callback
+* write: write emulation callback
+* mmap: mmap emulation callback
+* notifier: notifier callback
+
+A driver should use the parent_ops structure in the function call to register
+itself with the mdev core driver:
+
+extern int  mdev_register_device(struct device *dev,
+                                 const struct parent_ops *ops);
+
+However, the parent_ops structure is not required in the function call that a
+driver should use to unregister itself with the mdev core driver:
+
+extern void mdev_unregister_device(struct device *dev);
+
+
+Mediated Device Management Interface Through sysfs
+==================================================
+
+The management interface through sysfs enables user space software, such as
+libvirt, to query and configure mediated devices in a hardware-agnostic fashion.
+This management interface provides flexibility to the underlying physical
+device's driver to support features such as:
+
+* Mediated device hot plug
+* Multiple mediated devices in a single virtual machine
+* Multiple mediated devices from different physical devices
+
+Links in the mdev_bus Class Directory
+-------------------------------------
+The /sys/class/mdev_bus/ directory contains links to devices that are registered
+with the mdev core driver.
+
+Directories and files under the sysfs for Each Physical Device
+--------------------------------------------------------------
+
+|- [parent physical device]
+|--- Vendor-specific-attributes [optional]
+|--- [mdev_supported_types]
+|     |--- [<type-id>]
+|     |   |--- create
+|     |   |--- name
+|     |   |--- available_instances
+|     |   |--- device_api
+|     |   |--- description
+|     |   |--- [devices]
+|     |--- [<type-id>]
+|     |   |--- create
+|     |   |--- name
+|     |   |--- available_instances
+|     |   |--- device_api
+|     |   |--- description
+|     |   |--- [devices]
+|     |--- [<type-id>]
+|          |--- create
+|          |--- name
+|          |--- available_instances
+|          |--- device_api
+|          |--- description
+|          |--- [devices]
+
+* [mdev_supported_types]
+
+  The list of currently supported mediated device types and their details.
+
+  [<type-id>], device_api, and available_instances are mandatory attributes
+  that should be provided by vendor driver.
+
+* [<type-id>]
+
+  The [<type-id>] name is created by adding the the device driver string as a
+  prefix to the string provided by the vendor driver. This format of this name
+  is as follows:
+
+	sprintf(buf, "%s-%s", dev_driver_string(parent->dev), group->name);
+
+* device_api
+
+  This attribute should show which device API is being created, for example,
+  "vfio-pci" for a PCI device.
+
+* available_instances
+
+  This attribute should show the number of devices of type <type-id> that can be
+  created.
+
+* [device]
+
+  This directory contains links to the devices of type <type-id> that have been
+created.
+
+* name
+
+  This attribute should show human readable name. This is optional attribute.
+
+* description
+
+  This attribute should show brief features/description of the type. This is
+  optional attribute.
+
+Directories and Files Under the sysfs for Each mdev Device
+----------------------------------------------------------
+
+|- [parent phy device]
+|--- [$MDEV_UUID]
+         |--- remove
+         |--- mdev_type {link to its type}
+         |--- vendor-specific-attributes [optional]
+
+* remove (write only)
+Writing '1' to the 'remove' file destroys the mdev device. The vendor driver can
+fail the remove() callback if that device is active and the vendor driver
+doesn't support hot unplug.
+
+Example:
+	# echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
+
+Mediated device Hot plug:
+------------------------
+
+Mediated devices can be created and assigned at runtime. The procedure to hot
+plug a mediated device is the same as the procedure to hot plug a PCI device.
+
+Translation APIs for Mediated Devices
+=====================================
+
+The following APIs are provided for translating user pfn to host pfn in a VFIO
+driver:
+
+ extern long vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
+                            long npage, int prot, unsigned long *phys_pfn);
+
+ extern long vfio_unpin_pages(struct device *dev, unsigned long *pfn,
+			      long npage);
+
+These functions call back into the back-end IOMMU module by using the pin_pages
+and unpin_pages callbacks of the struct vfio_iommu_driver_ops[4]. Currently
+these callbacks are supported in the TYPE1 IOMMU module. To enable them for
+other IOMMU backend modules, such as PPC64 sPAPR module, they need to provide
+these two callback functions.
+
+References
+----------
+
+[1] See Documentation/vfio.txt for more information on VFIO.
+[2] struct mdev_driver in include/linux/mdev.h
+[3] struct parent_ops in include/linux/mdev.h
+[4] struct vfio_iommu_driver_ops in include/linux/vfio.h
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1515420 — [PATCH v11 13/22] vfio: Introduce common function to add capabilities

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 13/22] vfio: Introduce common function to add capabilities
Message-ID<szU41-71A-27@gated-at.bofh.it>
In reply to#1515412
Vendor driver using mediated device framework should use
vfio_info_add_capability() to add capabilities.
Introduced this function to reduce code duplication in vendor drivers.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I6fca329fa2291f37a2c859d0bc97574d9e2ce1a6
---
 drivers/vfio/vfio.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/vfio.h |  3 +++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 4ed1a6a247c6..9a03be0942a1 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1797,8 +1797,66 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
 	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next - offset)
 		tmp->next += offset;
 }
-EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
+EXPORT_SYMBOL(vfio_info_cap_shift);
 
+static int sparse_mmap_cap(struct vfio_info_cap *caps, void *cap_type)
+{
+	struct vfio_info_cap_header *header;
+	struct vfio_region_info_cap_sparse_mmap *sparse_cap, *sparse = cap_type;
+	size_t size;
+
+	size = sizeof(*sparse) + sparse->nr_areas *  sizeof(*sparse->areas);
+	header = vfio_info_cap_add(caps, size,
+				   VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
+	if (IS_ERR(header))
+		return PTR_ERR(header);
+
+	sparse_cap = container_of(header,
+			struct vfio_region_info_cap_sparse_mmap, header);
+	sparse_cap->nr_areas = sparse->nr_areas;
+	memcpy(sparse_cap->areas, sparse->areas,
+	       sparse->nr_areas * sizeof(*sparse->areas));
+	return 0;
+}
+
+static int region_type_cap(struct vfio_info_cap *caps, void *cap_type)
+{
+	struct vfio_info_cap_header *header;
+	struct vfio_region_info_cap_type *type_cap, *cap = cap_type;
+
+	header = vfio_info_cap_add(caps, sizeof(*cap),
+				   VFIO_REGION_INFO_CAP_TYPE, 1);
+	if (IS_ERR(header))
+		return PTR_ERR(header);
+
+	type_cap = container_of(header, struct vfio_region_info_cap_type,
+				header);
+	type_cap->type = cap->type;
+	type_cap->subtype = cap->subtype;
+	return 0;
+}
+
+int vfio_info_add_capability(struct vfio_info_cap *caps, int cap_type_id,
+			     void *cap_type)
+{
+	int ret = -EINVAL;
+
+	if (!cap_type)
+		return 0;
+
+	switch (cap_type_id) {
+	case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
+		ret = sparse_mmap_cap(caps, cap_type);
+		break;
+
+	case VFIO_REGION_INFO_CAP_TYPE:
+		ret = region_type_cap(caps, cap_type);
+		break;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(vfio_info_add_capability);
 
 /*
  * Pin a set of guest PFNs and return their associated host PFNs for local
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index dcda8fccefab..cf90393a11e2 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -113,6 +113,9 @@ extern struct vfio_info_cap_header *vfio_info_cap_add(
 		struct vfio_info_cap *caps, size_t size, u16 id, u16 version);
 extern void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset);
 
+extern int vfio_info_add_capability(struct vfio_info_cap *caps,
+				    int cap_type_id, void *cap_type);
+
 struct pci_dev;
 #ifdef CONFIG_EEH
 extern void vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1516901 — Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities

FromAlexey Kardashevskiy <aik@ozlabs.ru>
Date2016-11-08 08:30 +0100
SubjectRe: [PATCH v11 13/22] vfio: Introduce common function to add capabilities
Message-ID<sB90Z-6iJ-15@gated-at.bofh.it>
In reply to#1515420
On 05/11/16 08:10, Kirti Wankhede wrote:
> Vendor driver using mediated device framework should use
> vfio_info_add_capability() to add capabilities.
> Introduced this function to reduce code duplication in vendor drivers.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: I6fca329fa2291f37a2c859d0bc97574d9e2ce1a6
> ---
>  drivers/vfio/vfio.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/vfio.h |  3 +++
>  2 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 4ed1a6a247c6..9a03be0942a1 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1797,8 +1797,66 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
>  	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next - offset)
>  		tmp->next += offset;
>  }
> -EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
> +EXPORT_SYMBOL(vfio_info_cap_shift);


Why this change?


>  
> +static int sparse_mmap_cap(struct vfio_info_cap *caps, void *cap_type)
> +{
> +	struct vfio_info_cap_header *header;
> +	struct vfio_region_info_cap_sparse_mmap *sparse_cap, *sparse = cap_type;
> +	size_t size;
> +
> +	size = sizeof(*sparse) + sparse->nr_areas *  sizeof(*sparse->areas);
> +	header = vfio_info_cap_add(caps, size,
> +				   VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
> +	if (IS_ERR(header))
> +		return PTR_ERR(header);
> +
> +	sparse_cap = container_of(header,
> +			struct vfio_region_info_cap_sparse_mmap, header);
> +	sparse_cap->nr_areas = sparse->nr_areas;
> +	memcpy(sparse_cap->areas, sparse->areas,
> +	       sparse->nr_areas * sizeof(*sparse->areas));
> +	return 0;
> +}
> +
> +static int region_type_cap(struct vfio_info_cap *caps, void *cap_type)
> +{
> +	struct vfio_info_cap_header *header;
> +	struct vfio_region_info_cap_type *type_cap, *cap = cap_type;
> +
> +	header = vfio_info_cap_add(caps, sizeof(*cap),
> +				   VFIO_REGION_INFO_CAP_TYPE, 1);
> +	if (IS_ERR(header))
> +		return PTR_ERR(header);
> +
> +	type_cap = container_of(header, struct vfio_region_info_cap_type,
> +				header);
> +	type_cap->type = cap->type;
> +	type_cap->subtype = cap->subtype;
> +	return 0;
> +}
> +
> +int vfio_info_add_capability(struct vfio_info_cap *caps, int cap_type_id,
> +			     void *cap_type)
> +{
> +	int ret = -EINVAL;
> +
> +	if (!cap_type)
> +		return 0;
> +
> +	switch (cap_type_id) {
> +	case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
> +		ret = sparse_mmap_cap(caps, cap_type);
> +		break;
> +
> +	case VFIO_REGION_INFO_CAP_TYPE:
> +		ret = region_type_cap(caps, cap_type);
> +		break;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(vfio_info_add_capability);
>  
>  /*
>   * Pin a set of guest PFNs and return their associated host PFNs for local
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index dcda8fccefab..cf90393a11e2 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -113,6 +113,9 @@ extern struct vfio_info_cap_header *vfio_info_cap_add(
>  		struct vfio_info_cap *caps, size_t size, u16 id, u16 version);
>  extern void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset);
>  
> +extern int vfio_info_add_capability(struct vfio_info_cap *caps,
> +				    int cap_type_id, void *cap_type);
> +


It would make it easier to review and bisect if 14/22 was squashed into
this one. In the resulting patch, vfio_info_cap_add() can be made static as
it will only be used in drivers/vfio/vfio.c from now.




>  struct pci_dev;
>  #ifdef CONFIG_EEH
>  extern void vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
> 


-- 
Alexey

[toc] | [prev] | [next] | [standalone]


#1517577 — Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-08 21:50 +0100
SubjectRe: [PATCH v11 13/22] vfio: Introduce common function to add capabilities
Message-ID<sBlvb-5XM-15@gated-at.bofh.it>
In reply to#1516901

On 11/8/2016 12:59 PM, Alexey Kardashevskiy wrote:
> On 05/11/16 08:10, Kirti Wankhede wrote:
>> Vendor driver using mediated device framework should use
>> vfio_info_add_capability() to add capabilities.
>> Introduced this function to reduce code duplication in vendor drivers.
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>> Change-Id: I6fca329fa2291f37a2c859d0bc97574d9e2ce1a6
>> ---
>>  drivers/vfio/vfio.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  include/linux/vfio.h |  3 +++
>>  2 files changed, 62 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>> index 4ed1a6a247c6..9a03be0942a1 100644
>> --- a/drivers/vfio/vfio.c
>> +++ b/drivers/vfio/vfio.c
>> @@ -1797,8 +1797,66 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
>>  	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next - offset)
>>  		tmp->next += offset;
>>  }
>> -EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
>> +EXPORT_SYMBOL(vfio_info_cap_shift);
> 
> 
> Why this change?
> 
> 

We want this symbol to be available to all drivers.


>>  
>> +static int sparse_mmap_cap(struct vfio_info_cap *caps, void *cap_type)
>> +{
>> +	struct vfio_info_cap_header *header;
>> +	struct vfio_region_info_cap_sparse_mmap *sparse_cap, *sparse = cap_type;
>> +	size_t size;
>> +
>> +	size = sizeof(*sparse) + sparse->nr_areas *  sizeof(*sparse->areas);
>> +	header = vfio_info_cap_add(caps, size,
>> +				   VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
>> +	if (IS_ERR(header))
>> +		return PTR_ERR(header);
>> +
>> +	sparse_cap = container_of(header,
>> +			struct vfio_region_info_cap_sparse_mmap, header);
>> +	sparse_cap->nr_areas = sparse->nr_areas;
>> +	memcpy(sparse_cap->areas, sparse->areas,
>> +	       sparse->nr_areas * sizeof(*sparse->areas));
>> +	return 0;
>> +}
>> +
>> +static int region_type_cap(struct vfio_info_cap *caps, void *cap_type)
>> +{
>> +	struct vfio_info_cap_header *header;
>> +	struct vfio_region_info_cap_type *type_cap, *cap = cap_type;
>> +
>> +	header = vfio_info_cap_add(caps, sizeof(*cap),
>> +				   VFIO_REGION_INFO_CAP_TYPE, 1);
>> +	if (IS_ERR(header))
>> +		return PTR_ERR(header);
>> +
>> +	type_cap = container_of(header, struct vfio_region_info_cap_type,
>> +				header);
>> +	type_cap->type = cap->type;
>> +	type_cap->subtype = cap->subtype;
>> +	return 0;
>> +}
>> +
>> +int vfio_info_add_capability(struct vfio_info_cap *caps, int cap_type_id,
>> +			     void *cap_type)
>> +{
>> +	int ret = -EINVAL;
>> +
>> +	if (!cap_type)
>> +		return 0;
>> +
>> +	switch (cap_type_id) {
>> +	case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
>> +		ret = sparse_mmap_cap(caps, cap_type);
>> +		break;
>> +
>> +	case VFIO_REGION_INFO_CAP_TYPE:
>> +		ret = region_type_cap(caps, cap_type);
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(vfio_info_add_capability);
>>  
>>  /*
>>   * Pin a set of guest PFNs and return their associated host PFNs for local
>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>> index dcda8fccefab..cf90393a11e2 100644
>> --- a/include/linux/vfio.h
>> +++ b/include/linux/vfio.h
>> @@ -113,6 +113,9 @@ extern struct vfio_info_cap_header *vfio_info_cap_add(
>>  		struct vfio_info_cap *caps, size_t size, u16 id, u16 version);
>>  extern void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset);
>>  
>> +extern int vfio_info_add_capability(struct vfio_info_cap *caps,
>> +				    int cap_type_id, void *cap_type);
>> +
> 
> 
> It would make it easier to review and bisect if 14/22 was squashed into
> this one. 

This was split based on Alex's suggestion on earlier version of this
patchset.

> In the resulting patch, vfio_info_cap_add() can be made static as
> it will only be used in drivers/vfio/vfio.c from now.
> 

Not sure that vfio_info_cap_add() should be made static. If there are
any other drivers using this symbol outside kernel might break with that
change.

Thanks,
Kirti

> 
> 
> 
>>  struct pci_dev;
>>  #ifdef CONFIG_EEH
>>  extern void vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
>>
> 
> 

[toc] | [prev] | [next] | [standalone]


#1517604 — Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities

FromAlex Williamson <alex.williamson@redhat.com>
Date2016-11-08 22:50 +0100
SubjectRe: [PATCH v11 13/22] vfio: Introduce common function to add capabilities
Message-ID<sBmrf-6Av-11@gated-at.bofh.it>
In reply to#1517577
On Wed, 9 Nov 2016 02:16:17 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 11/8/2016 12:59 PM, Alexey Kardashevskiy wrote:
> > On 05/11/16 08:10, Kirti Wankhede wrote:  
> >> Vendor driver using mediated device framework should use
> >> vfio_info_add_capability() to add capabilities.
> >> Introduced this function to reduce code duplication in vendor drivers.
> >>
> >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> >> Signed-off-by: Neo Jia <cjia@nvidia.com>
> >> Change-Id: I6fca329fa2291f37a2c859d0bc97574d9e2ce1a6
> >> ---
> >>  drivers/vfio/vfio.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  include/linux/vfio.h |  3 +++
> >>  2 files changed, 62 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> >> index 4ed1a6a247c6..9a03be0942a1 100644
> >> --- a/drivers/vfio/vfio.c
> >> +++ b/drivers/vfio/vfio.c
> >> @@ -1797,8 +1797,66 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
> >>  	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next - offset)
> >>  		tmp->next += offset;
> >>  }
> >> -EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
> >> +EXPORT_SYMBOL(vfio_info_cap_shift);  
> > 
> > 
> > Why this change?
> > 
> >   
> 
> We want this symbol to be available to all drivers.

IOW, from proprietary drivers.  It makes me uncomfortable how many
non-GPL symbols we're adding (or converting) in this effort, but I'm
trying to look objectively at every export as to whether a non-GPL
caller of the function is legitimately separate from in-kernel code.
For instance are they making use of data structures intrinsic to GPL'd
code.  In this case we're converting a symbol that's just manipulating
a data buffer to add an offset to each element in a chain.  The entries
are documented in a uapi header.  Kirti asked me about this one, and I
couldn't find any basis to raise an objection.  If you spot any reason
that any of the export symbols in these series really should be GPL,
please raise the issue.

> >>  
> >> +static int sparse_mmap_cap(struct vfio_info_cap *caps, void *cap_type)
> >> +{
> >> +	struct vfio_info_cap_header *header;
> >> +	struct vfio_region_info_cap_sparse_mmap *sparse_cap, *sparse = cap_type;
> >> +	size_t size;
> >> +
> >> +	size = sizeof(*sparse) + sparse->nr_areas *  sizeof(*sparse->areas);
> >> +	header = vfio_info_cap_add(caps, size,
> >> +				   VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
> >> +	if (IS_ERR(header))
> >> +		return PTR_ERR(header);
> >> +
> >> +	sparse_cap = container_of(header,
> >> +			struct vfio_region_info_cap_sparse_mmap, header);
> >> +	sparse_cap->nr_areas = sparse->nr_areas;
> >> +	memcpy(sparse_cap->areas, sparse->areas,
> >> +	       sparse->nr_areas * sizeof(*sparse->areas));
> >> +	return 0;
> >> +}
> >> +
> >> +static int region_type_cap(struct vfio_info_cap *caps, void *cap_type)
> >> +{
> >> +	struct vfio_info_cap_header *header;
> >> +	struct vfio_region_info_cap_type *type_cap, *cap = cap_type;
> >> +
> >> +	header = vfio_info_cap_add(caps, sizeof(*cap),
> >> +				   VFIO_REGION_INFO_CAP_TYPE, 1);
> >> +	if (IS_ERR(header))
> >> +		return PTR_ERR(header);
> >> +
> >> +	type_cap = container_of(header, struct vfio_region_info_cap_type,
> >> +				header);
> >> +	type_cap->type = cap->type;
> >> +	type_cap->subtype = cap->subtype;
> >> +	return 0;
> >> +}
> >> +
> >> +int vfio_info_add_capability(struct vfio_info_cap *caps, int cap_type_id,
> >> +			     void *cap_type)
> >> +{
> >> +	int ret = -EINVAL;
> >> +
> >> +	if (!cap_type)
> >> +		return 0;
> >> +
> >> +	switch (cap_type_id) {
> >> +	case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
> >> +		ret = sparse_mmap_cap(caps, cap_type);
> >> +		break;
> >> +
> >> +	case VFIO_REGION_INFO_CAP_TYPE:
> >> +		ret = region_type_cap(caps, cap_type);
> >> +		break;
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL(vfio_info_add_capability);
> >>  
> >>  /*
> >>   * Pin a set of guest PFNs and return their associated host PFNs for local
> >> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> >> index dcda8fccefab..cf90393a11e2 100644
> >> --- a/include/linux/vfio.h
> >> +++ b/include/linux/vfio.h
> >> @@ -113,6 +113,9 @@ extern struct vfio_info_cap_header *vfio_info_cap_add(
> >>  		struct vfio_info_cap *caps, size_t size, u16 id, u16 version);
> >>  extern void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset);
> >>  
> >> +extern int vfio_info_add_capability(struct vfio_info_cap *caps,
> >> +				    int cap_type_id, void *cap_type);
> >> +  
> > 
> > 
> > It would make it easier to review and bisect if 14/22 was squashed into
> > this one.   
> 
> This was split based on Alex's suggestion on earlier version of this
> patchset.

Yeah, generally squashing patches together is the opposite of what we
want for review and bisect.  In this case the symbol exports should
avoid any defined-but-unused warnings.  Thanks,

Alex

[toc] | [prev] | [next] | [standalone]


#1517731 — Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities

FromAlexey Kardashevskiy <aik@ozlabs.ru>
Date2016-11-09 03:30 +0100
SubjectRe: [PATCH v11 13/22] vfio: Introduce common function to add capabilities
Message-ID<sBqOd-19e-3@gated-at.bofh.it>
In reply to#1517604
On 09/11/16 08:42, Alex Williamson wrote:
> On Wed, 9 Nov 2016 02:16:17 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
>> On 11/8/2016 12:59 PM, Alexey Kardashevskiy wrote:
>>> On 05/11/16 08:10, Kirti Wankhede wrote:  
>>>> Vendor driver using mediated device framework should use
>>>> vfio_info_add_capability() to add capabilities.
>>>> Introduced this function to reduce code duplication in vendor drivers.
>>>>
>>>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>>>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>>>> Change-Id: I6fca329fa2291f37a2c859d0bc97574d9e2ce1a6
>>>> ---
>>>>  drivers/vfio/vfio.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>  include/linux/vfio.h |  3 +++
>>>>  2 files changed, 62 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>>>> index 4ed1a6a247c6..9a03be0942a1 100644
>>>> --- a/drivers/vfio/vfio.c
>>>> +++ b/drivers/vfio/vfio.c
>>>> @@ -1797,8 +1797,66 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
>>>>  	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next - offset)
>>>>  		tmp->next += offset;
>>>>  }
>>>> -EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
>>>> +EXPORT_SYMBOL(vfio_info_cap_shift);  
>>>
>>>
>>> Why this change?
>>>
>>>   
>>
>> We want this symbol to be available to all drivers.
> 
> IOW, from proprietary drivers.  It makes me uncomfortable how many
> non-GPL symbols we're adding (or converting) in this effort, but I'm
> trying to look objectively at every export as to whether a non-GPL
> caller of the function is legitimately separate from in-kernel code.
> For instance are they making use of data structures intrinsic to GPL'd
> code.  In this case we're converting a symbol that's just manipulating
> a data buffer to add an offset to each element in a chain.  The entries
> are documented in a uapi header.  Kirti asked me about this one, and I
> couldn't find any basis to raise an objection.  If you spot any reason
> that any of the export symbols in these series really should be GPL,
> please raise the issue.
> 
>>>>  
>>>> +static int sparse_mmap_cap(struct vfio_info_cap *caps, void *cap_type)
>>>> +{
>>>> +	struct vfio_info_cap_header *header;
>>>> +	struct vfio_region_info_cap_sparse_mmap *sparse_cap, *sparse = cap_type;
>>>> +	size_t size;
>>>> +
>>>> +	size = sizeof(*sparse) + sparse->nr_areas *  sizeof(*sparse->areas);
>>>> +	header = vfio_info_cap_add(caps, size,
>>>> +				   VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
>>>> +	if (IS_ERR(header))
>>>> +		return PTR_ERR(header);
>>>> +
>>>> +	sparse_cap = container_of(header,
>>>> +			struct vfio_region_info_cap_sparse_mmap, header);
>>>> +	sparse_cap->nr_areas = sparse->nr_areas;
>>>> +	memcpy(sparse_cap->areas, sparse->areas,
>>>> +	       sparse->nr_areas * sizeof(*sparse->areas));
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int region_type_cap(struct vfio_info_cap *caps, void *cap_type)
>>>> +{
>>>> +	struct vfio_info_cap_header *header;
>>>> +	struct vfio_region_info_cap_type *type_cap, *cap = cap_type;
>>>> +
>>>> +	header = vfio_info_cap_add(caps, sizeof(*cap),
>>>> +				   VFIO_REGION_INFO_CAP_TYPE, 1);
>>>> +	if (IS_ERR(header))
>>>> +		return PTR_ERR(header);
>>>> +
>>>> +	type_cap = container_of(header, struct vfio_region_info_cap_type,
>>>> +				header);
>>>> +	type_cap->type = cap->type;
>>>> +	type_cap->subtype = cap->subtype;
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +int vfio_info_add_capability(struct vfio_info_cap *caps, int cap_type_id,
>>>> +			     void *cap_type)
>>>> +{
>>>> +	int ret = -EINVAL;
>>>> +
>>>> +	if (!cap_type)
>>>> +		return 0;
>>>> +
>>>> +	switch (cap_type_id) {
>>>> +	case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
>>>> +		ret = sparse_mmap_cap(caps, cap_type);
>>>> +		break;
>>>> +
>>>> +	case VFIO_REGION_INFO_CAP_TYPE:
>>>> +		ret = region_type_cap(caps, cap_type);
>>>> +		break;
>>>> +	}
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +EXPORT_SYMBOL(vfio_info_add_capability);
>>>>  
>>>>  /*
>>>>   * Pin a set of guest PFNs and return their associated host PFNs for local
>>>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>>>> index dcda8fccefab..cf90393a11e2 100644
>>>> --- a/include/linux/vfio.h
>>>> +++ b/include/linux/vfio.h
>>>> @@ -113,6 +113,9 @@ extern struct vfio_info_cap_header *vfio_info_cap_add(
>>>>  		struct vfio_info_cap *caps, size_t size, u16 id, u16 version);
>>>>  extern void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset);
>>>>  
>>>> +extern int vfio_info_add_capability(struct vfio_info_cap *caps,
>>>> +				    int cap_type_id, void *cap_type);
>>>> +  
>>>
>>>
>>> It would make it easier to review and bisect if 14/22 was squashed into
>>> this one.   
>>
>> This was split based on Alex's suggestion on earlier version of this
>> patchset.


This could have been mentioned in changelog...

> 
> Yeah, generally squashing patches together is the opposite of what we
> want for review and bisect. 


13/22 adds a helper which is not used in it so if there is an error, it
won't be caught by bisecting, bisect will incorrectly point to 14/22.

Also, since the new helper in 13/22 is made from chunks removed in 14/22,
I'd like to see both changes in one patch to make sure that nothing was
lost during cut-n-paste. Especially when it is not just 2 patches, like 3
patches later in the series.

imho splitting like this only makes sense (or, rather, just make life
easier) when patches go via different maintainers trees.

However, since Alex is happy, you can ignore me.


> In this case the symbol exports should
> avoid any defined-but-unused warnings.  Thanks,
> 
> Alex
> 


-- 
Alexey

[toc] | [prev] | [next] | [standalone]


#1515421 — [PATCH v11 03/22] vfio: Rearrange functions to get vfio_group from dev

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 03/22] vfio: Rearrange functions to get vfio_group from dev
Message-ID<szU41-71A-29@gated-at.bofh.it>
In reply to#1515412
This patch rearranges functions to get vfio_group from device

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I1f93262bdbab75094bc24b087b29da35ba70c4c6
---
 drivers/vfio/vfio.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index d1d70e0b011b..23bc86c1d05d 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -480,6 +480,21 @@ static struct vfio_group *vfio_group_get_from_minor(int minor)
 	return group;
 }
 
+static struct vfio_group *vfio_group_get_from_dev(struct device *dev)
+{
+	struct iommu_group *iommu_group;
+	struct vfio_group *group;
+
+	iommu_group = iommu_group_get(dev);
+	if (!iommu_group)
+		return NULL;
+
+	group = vfio_group_get_from_iommu(iommu_group);
+	iommu_group_put(iommu_group);
+
+	return group;
+}
+
 /**
  * Device objects - create, release, get, put, search
  */
@@ -811,16 +826,10 @@ EXPORT_SYMBOL_GPL(vfio_add_group_dev);
  */
 struct vfio_device *vfio_device_get_from_dev(struct device *dev)
 {
-	struct iommu_group *iommu_group;
 	struct vfio_group *group;
 	struct vfio_device *device;
 
-	iommu_group = iommu_group_get(dev);
-	if (!iommu_group)
-		return NULL;
-
-	group = vfio_group_get_from_iommu(iommu_group);
-	iommu_group_put(iommu_group);
+	group = vfio_group_get_from_dev(dev);
 	if (!group)
 		return NULL;
 
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1515422 — [PATCH v11 14/22] vfio_pci: Update vfio_pci to use vfio_info_add_capability()

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 14/22] vfio_pci: Update vfio_pci to use vfio_info_add_capability()
Message-ID<szU42-71A-31@gated-at.bofh.it>
In reply to#1515412
Update msix_sparse_mmap_cap() to use vfio_info_add_capability()
Update region type capability to use vfio_info_add_capability()

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I52bb28c7875a6da5a79ddad1843e6088aff58a45
---
 drivers/vfio/pci/vfio_pci.c | 49 ++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 031bc08d000d..03b5434f4d5b 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -558,10 +558,9 @@ static int vfio_pci_for_each_slot_or_bus(struct pci_dev *pdev,
 static int msix_sparse_mmap_cap(struct vfio_pci_device *vdev,
 				struct vfio_info_cap *caps)
 {
-	struct vfio_info_cap_header *header;
 	struct vfio_region_info_cap_sparse_mmap *sparse;
 	size_t end, size;
-	int nr_areas = 2, i = 0;
+	int nr_areas = 2, i = 0, ret;
 
 	end = pci_resource_len(vdev->pdev, vdev->msix_bar);
 
@@ -572,13 +571,10 @@ static int msix_sparse_mmap_cap(struct vfio_pci_device *vdev,
 
 	size = sizeof(*sparse) + (nr_areas * sizeof(*sparse->areas));
 
-	header = vfio_info_cap_add(caps, size,
-				   VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
-	if (IS_ERR(header))
-		return PTR_ERR(header);
+	sparse = kzalloc(size, GFP_KERNEL);
+	if (!sparse)
+		return -ENOMEM;
 
-	sparse = container_of(header,
-			      struct vfio_region_info_cap_sparse_mmap, header);
 	sparse->nr_areas = nr_areas;
 
 	if (vdev->msix_offset & PAGE_MASK) {
@@ -594,26 +590,11 @@ static int msix_sparse_mmap_cap(struct vfio_pci_device *vdev,
 		i++;
 	}
 
-	return 0;
-}
-
-static int region_type_cap(struct vfio_pci_device *vdev,
-			   struct vfio_info_cap *caps,
-			   unsigned int type, unsigned int subtype)
-{
-	struct vfio_info_cap_header *header;
-	struct vfio_region_info_cap_type *cap;
-
-	header = vfio_info_cap_add(caps, sizeof(*cap),
-				   VFIO_REGION_INFO_CAP_TYPE, 1);
-	if (IS_ERR(header))
-		return PTR_ERR(header);
-
-	cap = container_of(header, struct vfio_region_info_cap_type, header);
-	cap->type = type;
-	cap->subtype = subtype;
+	ret = vfio_info_add_capability(caps, VFIO_REGION_INFO_CAP_SPARSE_MMAP,
+				       sparse);
+	kfree(sparse);
 
-	return 0;
+	return ret;
 }
 
 int vfio_pci_register_dev_region(struct vfio_pci_device *vdev,
@@ -752,6 +733,9 @@ static long vfio_pci_ioctl(void *device_data,
 
 			break;
 		default:
+		{
+			struct vfio_region_info_cap_type cap_type;
+
 			if (info.index >=
 			    VFIO_PCI_NUM_REGIONS + vdev->num_regions)
 				return -EINVAL;
@@ -762,11 +746,16 @@ static long vfio_pci_ioctl(void *device_data,
 			info.size = vdev->region[i].size;
 			info.flags = vdev->region[i].flags;
 
-			ret = region_type_cap(vdev, &caps,
-					      vdev->region[i].type,
-					      vdev->region[i].subtype);
+			cap_type.type = vdev->region[i].type;
+			cap_type.subtype = vdev->region[i].subtype;
+
+			ret = vfio_info_add_capability(&caps,
+						      VFIO_REGION_INFO_CAP_TYPE,
+						      &cap_type);
 			if (ret)
 				return ret;
+
+		}
 		}
 
 		if (caps.size) {
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1515423 — [PATCH v11 09/22] vfio iommu type1: Add task structure to vfio_dma

FromKirti Wankhede <kwankhede@nvidia.com>
Date2016-11-04 22:20 +0100
Subject[PATCH v11 09/22] vfio iommu type1: Add task structure to vfio_dma
Message-ID<szU42-71A-33@gated-at.bofh.it>
In reply to#1515412
Add task structure to vfio_dma.
Add address space structure. Each vfio_dma structure points to the address
space of the task who mapped it.
List of address spaces is maintained in vfio_iommu structure.
From DMA_MAP call if address space already exist in address space list,
vfio_dma points to it. If address space doesn't exist, allocate address
space, save pointer of mm to it and vfio_dma points to it.
Two tasks can share same address space and so we need keep address space
structure different from task in vfio_dma structure. vfio_dma keeps
pointer to its corresponding address space.
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.
This address space structure is used to track pages pinned by external
user in later changes.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I7600f1bea6b384fd589fa72421ccf031bcfd9ac5
---
 drivers/vfio/vfio_iommu_type1.c | 182 +++++++++++++++++++++++++++++-----------
 1 file changed, 134 insertions(+), 48 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 422c8d198abb..8d64528dcc22 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -55,12 +55,20 @@ MODULE_PARM_DESC(disable_hugepages,
 
 struct vfio_iommu {
 	struct list_head	domain_list;
+	struct list_head	addr_space_list;
 	struct mutex		lock;
 	struct rb_root		dma_list;
 	bool			v2;
 	bool			nesting;
 };
 
+/* address space */
+struct vfio_addr_space {
+	struct mm_struct	*mm;
+	struct list_head	next;
+	atomic_t		ref_count;
+};
+
 struct vfio_domain {
 	struct iommu_domain	*domain;
 	struct list_head	next;
@@ -75,6 +83,9 @@ struct vfio_dma {
 	unsigned long		vaddr;		/* Process virtual addr */
 	size_t			size;		/* Map size (bytes) */
 	int			prot;		/* IOMMU_READ/WRITE */
+	struct vfio_addr_space	*addr_space;
+	struct task_struct	*task;
+	bool			mlock_cap;
 };
 
 struct vfio_group {
@@ -130,6 +141,18 @@ static void vfio_unlink_dma(struct vfio_iommu *iommu, struct vfio_dma *old)
 	rb_erase(&old->node, &iommu->dma_list);
 }
 
+static struct vfio_addr_space *vfio_find_addr_space(struct vfio_iommu *iommu,
+						    struct mm_struct *mm)
+{
+	struct vfio_addr_space *as;
+
+	list_for_each_entry(as, &iommu->addr_space_list, next) {
+		if (as->mm == mm)
+			return as;
+	}
+	return NULL;
+}
+
 struct vwork {
 	struct mm_struct	*mm;
 	long			npage;
@@ -273,24 +296,24 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
  * 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_remote(unsigned long vaddr, long npage,
-				    int prot, unsigned long *pfn_base)
+static long __vfio_pin_pages_remote(struct vfio_dma *dma, 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);
+	struct task_struct *task = dma->task;
+	unsigned long limit = task_rlimit(task, RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+	bool lock_cap = dma->mlock_cap;
+	struct mm_struct *mm = dma->addr_space->mm;
 	long ret, i;
 	bool rsvd;
 
-	if (!current->mm)
-		return -ENODEV;
-
-	ret = vaddr_get_pfn(current->mm, vaddr, prot, pfn_base);
+	ret = vaddr_get_pfn(mm, vaddr, prot, pfn_base);
 	if (ret)
 		return ret;
 
 	rsvd = is_invalid_reserved_pfn(*pfn_base);
 
-	if (!rsvd && !lock_cap && current->mm->locked_vm + 1 > limit) {
+	if (!rsvd && !lock_cap && mm->locked_vm + 1 > limit) {
 		put_pfn(*pfn_base, prot);
 		pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n", __func__,
 			limit << PAGE_SHIFT);
@@ -299,7 +322,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
 
 	if (unlikely(disable_hugepages)) {
 		if (!rsvd)
-			vfio_lock_acct(current->mm, 1);
+			vfio_lock_acct(mm, 1);
 		return 1;
 	}
 
@@ -307,7 +330,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
 	for (i = 1, vaddr += PAGE_SIZE; i < npage; i++, vaddr += PAGE_SIZE) {
 		unsigned long pfn = 0;
 
-		ret = vaddr_get_pfn(current->mm, vaddr, prot, &pfn);
+		ret = vaddr_get_pfn(mm, vaddr, prot, &pfn);
 		if (ret)
 			break;
 
@@ -318,7 +341,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
 		}
 
 		if (!rsvd && !lock_cap &&
-		    current->mm->locked_vm + i + 1 > limit) {
+		    mm->locked_vm + i + 1 > limit) {
 			put_pfn(pfn, prot);
 			pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
 				__func__, limit << PAGE_SHIFT);
@@ -327,13 +350,13 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
 	}
 
 	if (!rsvd)
-		vfio_lock_acct(current->mm, i);
+		vfio_lock_acct(mm, i);
 
 	return i;
 }
 
-static long __vfio_unpin_pages_remote(unsigned long pfn, long npage,
-				      int prot, bool do_accounting)
+static long __vfio_unpin_pages_remote(struct vfio_dma *dma, unsigned long pfn,
+				      long npage, int prot, bool do_accounting)
 {
 	unsigned long unlocked = 0;
 	long i;
@@ -342,7 +365,7 @@ static long __vfio_unpin_pages_remote(unsigned long pfn, long npage,
 		unlocked += put_pfn(pfn++, prot);
 
 	if (do_accounting)
-		vfio_lock_acct(current->mm, -unlocked);
+		vfio_lock_acct(dma->addr_space->mm, -unlocked);
 
 	return unlocked;
 }
@@ -396,7 +419,7 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
 		if (WARN_ON(!unmapped))
 			break;
 
-		unlocked += __vfio_unpin_pages_remote(phys >> PAGE_SHIFT,
+		unlocked += __vfio_unpin_pages_remote(dma, phys >> PAGE_SHIFT,
 						      unmapped >> PAGE_SHIFT,
 						      dma->prot, false);
 		iova += unmapped;
@@ -404,13 +427,20 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
 		cond_resched();
 	}
 
-	vfio_lock_acct(current->mm, -unlocked);
+	vfio_lock_acct(dma->addr_space->mm, -unlocked);
 }
 
 static void vfio_remove_dma(struct vfio_iommu *iommu, struct vfio_dma *dma)
 {
 	vfio_unmap_unpin(iommu, dma);
 	vfio_unlink_dma(iommu, dma);
+
+	if (atomic_dec_and_test(&dma->addr_space->ref_count)) {
+		mmput(dma->addr_space->mm);
+		put_task_struct(dma->task);
+		list_del(&dma->addr_space->next);
+		kfree(dma->addr_space);
+	}
 	kfree(dma);
 }
 
@@ -506,6 +536,12 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
 	while ((dma = vfio_find_dma(iommu, unmap->iova, unmap->size))) {
 		if (!iommu->v2 && unmap->iova > dma->iova)
 			break;
+		/*
+		 * Task with same address space who mapped this iova range is
+		 * allowed to unmap the iova range.
+		 */
+		if (dma->task->mm != current->mm)
+			break;
 		unmapped += dma->size;
 		vfio_remove_dma(iommu, dma);
 	}
@@ -572,17 +608,58 @@ unwind:
 	return ret;
 }
 
+static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
+			    size_t map_size)
+{
+	dma_addr_t iova = dma->iova;
+	unsigned long vaddr = dma->vaddr;
+	size_t size = map_size;
+	long npage;
+	unsigned long pfn;
+	int ret = 0;
+
+	while (size) {
+		/* Pin a contiguous chunk of memory */
+		npage = __vfio_pin_pages_remote(dma, vaddr + dma->size,
+						size >> PAGE_SHIFT, dma->prot,
+						&pfn);
+		if (npage <= 0) {
+			WARN_ON(!npage);
+			ret = (int)npage;
+			break;
+		}
+
+		/* Map it! */
+		ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage,
+				     dma->prot);
+		if (ret) {
+			__vfio_unpin_pages_remote(dma, pfn, npage, dma->prot,
+						  true);
+			break;
+		}
+
+		size -= npage << PAGE_SHIFT;
+		dma->size += npage << PAGE_SHIFT;
+	}
+
+	if (ret)
+		vfio_remove_dma(iommu, dma);
+
+	return ret;
+}
+
 static int vfio_dma_do_map(struct vfio_iommu *iommu,
 			   struct vfio_iommu_type1_dma_map *map)
 {
 	dma_addr_t iova = map->iova;
 	unsigned long vaddr = map->vaddr;
 	size_t size = map->size;
-	long npage;
 	int ret = 0, prot = 0;
 	uint64_t mask;
 	struct vfio_dma *dma;
-	unsigned long pfn;
+	struct vfio_addr_space *addr_space;
+	struct mm_struct *mm;
+	bool free_addr_space_on_err = false;
 
 	/* Verify that none of our __u64 fields overflow */
 	if (map->size != size || map->vaddr != vaddr || map->iova != iova)
@@ -608,47 +685,56 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
 	mutex_lock(&iommu->lock);
 
 	if (vfio_find_dma(iommu, iova, size)) {
-		mutex_unlock(&iommu->lock);
-		return -EEXIST;
+		ret = -EEXIST;
+		goto do_map_err;
+	}
+
+	mm = get_task_mm(current);
+	if (!mm) {
+		ret = -ENODEV;
+		goto do_map_err;
+	}
+
+	addr_space = vfio_find_addr_space(iommu, mm);
+	if (addr_space) {
+		atomic_inc(&addr_space->ref_count);
+		mmput(mm);
+	} else {
+		addr_space = kzalloc(sizeof(*addr_space), GFP_KERNEL);
+		if (!addr_space) {
+			ret = -ENOMEM;
+			goto do_map_err;
+		}
+		addr_space->mm = mm;
+		atomic_set(&addr_space->ref_count, 1);
+		list_add(&addr_space->next, &iommu->addr_space_list);
+		free_addr_space_on_err = true;
 	}
 
 	dma = kzalloc(sizeof(*dma), GFP_KERNEL);
 	if (!dma) {
-		mutex_unlock(&iommu->lock);
-		return -ENOMEM;
+		if (free_addr_space_on_err) {
+			mmput(mm);
+			list_del(&addr_space->next);
+			kfree(addr_space);
+		}
+		ret = -ENOMEM;
+		goto do_map_err;
 	}
 
 	dma->iova = iova;
 	dma->vaddr = vaddr;
 	dma->prot = prot;
+	dma->addr_space = addr_space;
+	get_task_struct(current);
+	dma->task = current;
+	dma->mlock_cap = capable(CAP_IPC_LOCK);
 
 	/* Insert zero-sized and grow as we map chunks of it */
 	vfio_link_dma(iommu, dma);
 
-	while (size) {
-		/* Pin a contiguous chunk of memory */
-		npage = __vfio_pin_pages_remote(vaddr + dma->size,
-						size >> PAGE_SHIFT, prot, &pfn);
-		if (npage <= 0) {
-			WARN_ON(!npage);
-			ret = (int)npage;
-			break;
-		}
-
-		/* Map it! */
-		ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage, prot);
-		if (ret) {
-			__vfio_unpin_pages_remote(pfn, npage, prot, true);
-			break;
-		}
-
-		size -= npage << PAGE_SHIFT;
-		dma->size += npage << PAGE_SHIFT;
-	}
-
-	if (ret)
-		vfio_remove_dma(iommu, dma);
-
+	ret = vfio_pin_map_dma(iommu, dma, size);
+do_map_err:
 	mutex_unlock(&iommu->lock);
 	return ret;
 }
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1516584 — Re: [PATCH v11 09/22] vfio iommu type1: Add task structure to vfio_dma

FromAlex Williamson <alex.williamson@redhat.com>
Date2016-11-07 22:10 +0100
SubjectRe: [PATCH v11 09/22] vfio iommu type1: Add task structure to vfio_dma
Message-ID<sAZkZ-8k-1@gated-at.bofh.it>
In reply to#1515423
On Sat, 5 Nov 2016 02:40:43 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> Add task structure to vfio_dma.
> Add address space structure. Each vfio_dma structure points to the address
> space of the task who mapped it.
> List of address spaces is maintained in vfio_iommu structure.
> From DMA_MAP call if address space already exist in address space list,
> vfio_dma points to it. If address space doesn't exist, allocate address
> space, save pointer of mm to it and vfio_dma points to it.
> Two tasks can share same address space and so we need keep address space
> structure different from task in vfio_dma structure. vfio_dma keeps
> pointer to its corresponding address space.
> 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.
> This address space structure is used to track pages pinned by external
> user in later changes.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: I7600f1bea6b384fd589fa72421ccf031bcfd9ac5
> ---
>  drivers/vfio/vfio_iommu_type1.c | 182 +++++++++++++++++++++++++++++-----------
>  1 file changed, 134 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 422c8d198abb..8d64528dcc22 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -55,12 +55,20 @@ MODULE_PARM_DESC(disable_hugepages,
>  
>  struct vfio_iommu {
>  	struct list_head	domain_list;
> +	struct list_head	addr_space_list;
>  	struct mutex		lock;
>  	struct rb_root		dma_list;
>  	bool			v2;
>  	bool			nesting;
>  };
>  
> +/* address space */
> +struct vfio_addr_space {
> +	struct mm_struct	*mm;
> +	struct list_head	next;
> +	atomic_t		ref_count;
> +};
> +
>  struct vfio_domain {
>  	struct iommu_domain	*domain;
>  	struct list_head	next;
> @@ -75,6 +83,9 @@ struct vfio_dma {
>  	unsigned long		vaddr;		/* Process virtual addr */
>  	size_t			size;		/* Map size (bytes) */
>  	int			prot;		/* IOMMU_READ/WRITE */
> +	struct vfio_addr_space	*addr_space;
> +	struct task_struct	*task;
> +	bool			mlock_cap;
>  };
>  
>  struct vfio_group {
> @@ -130,6 +141,18 @@ static void vfio_unlink_dma(struct vfio_iommu *iommu, struct vfio_dma *old)
>  	rb_erase(&old->node, &iommu->dma_list);
>  }
>  
> +static struct vfio_addr_space *vfio_find_addr_space(struct vfio_iommu *iommu,
> +						    struct mm_struct *mm)
> +{
> +	struct vfio_addr_space *as;
> +
> +	list_for_each_entry(as, &iommu->addr_space_list, next) {
> +		if (as->mm == mm)
> +			return as;
> +	}
> +	return NULL;
> +}
> +
>  struct vwork {
>  	struct mm_struct	*mm;
>  	long			npage;
> @@ -273,24 +296,24 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
>   * 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_remote(unsigned long vaddr, long npage,
> -				    int prot, unsigned long *pfn_base)
> +static long __vfio_pin_pages_remote(struct vfio_dma *dma, 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);
> +	struct task_struct *task = dma->task;
> +	unsigned long limit = task_rlimit(task, RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> +	bool lock_cap = dma->mlock_cap;
> +	struct mm_struct *mm = dma->addr_space->mm;
>  	long ret, i;
>  	bool rsvd;
>  
> -	if (!current->mm)
> -		return -ENODEV;
> -
> -	ret = vaddr_get_pfn(current->mm, vaddr, prot, pfn_base);
> +	ret = vaddr_get_pfn(mm, vaddr, prot, pfn_base);
>  	if (ret)
>  		return ret;
>  
>  	rsvd = is_invalid_reserved_pfn(*pfn_base);
>  
> -	if (!rsvd && !lock_cap && current->mm->locked_vm + 1 > limit) {
> +	if (!rsvd && !lock_cap && mm->locked_vm + 1 > limit) {
>  		put_pfn(*pfn_base, prot);
>  		pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n", __func__,
>  			limit << PAGE_SHIFT);
> @@ -299,7 +322,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
>  
>  	if (unlikely(disable_hugepages)) {
>  		if (!rsvd)
> -			vfio_lock_acct(current->mm, 1);
> +			vfio_lock_acct(mm, 1);
>  		return 1;
>  	}
>  
> @@ -307,7 +330,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
>  	for (i = 1, vaddr += PAGE_SIZE; i < npage; i++, vaddr += PAGE_SIZE) {
>  		unsigned long pfn = 0;
>  
> -		ret = vaddr_get_pfn(current->mm, vaddr, prot, &pfn);
> +		ret = vaddr_get_pfn(mm, vaddr, prot, &pfn);
>  		if (ret)
>  			break;
>  
> @@ -318,7 +341,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
>  		}
>  
>  		if (!rsvd && !lock_cap &&
> -		    current->mm->locked_vm + i + 1 > limit) {
> +		    mm->locked_vm + i + 1 > limit) {
>  			put_pfn(pfn, prot);
>  			pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
>  				__func__, limit << PAGE_SHIFT);
> @@ -327,13 +350,13 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
>  	}
>  
>  	if (!rsvd)
> -		vfio_lock_acct(current->mm, i);
> +		vfio_lock_acct(mm, i);
>  
>  	return i;
>  }
>  
> -static long __vfio_unpin_pages_remote(unsigned long pfn, long npage,
> -				      int prot, bool do_accounting)
> +static long __vfio_unpin_pages_remote(struct vfio_dma *dma, unsigned long pfn,
> +				      long npage, int prot, bool do_accounting)
>  {
>  	unsigned long unlocked = 0;
>  	long i;
> @@ -342,7 +365,7 @@ static long __vfio_unpin_pages_remote(unsigned long pfn, long npage,
>  		unlocked += put_pfn(pfn++, prot);
>  
>  	if (do_accounting)
> -		vfio_lock_acct(current->mm, -unlocked);
> +		vfio_lock_acct(dma->addr_space->mm, -unlocked);
>  
>  	return unlocked;
>  }
> @@ -396,7 +419,7 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
>  		if (WARN_ON(!unmapped))
>  			break;
>  
> -		unlocked += __vfio_unpin_pages_remote(phys >> PAGE_SHIFT,
> +		unlocked += __vfio_unpin_pages_remote(dma, phys >> PAGE_SHIFT,
>  						      unmapped >> PAGE_SHIFT,
>  						      dma->prot, false);
>  		iova += unmapped;
> @@ -404,13 +427,20 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
>  		cond_resched();
>  	}
>  
> -	vfio_lock_acct(current->mm, -unlocked);
> +	vfio_lock_acct(dma->addr_space->mm, -unlocked);
>  }
>  
>  static void vfio_remove_dma(struct vfio_iommu *iommu, struct vfio_dma *dma)
>  {
>  	vfio_unmap_unpin(iommu, dma);
>  	vfio_unlink_dma(iommu, dma);
> +
> +	if (atomic_dec_and_test(&dma->addr_space->ref_count)) {
> +		mmput(dma->addr_space->mm);
> +		put_task_struct(dma->task);
> +		list_del(&dma->addr_space->next);
> +		kfree(dma->addr_space);
> +	}
>  	kfree(dma);
>  }
>  
> @@ -506,6 +536,12 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
>  	while ((dma = vfio_find_dma(iommu, unmap->iova, unmap->size))) {
>  		if (!iommu->v2 && unmap->iova > dma->iova)
>  			break;
> +		/*
> +		 * Task with same address space who mapped this iova range is
> +		 * allowed to unmap the iova range.
> +		 */
> +		if (dma->task->mm != current->mm)
> +			break;
>  		unmapped += dma->size;
>  		vfio_remove_dma(iommu, dma);
>  	}
> @@ -572,17 +608,58 @@ unwind:
>  	return ret;
>  }
>  
> +static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
> +			    size_t map_size)
> +{
> +	dma_addr_t iova = dma->iova;
> +	unsigned long vaddr = dma->vaddr;
> +	size_t size = map_size;
> +	long npage;
> +	unsigned long pfn;
> +	int ret = 0;
> +
> +	while (size) {
> +		/* Pin a contiguous chunk of memory */
> +		npage = __vfio_pin_pages_remote(dma, vaddr + dma->size,
> +						size >> PAGE_SHIFT, dma->prot,
> +						&pfn);
> +		if (npage <= 0) {
> +			WARN_ON(!npage);
> +			ret = (int)npage;
> +			break;
> +		}
> +
> +		/* Map it! */
> +		ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage,
> +				     dma->prot);
> +		if (ret) {
> +			__vfio_unpin_pages_remote(dma, pfn, npage, dma->prot,
> +						  true);
> +			break;
> +		}
> +
> +		size -= npage << PAGE_SHIFT;
> +		dma->size += npage << PAGE_SHIFT;
> +	}
> +
> +	if (ret)
> +		vfio_remove_dma(iommu, dma);
> +
> +	return ret;
> +}
> +
>  static int vfio_dma_do_map(struct vfio_iommu *iommu,
>  			   struct vfio_iommu_type1_dma_map *map)
>  {
>  	dma_addr_t iova = map->iova;
>  	unsigned long vaddr = map->vaddr;
>  	size_t size = map->size;
> -	long npage;
>  	int ret = 0, prot = 0;
>  	uint64_t mask;
>  	struct vfio_dma *dma;
> -	unsigned long pfn;
> +	struct vfio_addr_space *addr_space;
> +	struct mm_struct *mm;
> +	bool free_addr_space_on_err = false;
>  
>  	/* Verify that none of our __u64 fields overflow */
>  	if (map->size != size || map->vaddr != vaddr || map->iova != iova)
> @@ -608,47 +685,56 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
>  	mutex_lock(&iommu->lock);
>  
>  	if (vfio_find_dma(iommu, iova, size)) {
> -		mutex_unlock(&iommu->lock);
> -		return -EEXIST;
> +		ret = -EEXIST;
> +		goto do_map_err;
> +	}
> +
> +	mm = get_task_mm(current);
> +	if (!mm) {
> +		ret = -ENODEV;

-EFAULT?

> +		goto do_map_err;
> +	}
> +
> +	addr_space = vfio_find_addr_space(iommu, mm);
> +	if (addr_space) {
> +		atomic_inc(&addr_space->ref_count);
> +		mmput(mm);
> +	} else {
> +		addr_space = kzalloc(sizeof(*addr_space), GFP_KERNEL);
> +		if (!addr_space) {
> +			ret = -ENOMEM;
> +			goto do_map_err;
> +		}
> +		addr_space->mm = mm;
> +		atomic_set(&addr_space->ref_count, 1);
> +		list_add(&addr_space->next, &iommu->addr_space_list);
> +		free_addr_space_on_err = true;
>  	}
>  
>  	dma = kzalloc(sizeof(*dma), GFP_KERNEL);
>  	if (!dma) {
> -		mutex_unlock(&iommu->lock);
> -		return -ENOMEM;
> +		if (free_addr_space_on_err) {
> +			mmput(mm);
> +			list_del(&addr_space->next);
> +			kfree(addr_space);
> +		}
> +		ret = -ENOMEM;
> +		goto do_map_err;
>  	}
>  
>  	dma->iova = iova;
>  	dma->vaddr = vaddr;
>  	dma->prot = prot;
> +	dma->addr_space = addr_space;
> +	get_task_struct(current);
> +	dma->task = current;
> +	dma->mlock_cap = capable(CAP_IPC_LOCK);


How do you reason we can cache this?  Does the fact that the process
had this capability at the time that it did a DMA_MAP imply that it
necessarily still has this capability when an external user (vendor
driver) tries to pin pages?  I don't see how we can make that
assumption.


>  
>  	/* Insert zero-sized and grow as we map chunks of it */
>  	vfio_link_dma(iommu, dma);
>  
> -	while (size) {
> -		/* Pin a contiguous chunk of memory */
> -		npage = __vfio_pin_pages_remote(vaddr + dma->size,
> -						size >> PAGE_SHIFT, prot, &pfn);
> -		if (npage <= 0) {
> -			WARN_ON(!npage);
> -			ret = (int)npage;
> -			break;
> -		}
> -
> -		/* Map it! */
> -		ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage, prot);
> -		if (ret) {
> -			__vfio_unpin_pages_remote(pfn, npage, prot, true);
> -			break;
> -		}
> -
> -		size -= npage << PAGE_SHIFT;
> -		dma->size += npage << PAGE_SHIFT;
> -	}
> -
> -	if (ret)
> -		vfio_remove_dma(iommu, dma);
> -
> +	ret = vfio_pin_map_dma(iommu, dma, size);
> +do_map_err:
>  	mutex_unlock(&iommu->lock);
>  	return ret;
>  }

[toc] | [prev] | [next] | [standalone]


Page 1 of 4  [1] 2 3 4  Next page →

Back to top | Article view | linux.kernel


csiph-web