[PATCHv7 05/26] vfio: introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag

[PATCHv7 05/26] vfio: introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag

[Antonios Motakis]

We introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag to the VFIO dma map call,
and expose its availability via the capability VFIO_IOMMU_PROT_NOEXEC.
This way the user can control whether the XN flag will be set on the
requested mappings. The IOMMU_NOEXEC flag needs to be available for all
the IOMMUs of the container used.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 include/uapi/linux/vfio.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 6612974..30f630c 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -29,6 +29,7 @@
  * capability is subject to change as groups are added or removed.
  */
 #define VFIO_DMA_CC_IOMMU		4
+#define VFIO_IOMMU_PROT_NOEXEC		5
 
 /* Check if EEH is supported */
 #define VFIO_EEH			5
@@ -401,6 +402,7 @@ struct vfio_iommu_type1_dma_map {
 	__u32	flags;
 #define VFIO_DMA_MAP_FLAG_READ (1 << 0)		/* readable from device */
 #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1)	/* writable from device */
+#define VFIO_DMA_MAP_FLAG_NOEXEC (1 << 2)	/* not executable from device */
 	__u64	vaddr;				/* Process virtual address */
 	__u64	iova;				/* IO virtual address */
 	__u64	size;				/* Size of mapping (bytes) */
-- 
1.8.3.2

[PATCHv7 08/26] driver core: amba: add documentation for binding path 'driver_override'

[Antonios Motakis]

Add documentation for alternative binding path 'driver_override' for
AMBA devices.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-bus-amba | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-amba

diff --git a/Documentation/ABI/testing/sysfs-bus-amba b/Documentation/ABI/testing/sysfs-bus-amba
new file mode 100644
index 0000000..e7b5467
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-amba
@@ -0,0 +1,20 @@
+What:		/sys/bus/amba/devices/.../driver_override
+Date:		September 2014
+Contact:	Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
+Description:
+		This file allows the driver for a device to be specified which
+		will override standard OF, ACPI, ID table, and name matching.
+		When specified, only a driver with a name matching the value
+		written to driver_override will have an opportunity to bind to
+		the device. The override is specified by writing a string to the
+		driver_override file (echo vfio-amba > driver_override)	and may
+		be cleared with an empty string (echo > driver_override).
+		This returns the device to standard matching rules binding.
+		Writing to driver_override does not automatically unbind the
+		device from its current driver or make any attempt to
+		automatically load the specified driver. If no driver with a
+		matching name is currently loaded in the kernel, the device will
+		not bind to any driver. This also allows devices to opt-out of
+		driver binding using a driver_override name such as "none".
+		Only a single driver may be specified in the override, there is
+		no support for parsing delimiters.
-- 
1.8.3.2

[PATCHv7 10/26] vfio: platform: probe to devices on the platform bus

[Antonios Motakis]

Driver to bind to Linux platform devices, and callbacks to discover their
resources to be used by the main VFIO PLATFORM code.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/platform/vfio_platform.c | 96 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h             |  1 +
 2 files changed, 97 insertions(+)
 create mode 100644 drivers/vfio/platform/vfio_platform.c

diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
new file mode 100644
index 0000000..024c026
--- /dev/null
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
+ *
+ * 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.
+ */
+
+#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>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/irq.h>
+
+#include "vfio_platform_private.h"
+
+#define DRIVER_VERSION  "0.7"
+#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
+#define DRIVER_DESC     "VFIO for platform devices - User Level meta-driver"
+
+/* probing devices from the linux platform bus */
+
+static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
+						int i)
+{
+	struct platform_device *pdev = (struct platform_device *) vdev->opaque;
+
+	return platform_get_resource(pdev, IORESOURCE_MEM, i);
+}
+
+static int get_platform_irq(struct vfio_platform_device *vdev, int i)
+{
+	struct platform_device *pdev = (struct platform_device *) vdev->opaque;
+
+	return platform_get_irq(pdev, i);
+}
+
+
+static int vfio_platform_probe(struct platform_device *pdev)
+{
+	struct vfio_platform_device *vdev;
+	int ret;
+
+	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
+	if (!vdev)
+		return -ENOMEM;
+
+	vdev->opaque = (void *) pdev;
+	vdev->name = pdev->name;
+	vdev->flags = VFIO_DEVICE_FLAGS_PLATFORM;
+	vdev->get_resource = get_platform_resource;
+	vdev->get_irq = get_platform_irq;
+
+	ret = vfio_platform_probe_common(vdev, &pdev->dev);
+	if (ret)
+		kfree(vdev);
+
+	return ret;
+}
+
+static int vfio_platform_remove(struct platform_device *pdev)
+{
+	return vfio_platform_remove_common(&pdev->dev);
+}
+
+static struct platform_driver vfio_platform_driver = {
+	.probe		= vfio_platform_probe,
+	.remove		= vfio_platform_remove,
+	.driver	= {
+		.name	= "vfio-platform",
+		.owner	= THIS_MODULE,
+	},
+};
+
+module_platform_driver(vfio_platform_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 30f630c..b022a25 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -158,6 +158,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_PLATFORM (1 << 2)	/* vfio-platform device */
 	__u32	num_regions;	/* Max region index + 1 */
 	__u32	num_irqs;	/* Max IRQ index + 1 */
 };
