[RFC 0/3] WIP VFIO for device tree devices on Arndale

[RFC 0/3] WIP VFIO for device tree devices on Arndale

[Antonios Motakis]

This is a very early base work, towards VFIO support on ARM platforms
with an IOMMU. It forms a base on to which to implement the functionality
necessary to enable using device tree devices on ARM (and other platforms
based on device trees) with VFIO.

This very early work of progress is only published for the sake of openness,
and is very far from usable yet. However the driver can bind to devices,
and return to userspace the info about the memory regions and IRQs.

This patch series has been tested on the Arndale board (with the Exynos 5250
System MMU).

It depends on Cho KyongHo's patch series "iommu/exynos: Fixes and Enhancements
of System MMU driver with DT", applied on a Linux 3.10.1 kernel, and also my
own "iommu/exynos: add devices attached to the System MMU to an IOMMU group".
Those patches are required at least in order to test the proposed module on
Arndale.

This should not be treated as anything more than a work in progress. Numerous
functions still need to be implemented properly, e.g.
 - Proper binding of the VFIO_DT driver to devices; currently to test the
   driver, one has to edit the device tree and add the "vfio-dt" to the
   compatible property. However Linux does not support OF drivers that
   can be dynamically bound to any device.
 - Most IOCTLs are not implemented yet. Memory region mapping, DMA mapping,
   IRQFD still need to be added.
 - The VFIO_IOMMU_TYPE1 is patched to work instead of PCI IOMMU, with platform
   IOMMUs such as the one that is found on Arndale. This is a proof of concept
   hack, and a more permanent fix will be proposed as the code matures.

The API used is identical to the existing VFIO API that is also used with
PCI devices. Only devices that include a basic set of IRQs and memory regions
are targeted; devices with complicated relationships with other devices on the
device tree are not taken into account at this stage.

The API is not extended with device tree specific information; this would
complicate the driver unnecessarily as it is not needed for the base use cases.

Antonios Motakis (3):
  VFIO_IOMMU_TYPE1 workaround to build for platform devices
  Initial skeleton of VFIO support for Device Tree based devices
  Return info for device and its memory regions and interrupts

 drivers/vfio/Kconfig            |  12 ++-
 drivers/vfio/Makefile           |   1 +
 drivers/vfio/vfio_dt.c          | 233 ++++++++++++++++++++++++++++++++++++++++
 drivers/vfio/vfio_iommu_type1.c |  15 ++-
 include/uapi/linux/vfio.h       |   1 +
 5 files changed, 258 insertions(+), 4 deletions(-)
 create mode 100644 drivers/vfio/vfio_dt.c

-- 
1.8.1.2

[PATCH 1/3] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Antonios Motakis]

This is a workaround to make the VFIO_IOMMU_TYPE1 driver usable with
platform devices instead of PCI. A future permanent fix should support
both. This is required in order to use the Exynos SMMU, or the ARM SMMU
driver with VFIO.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
---
 drivers/vfio/Kconfig            |  2 +-
 drivers/vfio/vfio_iommu_type1.c | 15 ++++++++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 7cd5dec..1f84eda 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -6,7 +6,7 @@ config VFIO_IOMMU_TYPE1
 menuconfig VFIO
 	tristate "VFIO Non-Privileged userspace driver framework"
 	depends on IOMMU_API
-	select VFIO_IOMMU_TYPE1 if X86
+	select VFIO_IOMMU_TYPE1 if X86 || ARM
 	help
 	  VFIO provides a framework for secure userspace device drivers.
 	  See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 6f3fbc4..4339603 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -30,7 +30,8 @@
 #include <linux/iommu.h>
 #include <linux/module.h>
 #include <linux/mm.h>
-#include <linux/pci.h>		/* pci_bus_type */
+#include <linux/pci.h>			/* pci_bus_type */
+#include <linux/platform_device.h>	/* platform_bus_type */
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
@@ -610,9 +611,17 @@ static void *vfio_iommu_type1_open(unsigned long arg)
 	mutex_init(&iommu->lock);
 
 	/*
+	 * ARM SMMU compatibility workaround
+	 */
+	iommu->domain = iommu_domain_alloc(&platform_bus_type);
+	if (iommu->domain)
+		return iommu;
+
+	/*
 	 * Wish we didn't have to know about bus_type here.
 	 */