-- 
1.8.3.2

[PATCHv7 12/26] vfio: amba: VFIO support for AMBA devices

[Antonios Motakis]

Add support for discovering AMBA devices with VFIO and handle them
similarly to Linux platform devices.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/platform/vfio_amba.c | 108 ++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h         |   1 +
 2 files changed, 109 insertions(+)
 create mode 100644 drivers/vfio/platform/vfio_amba.c

diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c
new file mode 100644
index 0000000..e8c5e1a
--- /dev/null
+++ b/drivers/vfio/platform/vfio_amba.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
+ *
+ * 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.
+ */
+
+#include <linux/device.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>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/amba/bus.h>
+
+#include "vfio_platform_private.h"
+
+#define DRIVER_VERSION  "0.7"
+#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
+#define DRIVER_DESC     "VFIO for AMBA devices - User Level meta-driver"
+
+/* probing devices from the AMBA bus */
+
+static struct resource *get_amba_resource(struct vfio_platform_device *vdev,
+						int i)
+{
+	struct amba_device *adev = (struct amba_device *) vdev->opaque;
+
+	if (i == 0)
+		return &adev->res;
+
+	return NULL;
+}
+
+static int get_amba_irq(struct vfio_platform_device *vdev, int i)
+{
+	struct amba_device *adev = (struct amba_device *) vdev->opaque;
+
+	if (i < AMBA_NR_IRQS)
+		return adev->irq[i];
+
+	return 0;
+}
+
+static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id)
+{
+
+	struct vfio_platform_device *vdev;
+	int ret;
+
+	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
+	if (!vdev)
+		return -ENOMEM;
+
+	vdev->opaque = (void *) adev;
+	vdev->name = "vfio-amba-dev";
+	vdev->flags = VFIO_DEVICE_FLAGS_AMBA;
+	vdev->get_resource = get_amba_resource;
+	vdev->get_irq = get_amba_irq;
+
+	ret = vfio_platform_probe_common(vdev, &adev->dev);
+	if (ret)
+		kfree(vdev);
+
+	return ret;
+}
+
+static int vfio_amba_remove(struct amba_device *adev)
+{
+	return vfio_platform_remove_common(&adev->dev);
+}
+
+static struct amba_id pl330_ids[] = {
+	{ 0, 0 },
+};
+
+MODULE_DEVICE_TABLE(amba, pl330_ids);
+
+static struct amba_driver vfio_amba_driver = {
+	.probe = vfio_amba_probe,
+	.remove = vfio_amba_remove,
+	.id_table = pl330_ids,
+	.drv = {
+		.name = "vfio-amba",
+		.owner = THIS_MODULE,
+	},
+};
+
+module_amba_driver(vfio_amba_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 b022a25..72f121f 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -159,6 +159,7 @@ struct vfio_device_info {
 #define VFIO_DEVICE_FLAGS_RESET	(1 << 0)	/* Device supports reset */
 #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
 #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2)	/* vfio-platform device */
+#define VFIO_DEVICE_FLAGS_AMBA  (1 << 3)	/* vfio-amba device */
 	__u32	num_regions;	/* Max region index + 1 */
 	__u32	num_irqs;	/* Max IRQ index + 1 */
 };
-- 
1.8.3.2

Re: [PATCHv7 05/26] vfio: introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag

[Alex Williamson]

On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
> We introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag to the VFIO dma map call,
> and expose its availability via the capability VFIO_IOMMU_PROT_NOEXEC.
> This way the user can control whether the XN flag will be set on the
> requested mappings. The IOMMU_NOEXEC flag needs to be available for all
> the IOMMUs of the container used.
> 
> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  include/uapi/linux/vfio.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 6612974..30f630c 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -29,6 +29,7 @@
>   * capability is subject to change as groups are added or removed.
>   */
>  #define VFIO_DMA_CC_IOMMU		4
> +#define VFIO_IOMMU_PROT_NOEXEC		5

Can't we advertise this as a flag bit in vfio_iommu_type1_info instead?
Also, EEH already took 5 as seen immediately below.

>  
>  /* Check if EEH is supported */
>  #define VFIO_EEH			5
> @@ -401,6 +402,7 @@ struct vfio_iommu_type1_dma_map {
>  	__u32	flags;
>  #define VFIO_DMA_MAP_FLAG_READ (1 << 0)		/* readable from device */
>  #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1)	/* writable from device */
> +#define VFIO_DMA_MAP_FLAG_NOEXEC (1 << 2)	/* not executable from device */
>  	__u64	vaddr;				/* Process virtual address */
>  	__u64	iova;				/* IO virtual address */
>  	__u64	size;				/* Size of mapping (bytes) */

Re: [PATCHv7 05/26] vfio: introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag

[Alex Williamson]

On Tue, 2014-09-23 at 16:21 -0600, Alex Williamson wrote:
> On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
> > We introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag to the VFIO dma map call,
> > and expose its availability via the capability VFIO_IOMMU_PROT_NOEXEC.
> > This way the user can control whether the XN flag will be set on the
> > requested mappings. The IOMMU_NOEXEC flag needs to be available for all
> > the IOMMUs of the container used.
> > 
> > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > ---
> >  include/uapi/linux/vfio.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 6612974..30f630c 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -29,6 +29,7 @@
> >   * capability is subject to change as groups are added or removed.
> >   */
> >  #define VFIO_DMA_CC_IOMMU		4
> > +#define VFIO_IOMMU_PROT_NOEXEC		5
> 
> Can't we advertise this as a flag bit in vfio_iommu_type1_info instead?

Ok, I see in the next patch that it's pretty similar to
VFIO_DMA_CC_IOMMU, so the check extension is probably correct for
determining the current state.  Maybe we could name it more similarly,
VFIO_DMA_NOEXEC_IOMMU.  I guess the intended usage is that once a user
attaches a group to the container they can query whether the
VFIO_DMA_MAP_FLAG_NOEXEC is valid.  Ok.  Thanks,

Alex

> Also, EEH already took 5 as seen immediately below.
> 
> >  
> >  /* Check if EEH is supported */
> >  #define VFIO_EEH			5
> > @@ -401,6 +402,7 @@ struct vfio_iommu_type1_dma_map {
> >  	__u32	flags;
> >  #define VFIO_DMA_MAP_FLAG_READ (1 << 0)		/* readable from device */
> >  #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1)	/* writable from device */
> > +#define VFIO_DMA_MAP_FLAG_NOEXEC (1 << 2)	/* not executable from device */
> >  	__u64	vaddr;				/* Process virtual address */
> >  	__u64	iova;				/* IO virtual address */
> >  	__u64	size;				/* Size of mapping (bytes) */
> 
> 

Re: [PATCHv7 10/26] vfio: platform: probe to devices on the platform bus

[Alex Williamson]

On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
> Driver to bind to Linux platform devices, and callbacks to discover their
> resources to be used by the main VFIO PLATFORM code.
> 
> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  drivers/vfio/platform/vfio_platform.c | 96 +++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vfio.h             |  1 +
>  2 files changed, 97 insertions(+)
>  create mode 100644 drivers/vfio/platform/vfio_platform.c
> 
> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
> new file mode 100644
> index 0000000..024c026
> --- /dev/null
> +++ b/drivers/vfio/platform/vfio_platform.c
> @@ -0,0 +1,96 @@
> +/*
> + * Copyright (C) 2013 - Virtual Open Systems
> + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> + *
> + * 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.
> + */
> +
> +#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>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/irq.h>
> +
> +#include "vfio_platform_private.h"
> +
> +#define DRIVER_VERSION  "0.7"
> +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> +#define DRIVER_DESC     "VFIO for platform devices - User Level meta-driver"
> +
> +/* probing devices from the linux platform bus */
> +
> +static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> +						int i)
> +{
> +	struct platform_device *pdev = (struct platform_device *) vdev->opaque;
> +
> +	return platform_get_resource(pdev, IORESOURCE_MEM, i);

ARM may only support IORESOURCE_MEM, but I don't think platform devices
are limited to MMIO, right?  vfio-platform shouldn't be either.

> +}
> +
> +static int get_platform_irq(struct vfio_platform_device *vdev, int i)
> +{
> +	struct platform_device *pdev = (struct platform_device *) vdev->opaque;
> +
> +	return platform_get_irq(pdev, i);
> +}
> +
> +
> +static int vfio_platform_probe(struct platform_device *pdev)
> +{
> +	struct vfio_platform_device *vdev;
> +	int ret;
> +
> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> +	if (!vdev)
> +		return -ENOMEM;
> +
> +	vdev->opaque = (void *) pdev;
> +	vdev->name = pdev->name;
> +	vdev->flags = VFIO_DEVICE_FLAGS_PLATFORM;
> +	vdev->get_resource = get_platform_resource;
> +	vdev->get_irq = get_platform_irq;
> +
> +	ret = vfio_platform_probe_common(vdev, &pdev->dev);
> +	if (ret)
> +		kfree(vdev);
> +
> +	return ret;
> +}
> +
> +static int vfio_platform_remove(struct platform_device *pdev)
> +{
> +	return vfio_platform_remove_common(&pdev->dev);
> +}
> +
> +static struct platform_driver vfio_platform_driver = {
> +	.probe		= vfio_platform_probe,
> +	.remove		= vfio_platform_remove,
> +	.driver	= {
> +		.name	= "vfio-platform",
> +		.owner	= THIS_MODULE,
> +	},
> +};
> +
> +module_platform_driver(vfio_platform_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 30f630c..b022a25 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -158,6 +158,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_PLATFORM (1 << 2)	/* vfio-platform device */
>  	__u32	num_regions;	/* Max region index + 1 */
>  	__u32	num_irqs;	/* Max IRQ index + 1 */
>  };

Re: [PATCHv7 10/26] vfio: platform: probe to devices on the platform bus

[Antonios Motakis]

On Wed, Sep 24, 2014 at 1:01 AM, Alex Williamson
<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
> > Driver to bind to Linux platform devices, and callbacks to discover their
> > resources to be used by the main VFIO PLATFORM code.
> >
> > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > ---
> >  drivers/vfio/platform/vfio_platform.c | 96 +++++++++++++++++++++++++++++++++++
> >  include/uapi/linux/vfio.h             |  1 +
> >  2 files changed, 97 insertions(+)
> >  create mode 100644 drivers/vfio/platform/vfio_platform.c
> >
> > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
> > new file mode 100644
> > index 0000000..024c026
> > --- /dev/null
> > +++ b/drivers/vfio/platform/vfio_platform.c
> > @@ -0,0 +1,96 @@
> > +/*
> > + * Copyright (C) 2013 - Virtual Open Systems
> > + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > + *
> > + * 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.
> > + */
> > +
> > +#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>
> > +#include <linux/io.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/irq.h>
> > +
> > +#include "vfio_platform_private.h"
> > +
> > +#define DRIVER_VERSION  "0.7"
> > +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> > +#define DRIVER_DESC     "VFIO for platform devices - User Level meta-driver"
> > +
> > +/* probing devices from the linux platform bus */
> > +
> > +static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> > +                                             int i)
> > +{
> > +     struct platform_device *pdev = (struct platform_device *) vdev->opaque;
> > +
> > +     return platform_get_resource(pdev, IORESOURCE_MEM, i);
>
> ARM may only support IORESOURCE_MEM, but I don't think platform devices
> are limited to MMIO, right?  vfio-platform shouldn't be either.
>

Indeed. Should we however implement this lacking a target to verify it
is working correctly?

Leaving it out would mean PIO resources for those devices would not be
exposed before an update to VFIO, but we wouldn't have to break
backward compatibility I think.

Would you prefer to have it implemented regardless?

> > +}
> > +
> > +static int get_platform_irq(struct vfio_platform_device *vdev, int i)
> > +{
> > +     struct platform_device *pdev = (struct platform_device *) vdev->opaque;
> > +
> > +     return platform_get_irq(pdev, i);
> > +}
> > +
> > +
> > +static int vfio_platform_probe(struct platform_device *pdev)
> > +{
> > +     struct vfio_platform_device *vdev;
> > +     int ret;
> > +
> > +     vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > +     if (!vdev)
> > +             return -ENOMEM;
> > +
> > +     vdev->opaque = (void *) pdev;
> > +     vdev->name = pdev->name;
> > +     vdev->flags = VFIO_DEVICE_FLAGS_PLATFORM;
> > +     vdev->get_resource = get_platform_resource;
> > +     vdev->get_irq = get_platform_irq;
> > +
> > +     ret = vfio_platform_probe_common(vdev, &pdev->dev);
> > +     if (ret)
> > +             kfree(vdev);
> > +
> > +     return ret;
> > +}
> > +
> > +static int vfio_platform_remove(struct platform_device *pdev)
> > +{
> > +     return vfio_platform_remove_common(&pdev->dev);
> > +}
> > +
> > +static struct platform_driver vfio_platform_driver = {
> > +     .probe          = vfio_platform_probe,
> > +     .remove         = vfio_platform_remove,
> > +     .driver = {
> > +             .name   = "vfio-platform",
> > +             .owner  = THIS_MODULE,
> > +     },
> > +};
> > +
> > +module_platform_driver(vfio_platform_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 30f630c..b022a25 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -158,6 +158,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_PLATFORM (1 << 2)  /* vfio-platform device */
> >       __u32   num_regions;    /* Max region index + 1 */
> >       __u32   num_irqs;       /* Max IRQ index + 1 */
> >  };
>
>
>



-- 
Antonios Motakis
Virtual Open Systems

Re: [PATCHv7 05/26] vfio: introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag

[Antonios Motakis]

On Wed, Sep 24, 2014 at 12:30 AM, Alex Williamson
<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, 2014-09-23 at 16:21 -0600, Alex Williamson wrote:
>> On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
>> > We introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag to the VFIO dma map call,
>> > and expose its availability via the capability VFIO_IOMMU_PROT_NOEXEC.
>> > This way the user can control whether the XN flag will be set on the
>> > requested mappings. The IOMMU_NOEXEC flag needs to be available for all
>> > the IOMMUs of the container used.
>> >
>> > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>> > ---
>> >  include/uapi/linux/vfio.h | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> > index 6612974..30f630c 100644
>> > --- a/include/uapi/linux/vfio.h
>> > +++ b/include/uapi/linux/vfio.h
>> > @@ -29,6 +29,7 @@
>> >   * capability is subject to change as groups are added or removed.
>> >   */
>> >  #define VFIO_DMA_CC_IOMMU          4
>> > +#define VFIO_IOMMU_PROT_NOEXEC             5
>>
>> Can't we advertise this as a flag bit in vfio_iommu_type1_info instead?
>
> Ok, I see in the next patch that it's pretty similar to
> VFIO_DMA_CC_IOMMU, so the check extension is probably correct for
> determining the current state.  Maybe we could name it more similarly,
> VFIO_DMA_NOEXEC_IOMMU.  I guess the intended usage is that once a user
> attaches a group to the container they can query whether the
> VFIO_DMA_MAP_FLAG_NOEXEC is valid.  Ok.  Thanks,
>

Sorry for the numbering, ack and ack :)

> Alex
>
>> Also, EEH already took 5 as seen immediately below.
>>
>> >
>> >  /* Check if EEH is supported */
>> >  #define VFIO_EEH                   5
>> > @@ -401,6 +402,7 @@ struct vfio_iommu_type1_dma_map {
>> >     __u32   flags;
>> >  #define VFIO_DMA_MAP_FLAG_READ (1 << 0)            /* readable from device */
>> >  #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1)   /* writable from device */
>> > +#define VFIO_DMA_MAP_FLAG_NOEXEC (1 << 2)  /* not executable from device */
>> >     __u64   vaddr;                          /* Process virtual address */
>> >     __u64   iova;                           /* IO virtual address */
>> >     __u64   size;                           /* Size of mapping (bytes) */
>>
>>
>
>
>



-- 
Antonios Motakis
Virtual Open Systems

Re: [PATCHv7 10/26] vfio: platform: probe to devices on the platform bus

[Alex Williamson]

On Fri, 2014-09-26 at 17:30 +0200, Antonios Motakis wrote:
> On Wed, Sep 24, 2014 at 1:01 AM, Alex Williamson
> <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
> > > Driver to bind to Linux platform devices, and callbacks to discover their
> > > resources to be used by the main VFIO PLATFORM code.
> > >
> > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > > ---
> > >  drivers/vfio/platform/vfio_platform.c | 96 +++++++++++++++++++++++++++++++++++
> > >  include/uapi/linux/vfio.h             |  1 +
> > >  2 files changed, 97 insertions(+)
> > >  create mode 100644 drivers/vfio/platform/vfio_platform.c
> > >
> > > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
> > > new file mode 100644
> > > index 0000000..024c026
> > > --- /dev/null
> > > +++ b/drivers/vfio/platform/vfio_platform.c
> > > @@ -0,0 +1,96 @@
> > > +/*
> > > + * Copyright (C) 2013 - Virtual Open Systems
> > > + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > > + *
> > > + * 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.
> > > + */
> > > +
> > > +#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>
> > > +#include <linux/io.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/irq.h>
> > > +
> > > +#include "vfio_platform_private.h"
> > > +
> > > +#define DRIVER_VERSION  "0.7"
> > > +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> > > +#define DRIVER_DESC     "VFIO for platform devices - User Level meta-driver"
> > > +
> > > +/* probing devices from the linux platform bus */
> > > +
> > > +static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> > > +                                             int i)
> > > +{
> > > +     struct platform_device *pdev = (struct platform_device *) vdev->opaque;
> > > +
> > > +     return platform_get_resource(pdev, IORESOURCE_MEM, i);
> >
> > ARM may only support IORESOURCE_MEM, but I don't think platform devices
> > are limited to MMIO, right?  vfio-platform shouldn't be either.
> >
> 
> Indeed. Should we however implement this lacking a target to verify it
> is working correctly?
> 
> Leaving it out would mean PIO resources for those devices would not be
> exposed before an update to VFIO, but we wouldn't have to break
> backward compatibility I think.
> 
> Would you prefer to have it implemented regardless?

I think we need to have PIO figured out at least enough to have stubbed
read/write handlers that could be filled in by someone with test
hardware.  I'm not sure I fully understand how a user associates a
region index to a device tree description, whether it's ordering or
something more complicated, so I'm not sure if simply listing all the
PIO resources after the MMIO resources is sufficient and compatible.
Maybe you have some thoughts on that.  Thanks,

Alex

> > > +}
> > > +
> > > +static int get_platform_irq(struct vfio_platform_device *vdev, int i)
> > > +{
> > > +     struct platform_device *pdev = (struct platform_device *) vdev->opaque;
> > > +
> > > +     return platform_get_irq(pdev, i);
> > > +}
> > > +
> > > +
> > > +static int vfio_platform_probe(struct platform_device *pdev)
> > > +{
> > > +     struct vfio_platform_device *vdev;
> > > +     int ret;
> > > +
> > > +     vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > > +     if (!vdev)
> > > +             return -ENOMEM;
> > > +
> > > +     vdev->opaque = (void *) pdev;
> > > +     vdev->name = pdev->name;
> > > +     vdev->flags = VFIO_DEVICE_FLAGS_PLATFORM;
> > > +     vdev->get_resource = get_platform_resource;
> > > +     vdev->get_irq = get_platform_irq;
> > > +
> > > +     ret = vfio_platform_probe_common(vdev, &pdev->dev);
> > > +     if (ret)
> > > +             kfree(vdev);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static int vfio_platform_remove(struct platform_device *pdev)
> > > +{
> > > +     return vfio_platform_remove_common(&pdev->dev);
> > > +}
> > > +
> > > +static struct platform_driver vfio_platform_driver = {
> > > +     .probe          = vfio_platform_probe,
> > > +     .remove         = vfio_platform_remove,
> > > +     .driver = {
> > > +             .name   = "vfio-platform",
> > > +             .owner  = THIS_MODULE,
> > > +     },
> > > +};
> > > +
> > > +module_platform_driver(vfio_platform_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 30f630c..b022a25 100644
> > > --- a/include/uapi/linux/vfio.h
> > > +++ b/include/uapi/linux/vfio.h
> > > @@ -158,6 +158,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_PLATFORM (1 << 2)  /* vfio-platform device */
> > >       __u32   num_regions;    /* Max region index + 1 */
> > >       __u32   num_irqs;       /* Max IRQ index + 1 */
> > >  };
> >
> >
> >
> 
> 
> 

Re: [PATCHv7 10/26] vfio: platform: probe to devices on the platform bus

[Antonios Motakis]

On Fri, Sep 26, 2014 at 10:18 PM, Alex Williamson
<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 2014-09-26 at 17:30 +0200, Antonios Motakis wrote:
>> On Wed, Sep 24, 2014 at 1:01 AM, Alex Williamson
>> <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> >
>> > On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
>> > > Driver to bind to Linux platform devices, and callbacks to discover their
>> > > resources to be used by the main VFIO PLATFORM code.
>> > >
>> > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>> > > ---
>> > >  drivers/vfio/platform/vfio_platform.c | 96 +++++++++++++++++++++++++++++++++++
>> > >  include/uapi/linux/vfio.h             |  1 +
>> > >  2 files changed, 97 insertions(+)
>> > >  create mode 100644 drivers/vfio/platform/vfio_platform.c
>> > >
>> > > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
>> > > new file mode 100644
>> > > index 0000000..024c026
>> > > --- /dev/null
>> > > +++ b/drivers/vfio/platform/vfio_platform.c
>> > > @@ -0,0 +1,96 @@
>> > > +/*
>> > > + * Copyright (C) 2013 - Virtual Open Systems
>> > > + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>> > > + *
>> > > + * 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.
>> > > + */
>> > > +
>> > > +#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>
>> > > +#include <linux/io.h>
>> > > +#include <linux/platform_device.h>
>> > > +#include <linux/irq.h>
>> > > +
>> > > +#include "vfio_platform_private.h"
>> > > +
>> > > +#define DRIVER_VERSION  "0.7"
>> > > +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
>> > > +#define DRIVER_DESC     "VFIO for platform devices - User Level meta-driver"
>> > > +
>> > > +/* probing devices from the linux platform bus */
>> > > +
>> > > +static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
>> > > +                                             int i)
>> > > +{
>> > > +     struct platform_device *pdev = (struct platform_device *) vdev->opaque;
>> > > +
>> > > +     return platform_get_resource(pdev, IORESOURCE_MEM, i);
>> >
>> > ARM may only support IORESOURCE_MEM, but I don't think platform devices
>> > are limited to MMIO, right?  vfio-platform shouldn't be either.
>> >
>>
>> Indeed. Should we however implement this lacking a target to verify it
>> is working correctly?
>>
>> Leaving it out would mean PIO resources for those devices would not be
>> exposed before an update to VFIO, but we wouldn't have to break
>> backward compatibility I think.
>>
>> Would you prefer to have it implemented regardless?
>
> I think we need to have PIO figured out at least enough to have stubbed
> read/write handlers that could be filled in by someone with test
> hardware.  I'm not sure I fully understand how a user associates a
> region index to a device tree description, whether it's ordering or
> something more complicated, so I'm not sure if simply listing all the
> PIO resources after the MMIO resources is sufficient and compatible.
> Maybe you have some thoughts on that.  Thanks,

You are right. I'm not aware if the Linux calls used are guaranteed to
preserve ordering of PIO/MMIO resources, I will investigate a bit on
that.

>
> Alex
>
>> > > +}
>> > > +
>> > > +static int get_platform_irq(struct vfio_platform_device *vdev, int i)
>> > > +{
>> > > +     struct platform_device *pdev = (struct platform_device *) vdev->opaque;
>> > > +
>> > > +     return platform_get_irq(pdev, i);
>> > > +}
>> > > +
>> > > +
>> > > +static int vfio_platform_probe(struct platform_device *pdev)
>> > > +{
>> > > +     struct vfio_platform_device *vdev;
>> > > +     int ret;
>> > > +
>> > > +     vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
>> > > +     if (!vdev)
>> > > +             return -ENOMEM;
>> > > +
>> > > +     vdev->opaque = (void *) pdev;
>> > > +     vdev->name = pdev->name;
>> > > +     vdev->flags = VFIO_DEVICE_FLAGS_PLATFORM;
>> > > +     vdev->get_resource = get_platform_resource;
>> > > +     vdev->get_irq = get_platform_irq;
>> > > +
>> > > +     ret = vfio_platform_probe_common(vdev, &pdev->dev);
>> > > +     if (ret)
>> > > +             kfree(vdev);
>> > > +
>> > > +     return ret;
>> > > +}
>> > > +
>> > > +static int vfio_platform_remove(struct platform_device *pdev)
>> > > +{
>> > > +     return vfio_platform_remove_common(&pdev->dev);
>> > > +}
>> > > +
>> > > +static struct platform_driver vfio_platform_driver = {
>> > > +     .probe          = vfio_platform_probe,
>> > > +     .remove         = vfio_platform_remove,
>> > > +     .driver = {
>> > > +             .name   = "vfio-platform",
>> > > +             .owner  = THIS_MODULE,
>> > > +     },
>> > > +};
>> > > +
>> > > +module_platform_driver(vfio_platform_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 30f630c..b022a25 100644
>> > > --- a/include/uapi/linux/vfio.h
>> > > +++ b/include/uapi/linux/vfio.h
>> > > @@ -158,6 +158,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_PLATFORM (1 << 2)  /* vfio-platform device */
>> > >       __u32   num_regions;    /* Max region index + 1 */
>> > >       __u32   num_irqs;       /* Max IRQ index + 1 */
>> > >  };
>> >
>> >
>> >
>>
>>
>>
>
>
>



-- 
Antonios Motakis
Virtual Open Systems