-	iommu->domain = iommu_domain_alloc(&pci_bus_type);
+	//iommu->domain = iommu_domain_alloc(&pci_bus_type);
+
 	if (!iommu->domain) {
 		kfree(iommu);
 		return ERR_PTR(-EIO);
@@ -733,7 +742,7 @@ static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
 
 static int __init vfio_iommu_type1_init(void)
 {
-	if (!iommu_present(&pci_bus_type))
+	if (!iommu_present(&platform_bus_type))
 		return -ENODEV;
 
 	return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
-- 
1.8.1.2

[PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

[Antonios Motakis]

Platform devices in the Linux kernel are usually managed by the DT
interface. This patch forms the base to support these kind of devices
with VFIO.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
---
 drivers/vfio/Kconfig      |  10 +++
 drivers/vfio/Makefile     |   1 +
 drivers/vfio/vfio_dt.c    | 187 ++++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h |   1 +
 4 files changed, 199 insertions(+)
 create mode 100644 drivers/vfio/vfio_dt.c

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 1f84eda..a77a2e4 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -13,4 +13,14 @@ menuconfig VFIO
 
 	  If you don't know what to do here, say N.
 
+config VFIO_DT
+	tristate "VFIO support for Device Tree devices"
+	depends on VFIO && EVENTFD
+	help
+	  Support for the VFIO Device Tree driver.  This is required to make
+	  use of platform devices present on Device Tree nodes using the VFIO
+	  framework.
+
+	  If you don't know what to do here, say N.
+
 source "drivers/vfio/pci/Kconfig"
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 2398d4a..d599a67 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_VFIO) += vfio.o
 obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
 obj-$(CONFIG_VFIO_PCI) += pci/
+obj-$(CONFIG_VFIO_DT) += vfio_dt.o
diff --git a/drivers/vfio/vfio_dt.c b/drivers/vfio/vfio_dt.c
new file mode 100644
index 0000000..ad4d31d
--- /dev/null
+++ b/drivers/vfio/vfio_dt.c
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis <a.motakis@virtualopensystems.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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#include <linux/device.h>
+#include <linux/eventfd.h>
+#include <linux/interrupt.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+#include <linux/vfio.h>
+
+#define DRIVER_VERSION  "0.1"
+#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis@virtualopensystems.com>"
+#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
+
+struct vfio_dt_device {
+	struct platform_device	*pdev;
+};
+
+static void vfio_dt_release(void *device_data)
+{
+	module_put(THIS_MODULE);
+}
+
+static int vfio_dt_open(void *device_data)
+{
+	if (!try_module_get(THIS_MODULE))
+		return -ENODEV;
+
+	return 0;
+}
+
+static long vfio_dt_ioctl(void *device_data,
+			   unsigned int cmd, unsigned long arg)
+{
+	struct vfio_dt_device *vdev = device_data;
+	unsigned long minsz;
+
+	if (cmd == VFIO_DEVICE_GET_INFO) {
+		struct vfio_device_info info;
+
+		minsz = offsetofend(struct vfio_device_info, num_irqs);
+
+		if (copy_from_user(&info, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (info.argsz < minsz)
+			return -EINVAL;
+
+		info.flags = VFIO_DEVICE_FLAGS_DT;
+		info.num_regions = 0;
+		info.num_irqs = 0;
+
+		return copy_to_user((void __user *)arg, &info, minsz);
+
+	} else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
+		return -EINVAL;
+
+	else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
+		return -EINVAL;
+
+	else if (cmd == VFIO_DEVICE_SET_IRQS)
+		return -EINVAL;
+
+	else if (cmd == VFIO_DEVICE_RESET)
+		return -EINVAL;
+
+	return -ENOTTY;
+}
+
+static ssize_t vfio_dt_read(void *device_data, char __user *buf,
+			     size_t count, loff_t *ppos)
+{
+	return 0;
+}
+
+static ssize_t vfio_dt_write(void *device_data, const char __user *buf,
+			      size_t count, loff_t *ppos)
+{
+	return 0;
+}
+
+static int vfio_dt_mmap(void *device_data, struct vm_area_struct *vma)
+{
+	return -EINVAL;
+}
+
+static const struct vfio_device_ops vfio_dt_ops = {
+	.name		= "vfio-dts",
+	.open		= vfio_dt_open,
+	.release	= vfio_dt_release,
+	.ioctl		= vfio_dt_ioctl,
+	.read		= vfio_dt_read,
+	.write		= vfio_dt_write,
+	.mmap		= vfio_dt_mmap,
+};
+
+static int vfio_dt_probe(struct platform_device *pdev)
+{
+	struct vfio_dt_device *vdev;
+	struct iommu_group *group;
+	int ret;
+
+	group = iommu_group_get(&pdev->dev);
+	if (!group) {
+		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
+		return -EINVAL;
+	}
+
+	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
+	if (!vdev) {
+		iommu_group_put(group);
+		return -ENOMEM;
+	}
+
+	vdev->pdev = pdev;
+
+	ret = vfio_add_group_dev(&pdev->dev, &vfio_dt_ops, vdev);
+	if (ret) {
+		iommu_group_put(group);
+		kfree(vdev);
+	}
+
+	return ret;
+}
+
+static int vfio_dt_remove(struct platform_device *pdev)
+{
+	struct vfio_dt_device *vdev;
+
+	vdev = vfio_del_group_dev(&pdev->dev);
+	if (!vdev)
+		return -EINVAL;
+
+	iommu_group_put(pdev->dev.iommu_group);
+	kfree(vdev);
+
+	return 0;
+}
+
+static const struct of_device_id vfio_dt_match[] = {
+	/* In the future, we can implement a better mechanism to bind the
+	 * module to any device. For now add the compatible property to the
+	 * dtb of the devices we want to use.	*/
+	{
+		.compatible = "vfio-dt",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, vfio_dt_match);
+
+static struct platform_driver vfio_dt_driver = {
+	.probe		= vfio_dt_probe,
+	.remove		= vfio_dt_remove,
+	.driver	= {
+		.name	= "vfio-dt",
+		.owner	= THIS_MODULE,
+		.of_match_table = vfio_dt_match,
+	},
+};
+
+module_platform_driver(vfio_dt_driver);
+
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 284ff24..1e4bef2 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -147,6 +147,7 @@ struct vfio_device_info {
 	__u32	flags;
 #define VFIO_DEVICE_FLAGS_RESET	(1 << 0)	/* Device supports reset */
 #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
+#define VFIO_DEVICE_FLAGS_DT	(1 << 2)	/* vfio-dt device */
 	__u32	num_regions;	/* Max region index + 1 */
 	__u32	num_irqs;	/* Max IRQ index + 1 */
 };
-- 
1.8.1.2

[PATCH 3/3] Return info for device and its memory regions and interrupts

[Antonios Motakis]

A VFIO userspace driver will start by opening the VFIO device
that corresponds to an IOMMU group, and will use the ioctl interface
to get the basic device info, such as number of memory regions and
interrupts, and their properties.

This patch implements the IOCTLs:
 - VFIO_DEVICE_GET_INFO
 - VFIO_DEVICE_GET_REGION_INFO
 - VFIO_DEVICE_GET_IRQ_INFO

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
---
 drivers/vfio/vfio_dt.c | 60 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/vfio_dt.c b/drivers/vfio/vfio_dt.c
index ad4d31d..817c552 100644
--- a/drivers/vfio/vfio_dt.c
+++ b/drivers/vfio/vfio_dt.c
@@ -28,6 +28,10 @@
 #include <linux/types.h>
 #include <linux/uaccess.h>
 #include <linux/vfio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
 
 #define DRIVER_VERSION  "0.1"
 #define DRIVER_AUTHOR   "Antonios Motakis <a.motakis@virtualopensystems.com>"
@@ -54,10 +58,13 @@ static long vfio_dt_ioctl(void *device_data,
 			   unsigned int cmd, unsigned long arg)
 {
 	struct vfio_dt_device *vdev = device_data;
+	struct device_node *of_node = vdev->pdev->dev.of_node;
 	unsigned long minsz;
 
 	if (cmd == VFIO_DEVICE_GET_INFO) {
 		struct vfio_device_info info;
+		struct resource res;
+		int cnt = 0;
 
 		minsz = offsetofend(struct vfio_device_info, num_irqs);
 
@@ -68,18 +75,57 @@ static long vfio_dt_ioctl(void *device_data,
 			return -EINVAL;
 
 		info.flags = VFIO_DEVICE_FLAGS_DT;
-		info.num_regions = 0;
-		info.num_irqs = 0;
+
+		while (!of_address_to_resource(of_node, cnt, &res))
+			cnt++;
+
+		info.num_regions = cnt;
+
+		info.num_irqs = of_irq_count(of_node);
 
 		return copy_to_user((void __user *)arg, &info, minsz);
 
-	} else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
-		return -EINVAL;
+	} else if (cmd == VFIO_DEVICE_GET_REGION_INFO) {
+		struct vfio_region_info info;
+		struct resource res;
 
-	else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
-		return -EINVAL;
+		minsz = offsetofend(struct vfio_region_info, offset);
+
+		if (copy_from_user(&info, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (info.argsz < minsz)
+			return -EINVAL;
+
+		if(of_address_to_resource(of_node, info.index, &res))
+			return -EINVAL;
+
+		info.offset = res.start;	/* map phys addr with offset */
+		info.size = resource_size(&res);
+		info.flags = 0;
+
+		return copy_to_user((void __user *)arg, &info, minsz);	
+
+	} else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
+		struct vfio_irq_info info;
+		struct resource res;
+
+		minsz = offsetofend(struct vfio_irq_info, count);
+
+		if (copy_from_user(&info, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (info.argsz < minsz)
+			return -EINVAL;
+
+		of_irq_to_resource(of_node, info.index, &res);
+
+		info.flags = 0;
+		info.count = 1;
+
+		return copy_to_user((void __user *)arg, &info, minsz);
 
-	else if (cmd == VFIO_DEVICE_SET_IRQS)
+	} else if (cmd == VFIO_DEVICE_SET_IRQS)
 		return -EINVAL;
 
 	else if (cmd == VFIO_DEVICE_RESET)
-- 
1.8.1.2

[PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

[Mark Rutland]

[adding DT maintainers to Cc]

On Mon, Aug 05, 2013 at 02:17:11PM +0100, Antonios Motakis wrote:
> Platform devices in the Linux kernel are usually managed by the DT
> interface. This patch forms the base to support these kind of devices
> with VFIO.
> 
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> ---
>  drivers/vfio/Kconfig      |  10 +++
>  drivers/vfio/Makefile     |   1 +
>  drivers/vfio/vfio_dt.c    | 187 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vfio.h |   1 +
>  4 files changed, 199 insertions(+)
>  create mode 100644 drivers/vfio/vfio_dt.c
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 1f84eda..a77a2e4 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -13,4 +13,14 @@ menuconfig VFIO
>  
>  	  If you don't know what to do here, say N.
>  
> +config VFIO_DT
> +	tristate "VFIO support for Device Tree devices"
> +	depends on VFIO && EVENTFD
> +	help
> +	  Support for the VFIO Device Tree driver.  This is required to make
> +	  use of platform devices present on Device Tree nodes using the VFIO
> +	  framework.
> +
> +	  If you don't know what to do here, say N.
> +
>  source "drivers/vfio/pci/Kconfig"
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index 2398d4a..d599a67 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_VFIO) += vfio.o
>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
>  obj-$(CONFIG_VFIO_PCI) += pci/
> +obj-$(CONFIG_VFIO_DT) += vfio_dt.o
> diff --git a/drivers/vfio/vfio_dt.c b/drivers/vfio/vfio_dt.c
> new file mode 100644
> index 0000000..ad4d31d
> --- /dev/null
> +++ b/drivers/vfio/vfio_dt.c
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (C) 2013 - Virtual Open Systems
> + * Author: Antonios Motakis <a.motakis@virtualopensystems.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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/eventfd.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +#include <linux/vfio.h>
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis@virtualopensystems.com>"
> +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
> +
> +struct vfio_dt_device {
> +	struct platform_device	*pdev;
> +};
> +
> +static void vfio_dt_release(void *device_data)
> +{
> +	module_put(THIS_MODULE);
> +}
> +
> +static int vfio_dt_open(void *device_data)
> +{
> +	if (!try_module_get(THIS_MODULE))
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static long vfio_dt_ioctl(void *device_data,
> +			   unsigned int cmd, unsigned long arg)
> +{
> +	struct vfio_dt_device *vdev = device_data;
> +	unsigned long minsz;
> +
> +	if (cmd == VFIO_DEVICE_GET_INFO) {
> +		struct vfio_device_info info;
> +
> +		minsz = offsetofend(struct vfio_device_info, num_irqs);
> +
> +		if (copy_from_user(&info, (void __user *)arg, minsz))
> +			return -EFAULT;
> +
> +		if (info.argsz < minsz)
> +			return -EINVAL;
> +
> +		info.flags = VFIO_DEVICE_FLAGS_DT;
> +		info.num_regions = 0;
> +		info.num_irqs = 0;
> +
> +		return copy_to_user((void __user *)arg, &info, minsz);
> +
> +	} else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
> +		return -EINVAL;
> +
> +	else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
> +		return -EINVAL;
> +
> +	else if (cmd == VFIO_DEVICE_SET_IRQS)
> +		return -EINVAL;
> +
> +	else if (cmd == VFIO_DEVICE_RESET)
> +		return -EINVAL;
> +
> +	return -ENOTTY;
> +}
> +
> +static ssize_t vfio_dt_read(void *device_data, char __user *buf,
> +			     size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static ssize_t vfio_dt_write(void *device_data, const char __user *buf,
> +			      size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static int vfio_dt_mmap(void *device_data, struct vm_area_struct *vma)
> +{
> +	return -EINVAL;
> +}
> +
> +static const struct vfio_device_ops vfio_dt_ops = {
> +	.name		= "vfio-dts",
> +	.open		= vfio_dt_open,
> +	.release	= vfio_dt_release,
> +	.ioctl		= vfio_dt_ioctl,
> +	.read		= vfio_dt_read,
> +	.write		= vfio_dt_write,
> +	.mmap		= vfio_dt_mmap,
> +};
> +
> +static int vfio_dt_probe(struct platform_device *pdev)
> +{
> +	struct vfio_dt_device *vdev;
> +	struct iommu_group *group;
> +	int ret;
> +
> +	group = iommu_group_get(&pdev->dev);
> +	if (!group) {
> +		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
> +		return -EINVAL;
> +	}
> +
> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> +	if (!vdev) {
> +		iommu_group_put(group);
> +		return -ENOMEM;
> +	}
> +
> +	vdev->pdev = pdev;
> +
> +	ret = vfio_add_group_dev(&pdev->dev, &vfio_dt_ops, vdev);
> +	if (ret) {
> +		iommu_group_put(group);
> +		kfree(vdev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int vfio_dt_remove(struct platform_device *pdev)
> +{
> +	struct vfio_dt_device *vdev;
> +
> +	vdev = vfio_del_group_dev(&pdev->dev);
> +	if (!vdev)
> +		return -EINVAL;
> +
> +	iommu_group_put(pdev->dev.iommu_group);
> +	kfree(vdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id vfio_dt_match[] = {
> +	/* In the future, we can implement a better mechanism to bind the
> +	 * module to any device. For now add the compatible property to the
> +	 * dtb of the devices we want to use.	*/
> +	{
> +		.compatible = "vfio-dt",
> +	},
> +	{},
> +};

This definitely doesn't belong in the dt. It's purely a Linux
abstraction and does not represent a piece of hardware or common
interface.

We need to think of a better mechanism for binding the module to these
devices now.

Thanks,
Mark.

> +MODULE_DEVICE_TABLE(of, vfio_dt_match);
> +
> +static struct platform_driver vfio_dt_driver = {
> +	.probe		= vfio_dt_probe,
> +	.remove		= vfio_dt_remove,
> +	.driver	= {
> +		.name	= "vfio-dt",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = vfio_dt_match,
> +	},
> +};
> +
> +module_platform_driver(vfio_dt_driver);
> +
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 284ff24..1e4bef2 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -147,6 +147,7 @@ struct vfio_device_info {
>  	__u32	flags;
>  #define VFIO_DEVICE_FLAGS_RESET	(1 << 0)	/* Device supports reset */
>  #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
> +#define VFIO_DEVICE_FLAGS_DT	(1 << 2)	/* vfio-dt device */
>  	__u32	num_regions;	/* Max region index + 1 */
>  	__u32	num_irqs;	/* Max IRQ index + 1 */
>  };
> -- 
> 1.8.1.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

[Mark Rutland]

>      > +static const struct of_device_id vfio_dt_match[] = {
>      > + ? ? /* In the future, we can implement a better mechanism to bind
>      the
>      > + ? ? ?* module to any device. For now add the compatible property to
>      the
>      > + ? ? ?* dtb of the devices we want to use. ? */
>      > + ? ? {
>      > + ? ? ? ? ? ? .compatible = "vfio-dt",
>      > + ? ? },
>      > + ? ? {},
>      > +};
> 
>      This definitely doesn't belong in the dt. It's purely a Linux
>      abstraction and does not represent a piece of hardware or common
>      interface.
> 
>      We need to think of a better mechanism for binding the module to these
>      devices now.
> 
>    I already make this remark in the cover letter; thanks for confirming it.
>    ?
>    Antonios

Sorry, I found the cover letter a little unclear in that regard.

Thanks for the clarification :)

Mark.

[RFC 0/3] WIP VFIO for device tree devices on Arndale

[Yoder Stuart-B08248]

Thanks for starting work on this.

> -----Original Message-----
> From: Antonios Motakis [mailto:a.motakis at virtualopensystems.com]
> Sent: Monday, August 05, 2013 8:17 AM
> To: linux-arm-kernel at lists.infradead.org; alex.williamson at redhat.com
> Cc: kvmarm at lists.cs.columbia.edu; iommu at lists.linux-foundation.org;
> linux-samsung-soc at vger.kernel.org; kvm at vger.kernel.org; agraf at suse.de;
> Yoder Stuart-B08248; Antonios Motakis
> Subject: [RFC 0/3] WIP VFIO for device tree devices on Arndale

I think we should call this infrastructure vfio for "platform
devices".  These devices are on a platform bus, as can be seen 
in /sys/bus/platform.  It is conceivable that there are platform
devices not in a device tree.

Also, the term "device tree devices" just seems awkward.

> This is a very early base work, towards VFIO support on ARM platforms
> with an IOMMU. It forms a base on to which to implement the functionality
> necessary to enable using device tree devices on ARM (and other platforms
> based on device trees) with VFIO.
> 
> This very early work of progress is only published for the sake of
> openness,
> and is very far from usable yet. However the driver can bind to devices,
> and return to userspace the info about the memory regions and IRQs.
> 
> This patch series has been tested on the Arndale board (with the Exynos
> 5250
> System MMU).
> 
> It depends on Cho KyongHo's patch series "iommu/exynos: Fixes and
> Enhancements
> of System MMU driver with DT", applied on a Linux 3.10.1 kernel, and also
> my
> own "iommu/exynos: add devices attached to the System MMU to an IOMMU
> group".
> Those patches are required at least in order to test the proposed module
> on
> Arndale.
> 
> This should not be treated as anything more than a work in progress.
> Numerous
> functions still need to be implemented properly, e.g.
>  - Proper binding of the VFIO_DT driver to devices; currently to test the
>    driver, one has to edit the device tree and add the "vfio-dt" to the
>    compatible property. However Linux does not support OF drivers that
>    can be dynamically bound to any device.

Yes, we really need to solve this.  I need to get more familiar with
the platform device infrastructure in Linux, but it seems at a high level
that it shouldn't be that hard to support dynamic binding.  We have
device drivers that register to handle certain "compatible"
strings.  And we have device tree parsing code that does platform_device_add()
calls to associate/bind a device to a driver.

What we need is something like the "new_id" mechanism in PCI where we
can have a platform driver dynamically register to support a new
device type.

Also, note that any Linux driver needs to properly support 'unbinding'
as well.   We can't have the normal driver and vfio competing to
bind to the same device.

>  - Most IOCTLs are not implemented yet. Memory region mapping, DMA
> mapping,
>    IRQFD still need to be added.
>  - The VFIO_IOMMU_TYPE1 is patched to work instead of PCI IOMMU, with
> platform
>    IOMMUs such as the one that is found on Arndale. This is a proof of
> concept
>    hack, and a more permanent fix will be proposed as the code matures.
> 
> The API used is identical to the existing VFIO API that is also used with
> PCI devices. Only devices that include a basic set of IRQs and memory
> regions
> are targeted; devices with complicated relationships with other devices
> on the
> device tree are not taken into account at this stage.
> 
> The API is not extended with device tree specific information; this would
> complicate the driver unnecessarily as it is not needed for the base use
> cases.

I would suggest adding a patch to this series that updates Documentation/vfio.txt
with any platform device specifics.

For example, it should be stated explicitly that if a device has multiple
regions and multiple IRQs that the "index" exposed by VFIO is the same
as the index within the associated "reg" and "interrupt" properties
in the device tree representation.

.i.e. if a device is represented like this in the device tree:

        reg = <0x101e2000 0x1000 0x101e4000 0x1000>;
        interrupts = <24 25 26 27>;

region #0 is 0x101e2000, region #1 is 0x101e4000
interrupt #0 is 24, and so on.

Perhaps that seems obvious, but think it is good to state it
explicitly in Documentation/vfio.txt.

> Antonios Motakis (3):
>   VFIO_IOMMU_TYPE1 workaround to build for platform devices
>   Initial skeleton of VFIO support for Device Tree based devices
>   Return info for device and its memory regions and interrupts
> 
>  drivers/vfio/Kconfig            |  12 ++-
>  drivers/vfio/Makefile           |   1 +
>  drivers/vfio/vfio_dt.c          | 233

would prefer:
drivers/vfio/vfio_platform.c

Thanks,
Stuart

[PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

[Yoder Stuart-B08248]

> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 284ff24..1e4bef2 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -147,6 +147,7 @@ struct vfio_device_info {
>  	__u32	flags;
>  #define VFIO_DEVICE_FLAGS_RESET	(1 << 0)	/* Device supports reset
> */
>  #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
> +#define VFIO_DEVICE_FLAGS_DT	(1 << 2)	/* vfio-dt device */
>  	__u32	num_regions;	/* Max region index + 1 */
>  	__u32	num_irqs;	/* Max IRQ index + 1 */
>  };

In the RFC I sent out the proposed flags were:

   #define VFIO_DEVICE_FLAGS_PLATFORM (1 << ?) /* A platform bus device */
   #define VFIO_DEVICE_FLAGS_DEVTREE  (1 << ?) /* device tree info available */

Since you are only implementing the first part, the platform
device support, just call it:  VFIO_DEVICE_FLAGS_PLATFORM

I think the 'platform' term is more accurate as we are providing
a mechanism to expose devices on the /sys/bus/platform bus to
user space.

Stuart

[PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

[Alex Williamson]

On Mon, 2013-08-05 at 15:17 +0200, Antonios Motakis wrote:
> Platform devices in the Linux kernel are usually managed by the DT
> interface. This patch forms the base to support these kind of devices
> with VFIO.
> 
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> ---
>  drivers/vfio/Kconfig      |  10 +++
>  drivers/vfio/Makefile     |   1 +
>  drivers/vfio/vfio_dt.c    | 187 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vfio.h |   1 +
>  4 files changed, 199 insertions(+)
>  create mode 100644 drivers/vfio/vfio_dt.c
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 1f84eda..a77a2e4 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -13,4 +13,14 @@ menuconfig VFIO
>  
>  	  If you don't know what to do here, say N.
>  
> +config VFIO_DT
> +	tristate "VFIO support for Device Tree devices"
> +	depends on VFIO && EVENTFD

I think there needs to be another depends item here, this would allow
configuration even on architectures that have no concept of device tree.
Also, do we want to put this in a subdirectory like we've done for pci?
The rest looks like a reasonable start.  Thanks,

Alex

> +	help
> +	  Support for the VFIO Device Tree driver.  This is required to make
> +	  use of platform devices present on Device Tree nodes using the VFIO
> +	  framework.
> +
> +	  If you don't know what to do here, say N.
> +
>  source "drivers/vfio/pci/Kconfig"
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index 2398d4a..d599a67 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_VFIO) += vfio.o
>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
>  obj-$(CONFIG_VFIO_PCI) += pci/
> +obj-$(CONFIG_VFIO_DT) += vfio_dt.o
> diff --git a/drivers/vfio/vfio_dt.c b/drivers/vfio/vfio_dt.c
> new file mode 100644
> index 0000000..ad4d31d
> --- /dev/null
> +++ b/drivers/vfio/vfio_dt.c
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (C) 2013 - Virtual Open Systems
> + * Author: Antonios Motakis <a.motakis@virtualopensystems.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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/eventfd.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +#include <linux/vfio.h>
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis@virtualopensystems.com>"
> +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
> +
> +struct vfio_dt_device {
> +	struct platform_device	*pdev;
> +};
> +
> +static void vfio_dt_release(void *device_data)
> +{
> +	module_put(THIS_MODULE);
> +}
> +
> +static int vfio_dt_open(void *device_data)
> +{
> +	if (!try_module_get(THIS_MODULE))
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static long vfio_dt_ioctl(void *device_data,
> +			   unsigned int cmd, unsigned long arg)
> +{
> +	struct vfio_dt_device *vdev = device_data;
> +	unsigned long minsz;
> +
> +	if (cmd == VFIO_DEVICE_GET_INFO) {
> +		struct vfio_device_info info;
> +
> +		minsz = offsetofend(struct vfio_device_info, num_irqs);
> +
> +		if (copy_from_user(&info, (void __user *)arg, minsz))
> +			return -EFAULT;
> +
> +		if (info.argsz < minsz)
> +			return -EINVAL;
> +
> +		info.flags = VFIO_DEVICE_FLAGS_DT;
> +		info.num_regions = 0;
> +		info.num_irqs = 0;
> +
> +		return copy_to_user((void __user *)arg, &info, minsz);
> +
> +	} else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
> +		return -EINVAL;
> +
> +	else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
> +		return -EINVAL;
> +
> +	else if (cmd == VFIO_DEVICE_SET_IRQS)
> +		return -EINVAL;
> +
> +	else if (cmd == VFIO_DEVICE_RESET)
> +		return -EINVAL;
> +
> +	return -ENOTTY;
> +}
> +
> +static ssize_t vfio_dt_read(void *device_data, char __user *buf,
> +			     size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static ssize_t vfio_dt_write(void *device_data, const char __user *buf,
> +			      size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static int vfio_dt_mmap(void *device_data, struct vm_area_struct *vma)
> +{
> +	return -EINVAL;
> +}
> +
> +static const struct vfio_device_ops vfio_dt_ops = {
> +	.name		= "vfio-dts",
> +	.open		= vfio_dt_open,
> +	.release	= vfio_dt_release,
> +	.ioctl		= vfio_dt_ioctl,
> +	.read		= vfio_dt_read,
> +	.write		= vfio_dt_write,
> +	.mmap		= vfio_dt_mmap,
> +};
> +
> +static int vfio_dt_probe(struct platform_device *pdev)
> +{
> +	struct vfio_dt_device *vdev;
> +	struct iommu_group *group;
> +	int ret;
> +
> +	group = iommu_group_get(&pdev->dev);
> +	if (!group) {
> +		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
> +		return -EINVAL;
> +	}
> +
> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> +	if (!vdev) {
> +		iommu_group_put(group);
> +		return -ENOMEM;
> +	}
> +
> +	vdev->pdev = pdev;
> +
> +	ret = vfio_add_group_dev(&pdev->dev, &vfio_dt_ops, vdev);
> +	if (ret) {
> +		iommu_group_put(group);
> +		kfree(vdev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int vfio_dt_remove(struct platform_device *pdev)
> +{
> +	struct vfio_dt_device *vdev;
> +
> +	vdev = vfio_del_group_dev(&pdev->dev);
> +	if (!vdev)
> +		return -EINVAL;
> +
> +	iommu_group_put(pdev->dev.iommu_group);
> +	kfree(vdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id vfio_dt_match[] = {
> +	/* In the future, we can implement a better mechanism to bind the
> +	 * module to any device. For now add the compatible property to the
> +	 * dtb of the devices we want to use.	*/
> +	{
> +		.compatible = "vfio-dt",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, vfio_dt_match);
> +
> +static struct platform_driver vfio_dt_driver = {
> +	.probe		= vfio_dt_probe,
> +	.remove		= vfio_dt_remove,
> +	.driver	= {
> +		.name	= "vfio-dt",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = vfio_dt_match,
> +	},
> +};
> +
> +module_platform_driver(vfio_dt_driver);
> +
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 284ff24..1e4bef2 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -147,6 +147,7 @@ struct vfio_device_info {
>  	__u32	flags;
>  #define VFIO_DEVICE_FLAGS_RESET	(1 << 0)	/* Device supports reset */
>  #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
> +#define VFIO_DEVICE_FLAGS_DT	(1 << 2)	/* vfio-dt device */
>  	__u32	num_regions;	/* Max region index + 1 */
>  	__u32	num_irqs;	/* Max IRQ index + 1 */
>  };