Discussion:
[PATCH v8 01/18] vfio/platform: initial skeleton of VFIO support for platform devices
Antonios Motakis
2014-10-13 13:10:08 UTC
Permalink
This patch forms the common skeleton code for platform devices support
with VFIO. This will include the core functionality of VFIO_PLATFORM,
however binding to the device and discovering the device resources will
be done with the help of a separate file where any Linux platform bus
specific code will reside.

This will allow us to implement support for also discovering AMBA devices
and their resources, but still reuse a large part of the VFIO_PLATFORM
implementation.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_common.c | 129 ++++++++++++++++++++++++++
drivers/vfio/platform/vfio_platform_private.h | 35 +++++++
2 files changed, 164 insertions(+)
create mode 100644 drivers/vfio/platform/vfio_platform_common.c
create mode 100644 drivers/vfio/platform/vfio_platform_private.h

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
new file mode 100644
index 0000000..13808d0
--- /dev/null
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@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 "vfio_platform_private.h"
+
+static void vfio_platform_release(void *device_data)
+{
+ module_put(THIS_MODULE);
+}
+
+static int vfio_platform_open(void *device_data)
+{
+ if (!try_module_get(THIS_MODULE))
+ return -ENODEV;
+
+ return 0;
+}
+
+static long vfio_platform_ioctl(void *device_data,
+ unsigned int cmd, unsigned long arg)
+{
+ if (cmd == VFIO_DEVICE_GET_INFO)
+ return -EINVAL;
+
+ 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_platform_read(void *device_data, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ return -EINVAL;
+}
+
+static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ return -EINVAL;
+}
+
+static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
+{
+ return -EINVAL;
+}
+
+static const struct vfio_device_ops vfio_platform_ops = {
+ .name = "vfio-platform",
+ .open = vfio_platform_open,
+ .release = vfio_platform_release,
+ .ioctl = vfio_platform_ioctl,
+ .read = vfio_platform_read,
+ .write = vfio_platform_write,
+ .mmap = vfio_platform_mmap,
+};
+
+int vfio_platform_probe_common(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ struct iommu_group *group;
+ int ret;
+
+ if (!vdev)
+ return -EINVAL;
+
+ group = iommu_group_get(dev);
+ if (!group) {
+ pr_err("VFIO: No IOMMU group for device %s\n", vdev->name);
+ return -EINVAL;
+ }
+
+ ret = vfio_add_group_dev(dev, &vfio_platform_ops, vdev);
+ if (ret) {
+ iommu_group_put(group);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(vfio_platform_probe_common);
+
+int vfio_platform_remove_common(struct device *dev)
+{
+ struct vfio_platform_device *vdev;
+
+ vdev = vfio_del_group_dev(dev);
+ if (!vdev)
+ return -EINVAL;
+
+ iommu_group_put(dev->iommu_group);
+ kfree(vdev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(vfio_platform_remove_common);
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
new file mode 100644
index 0000000..ef76737
--- /dev/null
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@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.
+ */
+
+#ifndef VFIO_PLATFORM_PRIVATE_H
+#define VFIO_PLATFORM_PRIVATE_H
+
+struct vfio_platform_device {
+ /*
+ * These fields should be filled by the bus driver at binding time
+ */
+ void *opaque;
+ const char *name;
+ uint32_t flags;
+ /* callbacks to discover device resources */
+ struct resource*
+ (*get_resource)(struct vfio_platform_device *vdev, int i);
+ int (*get_irq)(struct vfio_platform_device *vdev, int i);
+};
+
+extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
+ struct device *dev);
+extern int vfio_platform_remove_common(struct device *dev);
+
+#endif /* VFIO_PLATFORM_PRIVATE_H */
--
2.1.1
Antonios Motakis
2014-10-13 13:10:09 UTC
Permalink
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/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform.c | 107 ++++++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 1 +
2 files changed, 108 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..baeb7da
--- /dev/null
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@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.8"
+#define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@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 num)
+{
+ struct platform_device *dev = (struct platform_device *) vdev->opaque;
+ int i;
+
+ for (i = 0; i < dev->num_resources; i++) {
+ struct resource *r = &dev->resource[i];
+
+ if (resource_type(r) & (IORESOURCE_MEM|IORESOURCE_IO)) {
+ num--;
+
+ if (!num)
+ return r;
+ }
+ }
+ return NULL;
+}
+
+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 111b5e8..aca6d3e 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 */
};
--
2.1.1
Alex Williamson
2014-10-21 16:17:49 UTC
Permalink
Post by Antonios Motakis
Driver to bind to Linux platform devices, and callbacks to discover their
resources to be used by the main VFIO PLATFORM code.
---
drivers/vfio/platform/vfio_platform.c | 107 ++++++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 1 +
2 files changed, 108 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..baeb7da
--- /dev/null
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ *
+ * 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.8"
+#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 num)
+{
+ struct platform_device *dev = (struct platform_device *) vdev->opaque;
+ int i;
+
+ for (i = 0; i < dev->num_resources; i++) {
+ struct resource *r = &dev->resource[i];
+
+ if (resource_type(r) & (IORESOURCE_MEM|IORESOURCE_IO)) {
+ num--;
+
+ if (!num)
+ return r;
Has this been tested? What happens when we call this with num = 0?
Post by Antonios Motakis
+ }
+ }
+ return NULL;
+}
+
+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 111b5e8..aca6d3e 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 */
};
Eric Auger
2014-10-21 16:37:46 UTC
Permalink
Post by Alex Williamson
Post by Antonios Motakis
Driver to bind to Linux platform devices, and callbacks to discover their
resources to be used by the main VFIO PLATFORM code.
---
drivers/vfio/platform/vfio_platform.c | 107 ++++++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 1 +
2 files changed, 108 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..baeb7da
--- /dev/null
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ *
+ * 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.8"
+#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 num)
+{
+ struct platform_device *dev = (struct platform_device *) vdev->opaque;
+ int i;
+
+ for (i = 0; i < dev->num_resources; i++) {
+ struct resource *r = &dev->resource[i];
+
+ if (resource_type(r) & (IORESOURCE_MEM|IORESOURCE_IO)) {
+ num--;
+
+ if (!num)
+ return r;
Has this been tested? What happens when we call this with num = 0?
Yep. I confirm I enter that case with my xgmac where the IORESOURCE_MEM
is the 1st resource. I Just ended to the same cause ;-)

Best Regards

Eric
Post by Alex Williamson
Post by Antonios Motakis
+ }
+ }
+ return NULL;
+}
+
+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 111b5e8..aca6d3e 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
2014-10-13 13:10:10 UTC
Permalink
Enable building the VFIO PLATFORM driver that allows to use Linux platform
devices with VFIO.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/Kconfig | 1 +
drivers/vfio/Makefile | 1 +
drivers/vfio/platform/Kconfig | 9 +++++++++
drivers/vfio/platform/Makefile | 4 ++++
4 files changed, 15 insertions(+)
create mode 100644 drivers/vfio/platform/Kconfig
create mode 100644 drivers/vfio/platform/Makefile

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index a0abe04..962fb80 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -27,3 +27,4 @@ menuconfig VFIO
If you don't know what to do here, say N.

source "drivers/vfio/pci/Kconfig"
+source "drivers/vfio/platform/Kconfig"
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 0b035b1..dadf0ca 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
obj-$(CONFIG_VFIO_PCI) += pci/
+obj-$(CONFIG_VFIO_PLATFORM) += platform/
diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
new file mode 100644
index 0000000..c51af17
--- /dev/null
+++ b/drivers/vfio/platform/Kconfig
@@ -0,0 +1,9 @@
+config VFIO_PLATFORM
+ tristate "VFIO support for platform devices"
+ depends on VFIO && EVENTFD && ARM
+ help
+ Support for platform devices with VFIO. This is required to make
+ use of platform devices present on the system using the VFIO
+ framework.
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
new file mode 100644
index 0000000..279862b
--- /dev/null
+++ b/drivers/vfio/platform/Makefile
@@ -0,0 +1,4 @@
+
+vfio-platform-y := vfio_platform.o vfio_platform_common.o
+
+obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
--
2.1.1
Antonios Motakis
2014-10-13 13:10:11 UTC
Permalink
Add support for discovering AMBA devices with VFIO and handle them
similarly to Linux platform devices.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@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..b16a40d
--- /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/***@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.8"
+#define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@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 aca6d3e..1a5986c 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 */
};
--
2.1.1
Antonios Motakis
2014-10-13 13:10:12 UTC
Permalink
Enable building the VFIO AMBA driver. VFIO_AMBA depends on VFIO_PLATFORM,
since it is sharing a portion of the code, and it is essentially implemented
as a platform device whose resources are discovered via AMBA specific APIs
in the kernel.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/Kconfig | 10 ++++++++++
drivers/vfio/platform/Makefile | 4 ++++
2 files changed, 14 insertions(+)

diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
index c51af17..c0a3bff 100644
--- a/drivers/vfio/platform/Kconfig
+++ b/drivers/vfio/platform/Kconfig
@@ -7,3 +7,13 @@ config VFIO_PLATFORM
framework.

If you don't know what to do here, say N.
+
+config VFIO_AMBA
+ tristate "VFIO support for AMBA devices"
+ depends on VFIO_PLATFORM && ARM_AMBA
+ help
+ Support for ARM AMBA devices with VFIO. This is required to make
+ use of ARM AMBA devices present on the system using the VFIO
+ framework.
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
index 279862b..1957170 100644
--- a/drivers/vfio/platform/Makefile
+++ b/drivers/vfio/platform/Makefile
@@ -2,3 +2,7 @@
vfio-platform-y := vfio_platform.o vfio_platform_common.o

obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
+
+vfio-amba-y := vfio_amba.o
+
+obj-$(CONFIG_VFIO_AMBA) += vfio-amba.o
--
2.1.1
Antonios Motakis
2014-10-13 13:10:13 UTC
Permalink
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 enables the
VFIO_DEVICE_GET_INFO ioctl call.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_common.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 13808d0..1e4073f 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -43,10 +43,27 @@ static int vfio_platform_open(void *device_data)
static long vfio_platform_ioctl(void *device_data,
unsigned int cmd, unsigned long arg)
{
- if (cmd == VFIO_DEVICE_GET_INFO)
- return -EINVAL;
+ struct vfio_platform_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 = vdev->flags;
+ 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)
+ } else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
return -EINVAL;

else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
--
2.1.1
Antonios Motakis
2014-10-13 13:10:14 UTC
Permalink
This patch enables the IOCTLs VFIO_DEVICE_GET_REGION_INFO ioctl call,
which allows the user to learn about the available MMIO resources of
a device.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_common.c | 93 +++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 22 +++++++
2 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 1e4073f..8a7e474 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -27,17 +27,84 @@

#include "vfio_platform_private.h"

+static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
+{
+ int cnt = 0, i;
+
+ while (vdev->get_resource(vdev, cnt))
+ cnt++;
+
+ vdev->regions = kcalloc(cnt, sizeof(struct vfio_platform_region),
+ GFP_KERNEL);
+ if (!vdev->regions)
+ return -ENOMEM;
+
+ for (i = 0; i < cnt; i++) {
+ struct resource *res =
+ vdev->get_resource(vdev, i);
+
+ if (!res)
+ goto err;
+
+ vdev->regions[i].addr = res->start;
+ vdev->regions[i].size = resource_size(res);
+ vdev->regions[i].flags = 0;
+
+ switch (resource_type(res)) {
+ case IORESOURCE_MEM:
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_MMIO;
+ break;
+ case IORESOURCE_IO:
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
+ break;
+ default:
+ goto err;
+ }
+ }
+
+ vdev->num_regions = cnt;
+
+ return 0;
+err:
+ kfree(vdev->regions);
+ return -EINVAL;
+}
+
+static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
+{
+ vdev->num_regions = 0;
+ kfree(vdev->regions);
+}
+
static void vfio_platform_release(void *device_data)
{
+ struct vfio_platform_device *vdev = device_data;
+
+ if (atomic_dec_and_test(&vdev->refcnt))
+ vfio_platform_regions_cleanup(vdev);
+
module_put(THIS_MODULE);
}

static int vfio_platform_open(void *device_data)
{
+ struct vfio_platform_device *vdev = device_data;
+ int ret;
+
if (!try_module_get(THIS_MODULE))
return -ENODEV;

+ if (atomic_inc_return(&vdev->refcnt) == 1) {
+ ret = vfio_platform_regions_init(vdev);
+ if (ret)
+ goto err_reg;
+ }
+
return 0;
+
+err_reg:
+ module_put(THIS_MODULE);
+ return ret;
}

static long vfio_platform_ioctl(void *device_data,
@@ -58,15 +125,33 @@ static long vfio_platform_ioctl(void *device_data,
return -EINVAL;

info.flags = vdev->flags;
- info.num_regions = 0;
+ info.num_regions = vdev->num_regions;
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_REGION_INFO) {
+ struct vfio_region_info info;
+
+ 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 (info.index >= vdev->num_regions)
+ return -EINVAL;
+
+ /* map offset to the physical address */
+ info.offset = VFIO_PLATFORM_INDEX_TO_OFFSET(info.index);
+ info.size = vdev->regions[info.index].size;
+ info.flags = vdev->regions[info.index].flags;
+
+ return copy_to_user((void __user *)arg, &info, minsz);

- else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
+ } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
return -EINVAL;

else if (cmd == VFIO_DEVICE_SET_IRQS)
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index ef76737..2a06035 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -15,7 +15,29 @@
#ifndef VFIO_PLATFORM_PRIVATE_H
#define VFIO_PLATFORM_PRIVATE_H

+#define VFIO_PLATFORM_OFFSET_SHIFT 40
+#define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1)
+
+#define VFIO_PLATFORM_OFFSET_TO_INDEX(off) \
+ (off >> VFIO_PLATFORM_OFFSET_SHIFT)
+
+#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \
+ ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT)
+
+struct vfio_platform_region {
+ u64 addr;
+ resource_size_t size;
+ u32 flags;
+ u32 type;
+#define VFIO_PLATFORM_REGION_TYPE_MMIO 1
+#define VFIO_PLATFORM_REGION_TYPE_PIO 2
+};
+
struct vfio_platform_device {
+ struct vfio_platform_region *regions;
+ u32 num_regions;
+ atomic_t refcnt;
+
/*
* These fields should be filled by the bus driver at binding time
*/
--
2.1.1
Alex Williamson
2014-10-21 16:34:42 UTC
Permalink
Post by Antonios Motakis
This patch enables the IOCTLs VFIO_DEVICE_GET_REGION_INFO ioctl call,
which allows the user to learn about the available MMIO resources of
a device.
---
drivers/vfio/platform/vfio_platform_common.c | 93 +++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 22 +++++++
2 files changed, 111 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 1e4073f..8a7e474 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -27,17 +27,84 @@
#include "vfio_platform_private.h"
+static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
+{
+ int cnt = 0, i;
+
+ while (vdev->get_resource(vdev, cnt))
+ cnt++;
+
+ vdev->regions = kcalloc(cnt, sizeof(struct vfio_platform_region),
+ GFP_KERNEL);
+ if (!vdev->regions)
+ return -ENOMEM;
+
+ for (i = 0; i < cnt; i++) {
+ struct resource *res =
+ vdev->get_resource(vdev, i);
+
+ if (!res)
+ goto err;
+
+ vdev->regions[i].addr = res->start;
+ vdev->regions[i].size = resource_size(res);
+ vdev->regions[i].flags = 0;
+
+ switch (resource_type(res)) {
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_MMIO;
+ break;
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
+ break;
Ok, we have support for PIO in platform now (thanks!), does the user
know what type of region they're dealing with? Do they care? For PCI
the user tests the PCI BAR in config space to determine which type it
is. I'm guessing that platform would do something similar against the
device tree or ACPI, right?
Post by Antonios Motakis
+ goto err;
+ }
+ }
+
+ vdev->num_regions = cnt;
+
+ return 0;
+ kfree(vdev->regions);
+ return -EINVAL;
+}
+
+static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
+{
+ vdev->num_regions = 0;
+ kfree(vdev->regions);
+}
+
static void vfio_platform_release(void *device_data)
{
+ struct vfio_platform_device *vdev = device_data;
+
+ if (atomic_dec_and_test(&vdev->refcnt))
+ vfio_platform_regions_cleanup(vdev);
+
module_put(THIS_MODULE);
}
static int vfio_platform_open(void *device_data)
{
+ struct vfio_platform_device *vdev = device_data;
+ int ret;
+
if (!try_module_get(THIS_MODULE))
return -ENODEV;
+ if (atomic_inc_return(&vdev->refcnt) == 1) {
+ ret = vfio_platform_regions_init(vdev);
+ if (ret)
+ goto err_reg;
+ }
+
return 0;
+
+ module_put(THIS_MODULE);
+ return ret;
Note that if vfio_platform_regions_init() fails then your refcnt is
wrong. We switched to a mutex in vfio-pci to avoid this. See
61d792562b53.
Post by Antonios Motakis
}
static long vfio_platform_ioctl(void *device_data,
@@ -58,15 +125,33 @@ static long vfio_platform_ioctl(void *device_data,
return -EINVAL;
info.flags = vdev->flags;
- info.num_regions = 0;
+ info.num_regions = vdev->num_regions;
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_REGION_INFO) {
+ struct vfio_region_info info;
+
+ 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 (info.index >= vdev->num_regions)
+ return -EINVAL;
+
+ /* map offset to the physical address */
+ info.offset = VFIO_PLATFORM_INDEX_TO_OFFSET(info.index);
+ info.size = vdev->regions[info.index].size;
+ info.flags = vdev->regions[info.index].flags;
+
+ return copy_to_user((void __user *)arg, &info, minsz);
- else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
+ } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
return -EINVAL;
else if (cmd == VFIO_DEVICE_SET_IRQS)
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index ef76737..2a06035 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -15,7 +15,29 @@
#ifndef VFIO_PLATFORM_PRIVATE_H
#define VFIO_PLATFORM_PRIVATE_H
+#define VFIO_PLATFORM_OFFSET_SHIFT 40
+#define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1)
+
+#define VFIO_PLATFORM_OFFSET_TO_INDEX(off) \
+ (off >> VFIO_PLATFORM_OFFSET_SHIFT)
+
+#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \
+ ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT)
+
+struct vfio_platform_region {
+ u64 addr;
+ resource_size_t size;
+ u32 flags;
+ u32 type;
+#define VFIO_PLATFORM_REGION_TYPE_MMIO 1
+#define VFIO_PLATFORM_REGION_TYPE_PIO 2
+};
+
struct vfio_platform_device {
+ struct vfio_platform_region *regions;
+ u32 num_regions;
+ atomic_t refcnt;
+
/*
* These fields should be filled by the bus driver at binding time
*/
B***@public.gmane.org
2014-10-21 18:18:50 UTC
Permalink
-----Original Message-----
Sent: Tuesday, October 21, 2014 10:05 PM
To: Antonios Motakis
Subject: Re: [PATCH v8 07/18] vfio/platform: return info for device memory
mapped IO regions
Post by Antonios Motakis
This patch enables the IOCTLs VFIO_DEVICE_GET_REGION_INFO ioctl call,
which allows the user to learn about the available MMIO resources of a
device.
---
drivers/vfio/platform/vfio_platform_common.c | 93
+++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 22 +++++++
2 files changed, 111 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c
b/drivers/vfio/platform/vfio_platform_common.c
index 1e4073f..8a7e474 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -27,17 +27,84 @@
#include "vfio_platform_private.h"
+static int vfio_platform_regions_init(struct vfio_platform_device
+*vdev) {
+ int cnt = 0, i;
+
+ while (vdev->get_resource(vdev, cnt))
+ cnt++;
+
+ vdev->regions = kcalloc(cnt, sizeof(struct vfio_platform_region),
+ GFP_KERNEL);
+ if (!vdev->regions)
+ return -ENOMEM;
+
+ for (i = 0; i < cnt; i++) {
+ struct resource *res =
+ vdev->get_resource(vdev, i);
+
+ if (!res)
+ goto err;
+
+ vdev->regions[i].addr = res->start;
+ vdev->regions[i].size = resource_size(res);
+ vdev->regions[i].flags = 0;
+
+ switch (resource_type(res)) {
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_MMIO;
+ break;
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
+ break;
Ok, we have support for PIO in platform now (thanks!), does the user know what
type of region they're dealing with? Do they care? For PCI the user tests the
PCI BAR in config space to determine which type it is. I'm guessing that
platform would do something similar against the device tree or ACPI, right?
Interested in knowing what type of PIO region in a platform device, is this for I2C/SPI type of device?


Thanks
-Bharat
Post by Antonios Motakis
+ goto err;
+ }
+ }
+
+ vdev->num_regions = cnt;
+
+ return 0;
+ kfree(vdev->regions);
+ return -EINVAL;
+}
+
+static void vfio_platform_regions_cleanup(struct vfio_platform_device
+*vdev) {
+ vdev->num_regions = 0;
+ kfree(vdev->regions);
+}
+
static void vfio_platform_release(void *device_data) {
+ struct vfio_platform_device *vdev = device_data;
+
+ if (atomic_dec_and_test(&vdev->refcnt))
+ vfio_platform_regions_cleanup(vdev);
+
module_put(THIS_MODULE);
}
static int vfio_platform_open(void *device_data) {
+ struct vfio_platform_device *vdev = device_data;
+ int ret;
+
if (!try_module_get(THIS_MODULE))
return -ENODEV;
+ if (atomic_inc_return(&vdev->refcnt) == 1) {
+ ret = vfio_platform_regions_init(vdev);
+ if (ret)
+ goto err_reg;
+ }
+
return 0;
+
+ module_put(THIS_MODULE);
+ return ret;
Note that if vfio_platform_regions_init() fails then your refcnt is wrong. We
switched to a mutex in vfio-pci to avoid this. See 61d792562b53.
Post by Antonios Motakis
}
@@ static long vfio_platform_ioctl(void *device_data,
return -EINVAL;
info.flags = vdev->flags;
- info.num_regions = 0;
+ info.num_regions = vdev->num_regions;
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_REGION_INFO) {
+ struct vfio_region_info info;
+
+ 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 (info.index >= vdev->num_regions)
+ return -EINVAL;
+
+ /* map offset to the physical address */
+ info.offset = VFIO_PLATFORM_INDEX_TO_OFFSET(info.index);
+ info.size = vdev->regions[info.index].size;
+ info.flags = vdev->regions[info.index].flags;
+
+ return copy_to_user((void __user *)arg, &info, minsz);
- else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
+ } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
return -EINVAL;
else if (cmd == VFIO_DEVICE_SET_IRQS) diff --git
a/drivers/vfio/platform/vfio_platform_private.h
b/drivers/vfio/platform/vfio_platform_private.h
index ef76737..2a06035 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -15,7 +15,29 @@
#ifndef VFIO_PLATFORM_PRIVATE_H
#define VFIO_PLATFORM_PRIVATE_H
+#define VFIO_PLATFORM_OFFSET_SHIFT 40
+#define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) <<
+VFIO_PLATFORM_OFFSET_SHIFT) - 1)
+
+#define VFIO_PLATFORM_OFFSET_TO_INDEX(off) \
+ (off >> VFIO_PLATFORM_OFFSET_SHIFT)
+
+#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \
+ ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT)
+
+struct vfio_platform_region {
+ u64 addr;
+ resource_size_t size;
+ u32 flags;
+ u32 type;
+#define VFIO_PLATFORM_REGION_TYPE_MMIO 1
+#define VFIO_PLATFORM_REGION_TYPE_PIO 2
+};
+
struct vfio_platform_device {
+ struct vfio_platform_region *regions;
+ u32 num_regions;
+ atomic_t refcnt;
+
/*
* These fields should be filled by the bus driver at binding time
*/
_______________________________________________
iommu mailing list
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Alex Williamson
2014-10-21 18:57:26 UTC
Permalink
Post by B***@public.gmane.org
-----Original Message-----
Sent: Tuesday, October 21, 2014 10:05 PM
To: Antonios Motakis
Subject: Re: [PATCH v8 07/18] vfio/platform: return info for device memory
mapped IO regions
Post by Antonios Motakis
This patch enables the IOCTLs VFIO_DEVICE_GET_REGION_INFO ioctl call,
which allows the user to learn about the available MMIO resources of a
device.
---
drivers/vfio/platform/vfio_platform_common.c | 93
+++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 22 +++++++
2 files changed, 111 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c
b/drivers/vfio/platform/vfio_platform_common.c
index 1e4073f..8a7e474 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -27,17 +27,84 @@
#include "vfio_platform_private.h"
+static int vfio_platform_regions_init(struct vfio_platform_device
+*vdev) {
+ int cnt = 0, i;
+
+ while (vdev->get_resource(vdev, cnt))
+ cnt++;
+
+ vdev->regions = kcalloc(cnt, sizeof(struct vfio_platform_region),
+ GFP_KERNEL);
+ if (!vdev->regions)
+ return -ENOMEM;
+
+ for (i = 0; i < cnt; i++) {
+ struct resource *res =
+ vdev->get_resource(vdev, i);
+
+ if (!res)
+ goto err;
+
+ vdev->regions[i].addr = res->start;
+ vdev->regions[i].size = resource_size(res);
+ vdev->regions[i].flags = 0;
+
+ switch (resource_type(res)) {
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_MMIO;
+ break;
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
+ break;
Ok, we have support for PIO in platform now (thanks!), does the user know what
type of region they're dealing with? Do they care? For PCI the user tests the
PCI BAR in config space to determine which type it is. I'm guessing that
platform would do something similar against the device tree or ACPI, right?
Interested in knowing what type of PIO region in a platform device, is this for I2C/SPI type of device?
I don't think we have any specific users, I had noted in a previous
version that we were searching only for MMIO regions and we at least
needed to figure out how PIO regions would be exposed so we don't have
compatibility issues when adding them (ie. how to match a vfio resource
index to a sysfs/dt/acpi description when we're only reporting a subset
of resources). Antonios has done a good job in this version in adding
all but the accessor functions for PIO. x86 could theoretically have
ACPI defined devices that are exposed as platform devices that could be
exposed using this code. Thanks,

Alex
Post by B***@public.gmane.org
Post by Antonios Motakis
+ goto err;
+ }
+ }
+
+ vdev->num_regions = cnt;
+
+ return 0;
+ kfree(vdev->regions);
+ return -EINVAL;
+}
+
+static void vfio_platform_regions_cleanup(struct vfio_platform_device
+*vdev) {
+ vdev->num_regions = 0;
+ kfree(vdev->regions);
+}
+
static void vfio_platform_release(void *device_data) {
+ struct vfio_platform_device *vdev = device_data;
+
+ if (atomic_dec_and_test(&vdev->refcnt))
+ vfio_platform_regions_cleanup(vdev);
+
module_put(THIS_MODULE);
}
static int vfio_platform_open(void *device_data) {
+ struct vfio_platform_device *vdev = device_data;
+ int ret;
+
if (!try_module_get(THIS_MODULE))
return -ENODEV;
+ if (atomic_inc_return(&vdev->refcnt) == 1) {
+ ret = vfio_platform_regions_init(vdev);
+ if (ret)
+ goto err_reg;
+ }
+
return 0;
+
+ module_put(THIS_MODULE);
+ return ret;
Note that if vfio_platform_regions_init() fails then your refcnt is wrong. We
switched to a mutex in vfio-pci to avoid this. See 61d792562b53.
Post by Antonios Motakis
}
@@ static long vfio_platform_ioctl(void *device_data,
return -EINVAL;
info.flags = vdev->flags;
- info.num_regions = 0;
+ info.num_regions = vdev->num_regions;
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_REGION_INFO) {
+ struct vfio_region_info info;
+
+ 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 (info.index >= vdev->num_regions)
+ return -EINVAL;
+
+ /* map offset to the physical address */
+ info.offset = VFIO_PLATFORM_INDEX_TO_OFFSET(info.index);
+ info.size = vdev->regions[info.index].size;
+ info.flags = vdev->regions[info.index].flags;
+
+ return copy_to_user((void __user *)arg, &info, minsz);
- else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
+ } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
return -EINVAL;
else if (cmd == VFIO_DEVICE_SET_IRQS) diff --git
a/drivers/vfio/platform/vfio_platform_private.h
b/drivers/vfio/platform/vfio_platform_private.h
index ef76737..2a06035 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -15,7 +15,29 @@
#ifndef VFIO_PLATFORM_PRIVATE_H
#define VFIO_PLATFORM_PRIVATE_H
+#define VFIO_PLATFORM_OFFSET_SHIFT 40
+#define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) <<
+VFIO_PLATFORM_OFFSET_SHIFT) - 1)
+
+#define VFIO_PLATFORM_OFFSET_TO_INDEX(off) \
+ (off >> VFIO_PLATFORM_OFFSET_SHIFT)
+
+#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \
+ ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT)
+
+struct vfio_platform_region {
+ u64 addr;
+ resource_size_t size;
+ u32 flags;
+ u32 type;
+#define VFIO_PLATFORM_REGION_TYPE_MMIO 1
+#define VFIO_PLATFORM_REGION_TYPE_PIO 2
+};
+
struct vfio_platform_device {
+ struct vfio_platform_region *regions;
+ u32 num_regions;
+ atomic_t refcnt;
+
/*
* These fields should be filled by the bus driver at binding time
*/
_______________________________________________
iommu mailing list
https://lists.linuxfoundation.org/mailman/listinfo/iommu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Antonios Motakis
2014-10-22 13:54:46 UTC
Permalink
On Tue, Oct 21, 2014 at 6:34 PM, Alex Williamson
Post by Alex Williamson
Post by Antonios Motakis
This patch enables the IOCTLs VFIO_DEVICE_GET_REGION_INFO ioctl call,
which allows the user to learn about the available MMIO resources of
a device.
---
drivers/vfio/platform/vfio_platform_common.c | 93 +++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 22 +++++++
2 files changed, 111 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 1e4073f..8a7e474 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -27,17 +27,84 @@
#include "vfio_platform_private.h"
+static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
+{
+ int cnt = 0, i;
+
+ while (vdev->get_resource(vdev, cnt))
+ cnt++;
+
+ vdev->regions = kcalloc(cnt, sizeof(struct vfio_platform_region),
+ GFP_KERNEL);
+ if (!vdev->regions)
+ return -ENOMEM;
+
+ for (i = 0; i < cnt; i++) {
+ struct resource *res =
+ vdev->get_resource(vdev, i);
+
+ if (!res)
+ goto err;
+
+ vdev->regions[i].addr = res->start;
+ vdev->regions[i].size = resource_size(res);
+ vdev->regions[i].flags = 0;
+
+ switch (resource_type(res)) {
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_MMIO;
+ break;
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
+ break;
Ok, we have support for PIO in platform now (thanks!), does the user
know what type of region they're dealing with? Do they care? For PCI
the user tests the PCI BAR in config space to determine which type it
is. I'm guessing that platform would do something similar against the
device tree or ACPI, right?
Maybe is worthwhile to add an explicit flag in
VFIO_DEVICE_GET_REGION_INFO for PIO regions. For platform devices I
don't know if we can always rely on DT or ACPI info to be available.
For VFIO PCI the BAR is always implemented, and while I have proposed
an RFC to return DT information, I don't think we can assume how a
device is described in the host, whether DT, ACPI, or dark magic.
Post by Alex Williamson
Post by Antonios Motakis
+ goto err;
+ }
+ }
+
+ vdev->num_regions = cnt;
+
+ return 0;
+ kfree(vdev->regions);
+ return -EINVAL;
+}
+
+static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
+{
+ vdev->num_regions = 0;
+ kfree(vdev->regions);
+}
+
static void vfio_platform_release(void *device_data)
{
+ struct vfio_platform_device *vdev = device_data;
+
+ if (atomic_dec_and_test(&vdev->refcnt))
+ vfio_platform_regions_cleanup(vdev);
+
module_put(THIS_MODULE);
}
static int vfio_platform_open(void *device_data)
{
+ struct vfio_platform_device *vdev = device_data;
+ int ret;
+
if (!try_module_get(THIS_MODULE))
return -ENODEV;
+ if (atomic_inc_return(&vdev->refcnt) == 1) {
+ ret = vfio_platform_regions_init(vdev);
+ if (ret)
+ goto err_reg;
+ }
+
return 0;
+
+ module_put(THIS_MODULE);
+ return ret;
Note that if vfio_platform_regions_init() fails then your refcnt is
wrong. We switched to a mutex in vfio-pci to avoid this. See
61d792562b53.
Ack.
Post by Alex Williamson
Post by Antonios Motakis
}
static long vfio_platform_ioctl(void *device_data,
@@ -58,15 +125,33 @@ static long vfio_platform_ioctl(void *device_data,
return -EINVAL;
info.flags = vdev->flags;
- info.num_regions = 0;
+ info.num_regions = vdev->num_regions;
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_REGION_INFO) {
+ struct vfio_region_info info;
+
+ 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 (info.index >= vdev->num_regions)
+ return -EINVAL;
+
+ /* map offset to the physical address */
+ info.offset = VFIO_PLATFORM_INDEX_TO_OFFSET(info.index);
+ info.size = vdev->regions[info.index].size;
+ info.flags = vdev->regions[info.index].flags;
+
+ return copy_to_user((void __user *)arg, &info, minsz);
- else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
+ } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
return -EINVAL;
else if (cmd == VFIO_DEVICE_SET_IRQS)
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index ef76737..2a06035 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -15,7 +15,29 @@
#ifndef VFIO_PLATFORM_PRIVATE_H
#define VFIO_PLATFORM_PRIVATE_H
+#define VFIO_PLATFORM_OFFSET_SHIFT 40
+#define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1)
+
+#define VFIO_PLATFORM_OFFSET_TO_INDEX(off) \
+ (off >> VFIO_PLATFORM_OFFSET_SHIFT)
+
+#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \
+ ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT)
+
+struct vfio_platform_region {
+ u64 addr;
+ resource_size_t size;
+ u32 flags;
+ u32 type;
+#define VFIO_PLATFORM_REGION_TYPE_MMIO 1
+#define VFIO_PLATFORM_REGION_TYPE_PIO 2
+};
+
struct vfio_platform_device {
+ struct vfio_platform_region *regions;
+ u32 num_regions;
+ atomic_t refcnt;
+
/*
* These fields should be filled by the bus driver at binding time
*/
--
Antonios Motakis
Virtual Open Systems
Alex Williamson
2014-10-22 16:46:29 UTC
Permalink
Post by Antonios Motakis
On Tue, Oct 21, 2014 at 6:34 PM, Alex Williamson
Post by Alex Williamson
Post by Antonios Motakis
This patch enables the IOCTLs VFIO_DEVICE_GET_REGION_INFO ioctl call,
which allows the user to learn about the available MMIO resources of
a device.
---
drivers/vfio/platform/vfio_platform_common.c | 93 +++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 22 +++++++
2 files changed, 111 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 1e4073f..8a7e474 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -27,17 +27,84 @@
#include "vfio_platform_private.h"
+static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
+{
+ int cnt = 0, i;
+
+ while (vdev->get_resource(vdev, cnt))
+ cnt++;
+
+ vdev->regions = kcalloc(cnt, sizeof(struct vfio_platform_region),
+ GFP_KERNEL);
+ if (!vdev->regions)
+ return -ENOMEM;
+
+ for (i = 0; i < cnt; i++) {
+ struct resource *res =
+ vdev->get_resource(vdev, i);
+
+ if (!res)
+ goto err;
+
+ vdev->regions[i].addr = res->start;
+ vdev->regions[i].size = resource_size(res);
+ vdev->regions[i].flags = 0;
+
+ switch (resource_type(res)) {
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_MMIO;
+ break;
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
+ break;
Ok, we have support for PIO in platform now (thanks!), does the user
know what type of region they're dealing with? Do they care? For PCI
the user tests the PCI BAR in config space to determine which type it
is. I'm guessing that platform would do something similar against the
device tree or ACPI, right?
Maybe is worthwhile to add an explicit flag in
VFIO_DEVICE_GET_REGION_INFO for PIO regions. For platform devices I
don't know if we can always rely on DT or ACPI info to be available.
For VFIO PCI the BAR is always implemented, and while I have proposed
an RFC to return DT information, I don't think we can assume how a
device is described in the host, whether DT, ACPI, or dark magic.
Is this already handled by the fact that vfio-platform is not meant to
be a generic meta driver to the same extent as vfio-pci? There is no
self-describing config space on platform devices like there is on PCI,
so the user will need to know in advance somehow what the device is and
what resources/irqs it uses. We do need to make sure though that we
provide a userspace ABI that allows them to match VFIO indexes to the
device in a predictable way. It's not fully clear to me how that works.
Thanks,

Alex
Antonios Motakis
2014-10-22 17:53:06 UTC
Permalink
On Wed, Oct 22, 2014 at 6:46 PM, Alex Williamson
Post by Alex Williamson
Post by Antonios Motakis
On Tue, Oct 21, 2014 at 6:34 PM, Alex Williamson
Post by Alex Williamson
Post by Antonios Motakis
This patch enables the IOCTLs VFIO_DEVICE_GET_REGION_INFO ioctl call,
which allows the user to learn about the available MMIO resources of
a device.
---
drivers/vfio/platform/vfio_platform_common.c | 93 +++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 22 +++++++
2 files changed, 111 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 1e4073f..8a7e474 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -27,17 +27,84 @@
#include "vfio_platform_private.h"
+static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
+{
+ int cnt = 0, i;
+
+ while (vdev->get_resource(vdev, cnt))
+ cnt++;
+
+ vdev->regions = kcalloc(cnt, sizeof(struct vfio_platform_region),
+ GFP_KERNEL);
+ if (!vdev->regions)
+ return -ENOMEM;
+
+ for (i = 0; i < cnt; i++) {
+ struct resource *res =
+ vdev->get_resource(vdev, i);
+
+ if (!res)
+ goto err;
+
+ vdev->regions[i].addr = res->start;
+ vdev->regions[i].size = resource_size(res);
+ vdev->regions[i].flags = 0;
+
+ switch (resource_type(res)) {
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_MMIO;
+ break;
+ vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
+ break;
Ok, we have support for PIO in platform now (thanks!), does the user
know what type of region they're dealing with? Do they care? For PCI
the user tests the PCI BAR in config space to determine which type it
is. I'm guessing that platform would do something similar against the
device tree or ACPI, right?
Maybe is worthwhile to add an explicit flag in
VFIO_DEVICE_GET_REGION_INFO for PIO regions. For platform devices I
don't know if we can always rely on DT or ACPI info to be available.
For VFIO PCI the BAR is always implemented, and while I have proposed
an RFC to return DT information, I don't think we can assume how a
device is described in the host, whether DT, ACPI, or dark magic.
Is this already handled by the fact that vfio-platform is not meant to
be a generic meta driver to the same extent as vfio-pci? There is no
self-describing config space on platform devices like there is on PCI,
so the user will need to know in advance somehow what the device is and
what resources/irqs it uses. We do need to make sure though that we
provide a userspace ABI that allows them to match VFIO indexes to the
device in a predictable way. It's not fully clear to me how that works.
Thanks,
Yeah, it is a fact of life that the user needs to know what regions he
is accessing, just from their ordering. Even with the extension for
accessing device tree data, the user still needs to know what each
region is and what info he is looking for from the device tree. In
that respect we could delegate the responsibility to the user to just
"know" what kind of device he is accessing and what kind of regions it
features (and in what order).
Post by Alex Williamson
Alex
--
Antonios Motakis
Virtual Open Systems
Antonios Motakis
2014-10-13 13:10:15 UTC
Permalink
VFIO returns a file descriptor which we can use to manipulate the memory
regions of the device. Usually, the user will mmap memory regions that are
addressable on page boundaries, however for memory regions where this is
not the case we cannot provide mmap functionality due to security concerns.
For this reason we also need allow to read and write to the memory regions
via the file descriptor. Implement this funcionality only for MMIO regions
of platform devices; PIO regions are not being handled at this point.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_common.c | 150 ++++++++++++++++++++++++++
drivers/vfio/platform/vfio_platform_private.h | 1 +
2 files changed, 151 insertions(+)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 8a7e474..ac74710 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -53,6 +53,10 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
switch (resource_type(res)) {
case IORESOURCE_MEM:
vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_MMIO;
+ vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
+ if (!(res->flags & IORESOURCE_READONLY))
+ vdev->regions[i].flags |=
+ VFIO_REGION_INFO_FLAG_WRITE;
break;
case IORESOURCE_IO:
vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
@@ -72,6 +76,11 @@ err:

static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
{
+ int i;
+
+ for (i = 0; i < vdev->num_regions; i++)
+ iounmap(vdev->regions[i].ioaddr);
+
vdev->num_regions = 0;
kfree(vdev->regions);
}
@@ -163,15 +172,156 @@ static long vfio_platform_ioctl(void *device_data,
return -ENOTTY;
}

+static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg,
+ char __user *buf, size_t count,
+ loff_t off)
+{
+ unsigned int done = 0;
+
+ if (!reg.ioaddr) {
+ reg.ioaddr =
+ ioremap_nocache(reg.addr, reg.size);
+
+ if (!reg.ioaddr)
+ return -ENOMEM;
+ }
+
+ while (count) {
+ size_t filled;
+
+ if (count >= 4 && !(off % 4)) {
+ u32 val;
+
+ val = ioread32(reg.ioaddr + off);
+ if (copy_to_user(buf, &val, 4))
+ goto err;
+
+ filled = 4;
+ } else if (count >= 2 && !(off % 2)) {
+ u16 val;
+
+ val = ioread16(reg.ioaddr + off);
+ if (copy_to_user(buf, &val, 2))
+ goto err;
+
+ filled = 2;
+ } else {
+ u8 val;
+
+ val = ioread8(reg.ioaddr + off);
+ if (copy_to_user(buf, &val, 1))
+ goto err;
+
+ filled = 1;
+ }
+
+
+ count -= filled;
+ done += filled;
+ off += filled;
+ buf += filled;
+ }
+
+ return done;
+err:
+ return -EFAULT;
+}
+
static ssize_t vfio_platform_read(void *device_data, char __user *buf,
size_t count, loff_t *ppos)
{
+ struct vfio_platform_device *vdev = device_data;
+ unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos);
+ loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK;
+
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+
+ if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_READ))
+ return -EINVAL;
+
+ if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
+ return vfio_platform_read_mmio(vdev->regions[index],
+ buf, count, off);
+ else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO)
+ return -EINVAL; /* not implemented */
+
return -EINVAL;
}

+static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg,
+ const char __user *buf, size_t count,
+ loff_t off)
+{
+ unsigned int done = 0;
+
+ if (!reg.ioaddr) {
+ reg.ioaddr =
+ ioremap_nocache(reg.addr, reg.size);
+
+ if (!reg.ioaddr)
+ return -ENOMEM;
+ }
+
+ while (count) {
+ size_t filled;
+
+ if (count >= 4 && !(off % 4)) {
+ u32 val;
+
+ if (copy_from_user(&val, buf, 4))
+ goto err;
+ iowrite32(val, reg.ioaddr + off);
+
+ filled = 4;
+ } else if (count >= 2 && !(off % 2)) {
+ u16 val;
+
+ if (copy_from_user(&val, buf, 2))
+ goto err;
+ iowrite16(val, reg.ioaddr + off);
+
+ filled = 2;
+ } else {
+ u8 val;
+
+ if (copy_from_user(&val, buf, 1))
+ goto err;
+ iowrite8(val, reg.ioaddr + off);
+
+ filled = 1;
+ }
+
+ count -= filled;
+ done += filled;
+ off += filled;
+ buf += filled;
+ }
+
+ return done;
+err:
+ return -EFAULT;
+}
+
static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
size_t count, loff_t *ppos)
{
+ struct vfio_platform_device *vdev = device_data;
+ unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos);
+ loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK;
+
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+
+ if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE))
+ return -EINVAL;
+
+ if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
+ return vfio_platform_write_mmio(vdev->regions[index],
+ buf, count, off);
+ else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO)
+ return -EINVAL; /* not implemented */
+
return -EINVAL;
}

diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 2a06035..db9f88d 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -31,6 +31,7 @@ struct vfio_platform_region {
u32 type;
#define VFIO_PLATFORM_REGION_TYPE_MMIO 1
#define VFIO_PLATFORM_REGION_TYPE_PIO 2
+ void __iomem *ioaddr;
};

struct vfio_platform_device {
--
2.1.1
Antonios Motakis
2014-10-13 13:10:16 UTC
Permalink
Allow to memory map the MMIO regions of the device so userspace can
directly access them. PIO regions are not being handled at this point.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_common.c | 57 ++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index ac74710..4db7187 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -57,6 +57,16 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
if (!(res->flags & IORESOURCE_READONLY))
vdev->regions[i].flags |=
VFIO_REGION_INFO_FLAG_WRITE;
+
+ /*
+ * Only regions addressed with PAGE granularity may be
+ * MMAPed securely.
+ */
+ if (!(vdev->regions[i].addr & ~PAGE_MASK) &&
+ !(vdev->regions[i].size & ~PAGE_MASK))
+ vdev->regions[i].flags |=
+ VFIO_REGION_INFO_FLAG_MMAP;
+
break;
case IORESOURCE_IO:
vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
@@ -325,8 +335,55 @@ static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
return -EINVAL;
}

+static int vfio_platform_mmap_mmio(struct vfio_platform_region region,
+ struct vm_area_struct *vma)
+{
+ u64 req_len, pgoff, req_start;
+
+ req_len = vma->vm_end - vma->vm_start;
+ pgoff = vma->vm_pgoff &
+ ((1U << (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
+ req_start = pgoff << PAGE_SHIFT;
+
+ if (region.size < PAGE_SIZE || req_start + req_len > region.size)
+ return -EINVAL;
+
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+ vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
+
+ return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+ req_len, vma->vm_page_prot);
+}
+
static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
{
+ struct vfio_platform_device *vdev = device_data;
+ unsigned int index;
+
+ index = vma->vm_pgoff >> (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT);
+
+ if (vma->vm_end < vma->vm_start)
+ return -EINVAL;
+ if ((vma->vm_flags & VM_SHARED) == 0)
+ return -EINVAL;
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+ if (vma->vm_start & ~PAGE_MASK)
+ return -EINVAL;
+ if (vma->vm_end & ~PAGE_MASK)
+ return -EINVAL;
+
+ if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP))
+ return -EINVAL;
+
+ vma->vm_private_data = vdev;
+
+ if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
+ return vfio_platform_mmap_mmio(vdev->regions[index], vma);
+
+ else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO)
+ return -EINVAL; /* not implemented */
+
return -EINVAL;
}
--
2.1.1
Alex Williamson
2014-10-21 16:51:47 UTC
Permalink
Post by Antonios Motakis
Allow to memory map the MMIO regions of the device so userspace can
directly access them. PIO regions are not being handled at this point.
---
drivers/vfio/platform/vfio_platform_common.c | 57 ++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index ac74710..4db7187 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -57,6 +57,16 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
if (!(res->flags & IORESOURCE_READONLY))
vdev->regions[i].flags |=
VFIO_REGION_INFO_FLAG_WRITE;
+
+ /*
+ * Only regions addressed with PAGE granularity may be
+ * MMAPed securely.
+ */
+ if (!(vdev->regions[i].addr & ~PAGE_MASK) &&
+ !(vdev->regions[i].size & ~PAGE_MASK))
+ vdev->regions[i].flags |=
+ VFIO_REGION_INFO_FLAG_MMAP;
+
Should this be included in the above !readonly test? I don't see that
we're doing anything below that would prevent writes to the mmap for a
readonly resource. I suspect that just like PCI, it's not all that
useful to provide mmap support for read-only regions. They're not
typically performance paths.
Post by Antonios Motakis
break;
vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
@@ -325,8 +335,55 @@ static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
return -EINVAL;
}
+static int vfio_platform_mmap_mmio(struct vfio_platform_region region,
+ struct vm_area_struct *vma)
+{
+ u64 req_len, pgoff, req_start;
+
+ req_len = vma->vm_end - vma->vm_start;
+ pgoff = vma->vm_pgoff &
+ ((1U << (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
+ req_start = pgoff << PAGE_SHIFT;
+
+ if (region.size < PAGE_SIZE || req_start + req_len > region.size)
+ return -EINVAL;
+
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+ vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
+
+ return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+ req_len, vma->vm_page_prot);
+}
+
static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
{
+ struct vfio_platform_device *vdev = device_data;
+ unsigned int index;
+
+ index = vma->vm_pgoff >> (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT);
+
+ if (vma->vm_end < vma->vm_start)
+ return -EINVAL;
+ if ((vma->vm_flags & VM_SHARED) == 0)
+ return -EINVAL;
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+ if (vma->vm_start & ~PAGE_MASK)
+ return -EINVAL;
+ if (vma->vm_end & ~PAGE_MASK)
+ return -EINVAL;
+
+ if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP))
+ return -EINVAL;
+
+ vma->vm_private_data = vdev;
+
+ if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
+ return vfio_platform_mmap_mmio(vdev->regions[index], vma);
+
+ else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO)
+ return -EINVAL; /* not implemented */
+
return -EINVAL;
}
Antonios Motakis
2014-10-22 13:55:11 UTC
Permalink
On Tue, Oct 21, 2014 at 6:51 PM, Alex Williamson
Post by Alex Williamson
Post by Antonios Motakis
Allow to memory map the MMIO regions of the device so userspace can
directly access them. PIO regions are not being handled at this point.
---
drivers/vfio/platform/vfio_platform_common.c | 57 ++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index ac74710..4db7187 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -57,6 +57,16 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
if (!(res->flags & IORESOURCE_READONLY))
vdev->regions[i].flags |=
VFIO_REGION_INFO_FLAG_WRITE;
+
+ /*
+ * Only regions addressed with PAGE granularity may be
+ * MMAPed securely.
+ */
+ if (!(vdev->regions[i].addr & ~PAGE_MASK) &&
+ !(vdev->regions[i].size & ~PAGE_MASK))
+ vdev->regions[i].flags |=
+ VFIO_REGION_INFO_FLAG_MMAP;
+
Should this be included in the above !readonly test? I don't see that
we're doing anything below that would prevent writes to the mmap for a
readonly resource. I suspect that just like PCI, it's not all that
useful to provide mmap support for read-only regions. They're not
typically performance paths.
Indeed. Alternatively we could just MMAP the region as read only as
well. Even if typically they are not performance critical areas, maybe
it doesn't hurt to allow MMAP when possible.
Post by Alex Williamson
Post by Antonios Motakis
break;
vdev->regions[i].type = VFIO_PLATFORM_REGION_TYPE_PIO;
@@ -325,8 +335,55 @@ static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
return -EINVAL;
}
+static int vfio_platform_mmap_mmio(struct vfio_platform_region region,
+ struct vm_area_struct *vma)
+{
+ u64 req_len, pgoff, req_start;
+
+ req_len = vma->vm_end - vma->vm_start;
+ pgoff = vma->vm_pgoff &
+ ((1U << (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
+ req_start = pgoff << PAGE_SHIFT;
+
+ if (region.size < PAGE_SIZE || req_start + req_len > region.size)
+ return -EINVAL;
+
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+ vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
+
+ return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+ req_len, vma->vm_page_prot);
+}
+
static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
{
+ struct vfio_platform_device *vdev = device_data;
+ unsigned int index;
+
+ index = vma->vm_pgoff >> (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT);
+
+ if (vma->vm_end < vma->vm_start)
+ return -EINVAL;
+ if ((vma->vm_flags & VM_SHARED) == 0)
+ return -EINVAL;
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+ if (vma->vm_start & ~PAGE_MASK)
+ return -EINVAL;
+ if (vma->vm_end & ~PAGE_MASK)
+ return -EINVAL;
+
+ if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP))
+ return -EINVAL;
+
+ vma->vm_private_data = vdev;
+
+ if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
+ return vfio_platform_mmap_mmio(vdev->regions[index], vma);
+
+ else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO)
+ return -EINVAL; /* not implemented */
+
return -EINVAL;
}
--
Antonios Motakis
Virtual Open Systems
Alex Williamson
2014-10-22 16:56:15 UTC
Permalink
Post by Antonios Motakis
On Tue, Oct 21, 2014 at 6:51 PM, Alex Williamson
Post by Alex Williamson
Post by Antonios Motakis
Allow to memory map the MMIO regions of the device so userspace can
directly access them. PIO regions are not being handled at this point.
---
drivers/vfio/platform/vfio_platform_common.c | 57 ++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index ac74710..4db7187 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -57,6 +57,16 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
if (!(res->flags & IORESOURCE_READONLY))
vdev->regions[i].flags |=
VFIO_REGION_INFO_FLAG_WRITE;
+
+ /*
+ * Only regions addressed with PAGE granularity may be
+ * MMAPed securely.
+ */
+ if (!(vdev->regions[i].addr & ~PAGE_MASK) &&
+ !(vdev->regions[i].size & ~PAGE_MASK))
+ vdev->regions[i].flags |=
+ VFIO_REGION_INFO_FLAG_MMAP;
+
Should this be included in the above !readonly test? I don't see that
we're doing anything below that would prevent writes to the mmap for a
readonly resource. I suspect that just like PCI, it's not all that
useful to provide mmap support for read-only regions. They're not
typically performance paths.
Indeed. Alternatively we could just MMAP the region as read only as
well. Even if typically they are not performance critical areas, maybe
it doesn't hurt to allow MMAP when possible.
Sure, we could allow a read-only mmap. On PCI the only read-only region
is the PCI option ROM. The PCI spec indicates that devices may use the
same address decoders for the ROM as for BARs, so the ROM and BAR may
not be simultaneously accessible. QEMU also caches the ROM once the
guest does read it, so it's never been an issue to allow mmap of that
resource. I would probably only bother to provide read-only mmap if you
know vfio-platform devices are going to make more significant use of
read-only regions. Thanks,

Alex
Antonios Motakis
2014-10-13 13:10:17 UTC
Permalink
Return information for the interrupts exposed by the device.
This patch extends VFIO_DEVICE_GET_INFO with the number of IRQs
and enables VFIO_DEVICE_GET_IRQ_INFO.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/Makefile | 2 +-
drivers/vfio/platform/vfio_platform_common.c | 34 ++++++++++++---
drivers/vfio/platform/vfio_platform_irq.c | 59 +++++++++++++++++++++++++++
drivers/vfio/platform/vfio_platform_private.h | 10 +++++
4 files changed, 99 insertions(+), 6 deletions(-)
create mode 100644 drivers/vfio/platform/vfio_platform_irq.c

diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
index 1957170..81de144 100644
--- a/drivers/vfio/platform/Makefile
+++ b/drivers/vfio/platform/Makefile
@@ -1,5 +1,5 @@

-vfio-platform-y := vfio_platform.o vfio_platform_common.o
+vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o

obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 4db7187..c26a545 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -99,8 +99,10 @@ static void vfio_platform_release(void *device_data)
{
struct vfio_platform_device *vdev = device_data;

- if (atomic_dec_and_test(&vdev->refcnt))
+ if (atomic_dec_and_test(&vdev->refcnt)) {
vfio_platform_regions_cleanup(vdev);
+ vfio_platform_irq_cleanup(vdev);
+ }

module_put(THIS_MODULE);
}
@@ -117,10 +119,16 @@ static int vfio_platform_open(void *device_data)
ret = vfio_platform_regions_init(vdev);
if (ret)
goto err_reg;
+
+ ret = vfio_platform_irq_init(vdev);
+ if (ret)
+ goto err_irq;
}

return 0;

+err_irq:
+ vfio_platform_regions_cleanup(vdev);
err_reg:
module_put(THIS_MODULE);
return ret;
@@ -145,7 +153,7 @@ static long vfio_platform_ioctl(void *device_data,

info.flags = vdev->flags;
info.num_regions = vdev->num_regions;
- info.num_irqs = 0;
+ info.num_irqs = vdev->num_irqs;

return copy_to_user((void __user *)arg, &info, minsz);

@@ -170,10 +178,26 @@ static long vfio_platform_ioctl(void *device_data,

return copy_to_user((void __user *)arg, &info, minsz);

- } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
- return -EINVAL;
+ } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
+ struct vfio_irq_info info;
+
+ minsz = offsetofend(struct vfio_irq_info, count);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ if (info.index >= vdev->num_irqs)
+ return -EINVAL;
+
+ info.flags = vdev->irqs[info.index].flags;
+ info.count = vdev->irqs[info.index].count;
+
+ 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)
diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
new file mode 100644
index 0000000..d99c71c
--- /dev/null
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -0,0 +1,59 @@
+/*
+ * VFIO platform devices interrupt handling
+ *
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@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/platform_device.h>
+#include <linux/irq.h>
+
+#include "vfio_platform_private.h"
+
+int vfio_platform_irq_init(struct vfio_platform_device *vdev)
+{
+ int cnt = 0, i;
+
+ while (vdev->get_irq(vdev, cnt) > 0)
+ cnt++;
+
+ vdev->irqs = kcalloc(cnt, sizeof(struct vfio_platform_irq), GFP_KERNEL);
+ if (!vdev->irqs)
+ return -ENOMEM;
+
+ for (i = 0; i < cnt; i++) {
+ vdev->irqs[i].flags = 0;
+ vdev->irqs[i].count = 1;
+ }
+
+ vdev->num_irqs = cnt;
+
+ return 0;
+}
+
+void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev)
+{
+ vdev->num_irqs = 0;
+ kfree(vdev->irqs);
+}
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index db9f88d..196c530 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -24,6 +24,11 @@
#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \
((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT)

+struct vfio_platform_irq {
+ u32 flags;
+ u32 count;
+};
+
struct vfio_platform_region {
u64 addr;
resource_size_t size;
@@ -37,6 +42,8 @@ struct vfio_platform_region {
struct vfio_platform_device {
struct vfio_platform_region *regions;
u32 num_regions;
+ struct vfio_platform_irq *irqs;
+ u32 num_irqs;
atomic_t refcnt;

/*
@@ -55,4 +62,7 @@ extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
struct device *dev);
extern int vfio_platform_remove_common(struct device *dev);

+extern int vfio_platform_irq_init(struct vfio_platform_device *vdev);
+extern void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev);
+
#endif /* VFIO_PLATFORM_PRIVATE_H */
--
2.1.1
Antonios Motakis
2014-10-13 13:10:18 UTC
Permalink
This patch is a skeleton for the VFIO_DEVICE_SET_IRQS IOCTL, around which
most IRQ functionality is implemented in VFIO.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_common.c | 52 +++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_irq.c | 56 +++++++++++++++++++++++++++
drivers/vfio/platform/vfio_platform_private.h | 6 +++
3 files changed, 111 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index c26a545..2a6c665 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -197,10 +197,54 @@ static long vfio_platform_ioctl(void *device_data,

return copy_to_user((void __user *)arg, &info, minsz);

- } else if (cmd == VFIO_DEVICE_SET_IRQS)
- return -EINVAL;
+ } else if (cmd == VFIO_DEVICE_SET_IRQS) {
+ struct vfio_irq_set hdr;
+ u8 *data = NULL;
+ int ret = 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;

- else if (cmd == VFIO_DEVICE_RESET)
+ 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;
+
+ data = memdup_user((void __user *)(arg + minsz), size);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+ }
+
+ mutex_lock(&vdev->igate);
+
+ ret = vfio_platform_set_irqs_ioctl(vdev, hdr.flags, hdr.index,
+ hdr.start, hdr.count, data);
+ mutex_unlock(&vdev->igate);
+ kfree(data);
+
+ return ret;
+
+ } else if (cmd == VFIO_DEVICE_RESET)
return -EINVAL;

return -ENOTTY;
@@ -442,6 +486,8 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
return ret;
}

+ mutex_init(&vdev->igate);
+
return 0;
}
EXPORT_SYMBOL_GPL(vfio_platform_probe_common);
diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index d99c71c..007b386 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -31,6 +31,53 @@

#include "vfio_platform_private.h"

+static int vfio_platform_set_irq_mask(struct vfio_platform_device *vdev,
+ unsigned index, unsigned start,
+ unsigned count, uint32_t flags, void *data)
+{
+ return -EINVAL;
+}
+
+static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
+ unsigned index, unsigned start,
+ unsigned count, uint32_t flags, void *data)
+{
+ return -EINVAL;
+}
+
+static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev,
+ unsigned index, unsigned start,
+ unsigned count, uint32_t flags, void *data)
+{
+ return -EINVAL;
+}
+
+int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
+ uint32_t flags, unsigned index, unsigned start,
+ unsigned count, void *data)
+{
+ int (*func)(struct vfio_platform_device *vdev, unsigned index,
+ unsigned start, unsigned count, uint32_t flags,
+ void *data) = NULL;
+
+ switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
+ case VFIO_IRQ_SET_ACTION_MASK:
+ func = vfio_platform_set_irq_mask;
+ break;
+ case VFIO_IRQ_SET_ACTION_UNMASK:
+ func = vfio_platform_set_irq_unmask;
+ break;
+ case VFIO_IRQ_SET_ACTION_TRIGGER:
+ func = vfio_platform_set_irq_trigger;
+ break;
+ }
+
+ if (!func)
+ return -ENOTTY;
+
+ return func(vdev, index, start, count, flags, data);
+}
+
int vfio_platform_irq_init(struct vfio_platform_device *vdev)
{
int cnt = 0, i;
@@ -43,13 +90,22 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
return -ENOMEM;

for (i = 0; i < cnt; i++) {
+ int hwirq = vdev->get_irq(vdev, i);
+
+ if (hwirq < 0)
+ goto err;
+
vdev->irqs[i].flags = 0;
vdev->irqs[i].count = 1;
+ vdev->irqs[i].hwirq = hwirq;
}

vdev->num_irqs = cnt;

return 0;
+err:
+ kfree(vdev->irqs);
+ return -EINVAL;
}

void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev)
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 196c530..11ad1bb 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -27,6 +27,7 @@
struct vfio_platform_irq {
u32 flags;
u32 count;
+ int hwirq;
};

struct vfio_platform_region {
@@ -45,6 +46,7 @@ struct vfio_platform_device {
struct vfio_platform_irq *irqs;
u32 num_irqs;
atomic_t refcnt;
+ struct mutex igate;

/*
* These fields should be filled by the bus driver at binding time
@@ -65,4 +67,8 @@ extern int vfio_platform_remove_common(struct device *dev);
extern int vfio_platform_irq_init(struct vfio_platform_device *vdev);
extern void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev);

+extern int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
+ uint32_t flags, unsigned index, unsigned start,
+ unsigned count, void *data);
+
#endif /* VFIO_PLATFORM_PRIVATE_H */
--
2.1.1
Antonios Motakis
2014-10-13 13:10:19 UTC
Permalink
This patch allows to set an eventfd for a patform device's interrupt,
and also to trigger the interrupt eventfd from userspace for testing.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_irq.c | 83 ++++++++++++++++++++++++++-
drivers/vfio/platform/vfio_platform_private.h | 2 +
2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index 007b386..4359b9c 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -45,11 +45,85 @@ static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
return -EINVAL;
}

+static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
+{
+ struct vfio_platform_irq *irq_ctx = dev_id;
+
+ eventfd_signal(irq_ctx->trigger, 1);
+
+ return IRQ_HANDLED;
+}
+
+static int vfio_set_trigger(struct vfio_platform_device *vdev,
+ int index, int fd)
+{
+ struct vfio_platform_irq *irq = &vdev->irqs[index];
+ struct eventfd_ctx *trigger;
+ int ret;
+
+ if (irq->trigger) {
+ free_irq(irq->hwirq, irq);
+ kfree(irq->name);
+ eventfd_ctx_put(irq->trigger);
+ irq->trigger = NULL;
+ }
+
+ if (fd < 0) /* Disable only */
+ return 0;
+
+ irq->name = kasprintf(GFP_KERNEL, "vfio-irq[%d](%s)",
+ irq->hwirq, vdev->name);
+ if (!irq->name)
+ return -ENOMEM;
+
+ trigger = eventfd_ctx_fdget(fd);
+ if (IS_ERR(trigger)) {
+ kfree(irq->name);
+ return PTR_ERR(trigger);
+ }
+
+ irq->trigger = trigger;
+
+ ret = request_irq(irq->hwirq, vfio_irq_handler, 0, irq->name, irq);
+ if (ret) {
+ kfree(irq->name);
+ eventfd_ctx_put(trigger);
+ irq->trigger = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+
static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
{
- return -EINVAL;
+ struct vfio_platform_irq *irq = &vdev->irqs[index];
+
+ if (!count && (flags & VFIO_IRQ_SET_DATA_NONE))
+ return vfio_set_trigger(vdev, index, -1);
+
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
+ int32_t fd = *(int32_t *)data;
+
+ return vfio_set_trigger(vdev, index, fd);
+ }
+
+ if (flags & VFIO_IRQ_SET_DATA_NONE) {
+ vfio_irq_handler(irq->hwirq, irq);
+
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
+ uint8_t trigger = *(uint8_t *)data;
+
+ if (trigger)
+ vfio_irq_handler(irq->hwirq, irq);
+ }
+
+ return 0;
}

int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
@@ -95,7 +169,7 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
if (hwirq < 0)
goto err;

- vdev->irqs[i].flags = 0;
+ vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD;
vdev->irqs[i].count = 1;
vdev->irqs[i].hwirq = hwirq;
}
@@ -110,6 +184,11 @@ err:

void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev)
{
+ int i;
+
+ for (i = 0; i < vdev->num_irqs; i++)
+ vfio_set_trigger(vdev, i, -1);
+
vdev->num_irqs = 0;
kfree(vdev->irqs);
}
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 11ad1bb..47af6e0 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -28,6 +28,8 @@ struct vfio_platform_irq {
u32 flags;
u32 count;
int hwirq;
+ char *name;
+ struct eventfd_ctx *trigger;
};

struct vfio_platform_region {
--
2.1.1
Antonios Motakis
2014-10-13 13:10:20 UTC
Permalink
Adds support to mask interrupts, and also for automasked interrupts.
Level sensitive interrupts are exposed as automasked interrupts and
are masked and disabled automatically when they fire.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_irq.c | 94 +++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 2 +
2 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index 4359b9c..7620a17 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -31,27 +31,103 @@

#include "vfio_platform_private.h"

+static void vfio_platform_mask(struct vfio_platform_irq *irq_ctx)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (!irq_ctx->masked) {
+ disable_irq(irq_ctx->hwirq);
+ irq_ctx->masked = true;
+ }
+
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
+}
+
static int vfio_platform_set_irq_mask(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
{
- return -EINVAL;
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD)
+ return -EINVAL; /* not implemented yet */
+
+ if (flags & VFIO_IRQ_SET_DATA_NONE) {
+ vfio_platform_mask(&vdev->irqs[index]);
+
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
+ uint8_t mask = *(uint8_t *)data;
+
+ if (mask)
+ vfio_platform_mask(&vdev->irqs[index]);
+ }
+
+ return 0;
+}
+
+static void vfio_platform_unmask(struct vfio_platform_irq *irq_ctx)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (irq_ctx->masked) {
+ enable_irq(irq_ctx->hwirq);
+ irq_ctx->masked = false;
+ }
+
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
}

static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
{
- return -EINVAL;
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD)
+ return -EINVAL; /* not implemented yet */
+
+ if (flags & VFIO_IRQ_SET_DATA_NONE) {
+ vfio_platform_unmask(&vdev->irqs[index]);
+
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
+ uint8_t unmask = *(uint8_t *)data;
+
+ if (unmask)
+ vfio_platform_unmask(&vdev->irqs[index]);
+ }
+
+ return 0;
}

static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
{
struct vfio_platform_irq *irq_ctx = dev_id;
+ unsigned long flags;
+ int ret = IRQ_NONE;

- eventfd_signal(irq_ctx->trigger, 1);
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (!irq_ctx->masked) {
+ ret = IRQ_HANDLED;
+
+ if (irq_ctx->flags & VFIO_IRQ_INFO_AUTOMASKED) {
+ disable_irq_nosync(irq_ctx->hwirq);
+ irq_ctx->masked = true;
+ }
+ }

- return IRQ_HANDLED;
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
+
+ if (ret == IRQ_HANDLED)
+ eventfd_signal(irq_ctx->trigger, 1);
+
+ return ret;
}

static int vfio_set_trigger(struct vfio_platform_device *vdev,
@@ -169,9 +245,17 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
if (hwirq < 0)
goto err;

- vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD;
+ spin_lock_init(&vdev->irqs[i].lock);
+
+ vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD
+ | VFIO_IRQ_INFO_MASKABLE;
+
+ if (irq_get_trigger_type(hwirq) & IRQ_TYPE_LEVEL_MASK)
+ vdev->irqs[i].flags |= VFIO_IRQ_INFO_AUTOMASKED;
+
vdev->irqs[i].count = 1;
vdev->irqs[i].hwirq = hwirq;
+ vdev->irqs[i].masked = false;
}

vdev->num_irqs = cnt;
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 47af6e0..65e80e7 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -30,6 +30,8 @@ struct vfio_platform_irq {
int hwirq;
char *name;
struct eventfd_ctx *trigger;
+ bool masked;
+ spinlock_t lock;
};

struct vfio_platform_region {
--
2.1.1
Alex Williamson
2014-10-21 17:47:18 UTC
Permalink
Post by Antonios Motakis
Adds support to mask interrupts, and also for automasked interrupts.
Level sensitive interrupts are exposed as automasked interrupts and
are masked and disabled automatically when they fire.
---
drivers/vfio/platform/vfio_platform_irq.c | 94 +++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 2 +
2 files changed, 91 insertions(+), 5 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index 4359b9c..7620a17 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -31,27 +31,103 @@
#include "vfio_platform_private.h"
+static void vfio_platform_mask(struct vfio_platform_irq *irq_ctx)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (!irq_ctx->masked) {
+ disable_irq(irq_ctx->hwirq);
+ irq_ctx->masked = true;
+ }
+
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
+}
+
static int vfio_platform_set_irq_mask(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
{
- return -EINVAL;
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD)
+ return -EINVAL; /* not implemented yet */
+
+ if (flags & VFIO_IRQ_SET_DATA_NONE) {
+ vfio_platform_mask(&vdev->irqs[index]);
+
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
+ uint8_t mask = *(uint8_t *)data;
+
+ if (mask)
+ vfio_platform_mask(&vdev->irqs[index]);
+ }
+
+ return 0;
+}
+
+static void vfio_platform_unmask(struct vfio_platform_irq *irq_ctx)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (irq_ctx->masked) {
+ enable_irq(irq_ctx->hwirq);
+ irq_ctx->masked = false;
+ }
+
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
}
static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
{
- return -EINVAL;
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD)
+ return -EINVAL; /* not implemented yet */
+
+ if (flags & VFIO_IRQ_SET_DATA_NONE) {
+ vfio_platform_unmask(&vdev->irqs[index]);
+
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
+ uint8_t unmask = *(uint8_t *)data;
+
+ if (unmask)
+ vfio_platform_unmask(&vdev->irqs[index]);
+ }
+
+ return 0;
}
static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
{
struct vfio_platform_irq *irq_ctx = dev_id;
+ unsigned long flags;
+ int ret = IRQ_NONE;
- eventfd_signal(irq_ctx->trigger, 1);
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (!irq_ctx->masked) {
+ ret = IRQ_HANDLED;
+
+ if (irq_ctx->flags & VFIO_IRQ_INFO_AUTOMASKED) {
+ disable_irq_nosync(irq_ctx->hwirq);
+ irq_ctx->masked = true;
+ }
+ }
- return IRQ_HANDLED;
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
+
+ if (ret == IRQ_HANDLED)
+ eventfd_signal(irq_ctx->trigger, 1);
+
+ return ret;
}
If you actually have edge interrupts, you're unnecessarily penalizing
them with the spinlock here. You could do like vfio-pci and only
advertise level interrupts as maskable then use separate edge vs level
handlers.
Post by Antonios Motakis
static int vfio_set_trigger(struct vfio_platform_device *vdev,
@@ -169,9 +245,17 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
if (hwirq < 0)
goto err;
- vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD;
+ spin_lock_init(&vdev->irqs[i].lock);
+
+ vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD
+ | VFIO_IRQ_INFO_MASKABLE;
+
+ if (irq_get_trigger_type(hwirq) & IRQ_TYPE_LEVEL_MASK)
+ vdev->irqs[i].flags |= VFIO_IRQ_INFO_AUTOMASKED;
+
vdev->irqs[i].count = 1;
vdev->irqs[i].hwirq = hwirq;
+ vdev->irqs[i].masked = false;
}
vdev->num_irqs = cnt;
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 47af6e0..65e80e7 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -30,6 +30,8 @@ struct vfio_platform_irq {
int hwirq;
char *name;
struct eventfd_ctx *trigger;
+ bool masked;
+ spinlock_t lock;
};
struct vfio_platform_region {
Antonios Motakis
2014-10-22 13:55:56 UTC
Permalink
On Tue, Oct 21, 2014 at 7:47 PM, Alex Williamson
Post by Alex Williamson
Post by Antonios Motakis
Adds support to mask interrupts, and also for automasked interrupts.
Level sensitive interrupts are exposed as automasked interrupts and
are masked and disabled automatically when they fire.
---
drivers/vfio/platform/vfio_platform_irq.c | 94 +++++++++++++++++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 2 +
2 files changed, 91 insertions(+), 5 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index 4359b9c..7620a17 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -31,27 +31,103 @@
#include "vfio_platform_private.h"
+static void vfio_platform_mask(struct vfio_platform_irq *irq_ctx)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (!irq_ctx->masked) {
+ disable_irq(irq_ctx->hwirq);
+ irq_ctx->masked = true;
+ }
+
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
+}
+
static int vfio_platform_set_irq_mask(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
{
- return -EINVAL;
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD)
+ return -EINVAL; /* not implemented yet */
+
+ if (flags & VFIO_IRQ_SET_DATA_NONE) {
+ vfio_platform_mask(&vdev->irqs[index]);
+
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
+ uint8_t mask = *(uint8_t *)data;
+
+ if (mask)
+ vfio_platform_mask(&vdev->irqs[index]);
+ }
+
+ return 0;
+}
+
+static void vfio_platform_unmask(struct vfio_platform_irq *irq_ctx)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (irq_ctx->masked) {
+ enable_irq(irq_ctx->hwirq);
+ irq_ctx->masked = false;
+ }
+
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
}
static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
{
- return -EINVAL;
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD)
+ return -EINVAL; /* not implemented yet */
+
+ if (flags & VFIO_IRQ_SET_DATA_NONE) {
+ vfio_platform_unmask(&vdev->irqs[index]);
+
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
+ uint8_t unmask = *(uint8_t *)data;
+
+ if (unmask)
+ vfio_platform_unmask(&vdev->irqs[index]);
+ }
+
+ return 0;
}
static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
{
struct vfio_platform_irq *irq_ctx = dev_id;
+ unsigned long flags;
+ int ret = IRQ_NONE;
- eventfd_signal(irq_ctx->trigger, 1);
+ spin_lock_irqsave(&irq_ctx->lock, flags);
+
+ if (!irq_ctx->masked) {
+ ret = IRQ_HANDLED;
+
+ if (irq_ctx->flags & VFIO_IRQ_INFO_AUTOMASKED) {
+ disable_irq_nosync(irq_ctx->hwirq);
+ irq_ctx->masked = true;
+ }
+ }
- return IRQ_HANDLED;
+ spin_unlock_irqrestore(&irq_ctx->lock, flags);
+
+ if (ret == IRQ_HANDLED)
+ eventfd_signal(irq_ctx->trigger, 1);
+
+ return ret;
}
If you actually have edge interrupts, you're unnecessarily penalizing
them with the spinlock here. You could do like vfio-pci and only
advertise level interrupts as maskable then use separate edge vs level
handlers.
Ok, I shall implement separate handlers then.
Post by Alex Williamson
Post by Antonios Motakis
static int vfio_set_trigger(struct vfio_platform_device *vdev,
@@ -169,9 +245,17 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
if (hwirq < 0)
goto err;
- vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD;
+ spin_lock_init(&vdev->irqs[i].lock);
+
+ vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD
+ | VFIO_IRQ_INFO_MASKABLE;
+
+ if (irq_get_trigger_type(hwirq) & IRQ_TYPE_LEVEL_MASK)
+ vdev->irqs[i].flags |= VFIO_IRQ_INFO_AUTOMASKED;
+
vdev->irqs[i].count = 1;
vdev->irqs[i].hwirq = hwirq;
+ vdev->irqs[i].masked = false;
}
vdev->num_irqs = cnt;
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 47af6e0..65e80e7 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -30,6 +30,8 @@ struct vfio_platform_irq {
int hwirq;
char *name;
struct eventfd_ctx *trigger;
+ bool masked;
+ spinlock_t lock;
};
struct vfio_platform_region {
--
Antonios Motakis
Virtual Open Systems
Antonios Motakis
2014-10-13 13:10:21 UTC
Permalink
The virqfd functionality that is used by VFIO_PCI to implement interrupt
masking and unmasking via an eventfd, is generic enough and can be reused
by another driver. Move it to a separate file in order to allow the code
to be shared.

Also properly export virqfd_enable and virqfd_disable in the process.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/Makefile | 4 +-
drivers/vfio/pci/vfio_pci_intrs.c | 213 -----------------------------------
drivers/vfio/pci/vfio_pci_private.h | 3 -
drivers/vfio/virqfd.c | 214 ++++++++++++++++++++++++++++++++++++
include/linux/vfio.h | 28 +++++
5 files changed, 245 insertions(+), 217 deletions(-)
create mode 100644 drivers/vfio/virqfd.c

diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index dadf0ca..d798b09 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,4 +1,6 @@
-obj-$(CONFIG_VFIO) += vfio.o
+vfio_core-y := vfio.o virqfd.o
+
+obj-$(CONFIG_VFIO) += vfio_core.o
obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index 9dd49c9..3f909bb 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -18,226 +18,13 @@
#include <linux/eventfd.h>
#include <linux/pci.h>
#include <linux/file.h>
-#include <linux/poll.h>
#include <linux/vfio.h>
#include <linux/wait.h>
-#include <linux/workqueue.h>
#include <linux/slab.h>

#include "vfio_pci_private.h"

/*
- * IRQfd - generic
- */
-struct virqfd {
- struct vfio_pci_device *vdev;
- struct eventfd_ctx *eventfd;
- int (*handler)(struct vfio_pci_device *, void *);
- void (*thread)(struct vfio_pci_device *, void *);
- void *data;
- struct work_struct inject;
- wait_queue_t wait;
- poll_table pt;
- struct work_struct shutdown;
- struct virqfd **pvirqfd;
-};
-
-static struct workqueue_struct *vfio_irqfd_cleanup_wq;
-
-int __init vfio_pci_virqfd_init(void)
-{
- vfio_irqfd_cleanup_wq =
- create_singlethread_workqueue("vfio-irqfd-cleanup");
- if (!vfio_irqfd_cleanup_wq)
- return -ENOMEM;
-
- return 0;
-}
-
-void vfio_pci_virqfd_exit(void)
-{
- destroy_workqueue(vfio_irqfd_cleanup_wq);
-}
-
-static void virqfd_deactivate(struct virqfd *virqfd)
-{
- queue_work(vfio_irqfd_cleanup_wq, &virqfd->shutdown);
-}
-
-static int virqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
-{
- struct virqfd *virqfd = container_of(wait, struct virqfd, wait);
- unsigned long flags = (unsigned long)key;
-
- if (flags & POLLIN) {
- /* An event has been signaled, call function */
- if ((!virqfd->handler ||
- virqfd->handler(virqfd->vdev, virqfd->data)) &&
- virqfd->thread)
- schedule_work(&virqfd->inject);
- }
-
- if (flags & POLLHUP) {
- unsigned long flags;
- spin_lock_irqsave(&virqfd->vdev->irqlock, flags);
-
- /*
- * The eventfd is closing, if the virqfd has not yet been
- * queued for release, as determined by testing whether the
- * vdev pointer to it is still valid, queue it now. As
- * with kvm irqfds, we know we won't race against the virqfd
- * going away because we hold wqh->lock to get here.
- */
- if (*(virqfd->pvirqfd) == virqfd) {
- *(virqfd->pvirqfd) = NULL;
- virqfd_deactivate(virqfd);
- }
-
- spin_unlock_irqrestore(&virqfd->vdev->irqlock, flags);
- }
-
- return 0;
-}
-
-static void virqfd_ptable_queue_proc(struct file *file,
- wait_queue_head_t *wqh, poll_table *pt)
-{
- struct virqfd *virqfd = container_of(pt, struct virqfd, pt);
- add_wait_queue(wqh, &virqfd->wait);
-}
-
-static void virqfd_shutdown(struct work_struct *work)
-{
- struct virqfd *virqfd = container_of(work, struct virqfd, shutdown);
- u64 cnt;
-
- eventfd_ctx_remove_wait_queue(virqfd->eventfd, &virqfd->wait, &cnt);
- flush_work(&virqfd->inject);
- eventfd_ctx_put(virqfd->eventfd);
-
- kfree(virqfd);
-}
-
-static void virqfd_inject(struct work_struct *work)
-{
- struct virqfd *virqfd = container_of(work, struct virqfd, inject);
- if (virqfd->thread)
- virqfd->thread(virqfd->vdev, virqfd->data);
-}
-
-static int virqfd_enable(struct vfio_pci_device *vdev,
- int (*handler)(struct vfio_pci_device *, void *),
- void (*thread)(struct vfio_pci_device *, void *),
- void *data, struct virqfd **pvirqfd, int fd)
-{
- struct fd irqfd;
- struct eventfd_ctx *ctx;
- struct virqfd *virqfd;
- int ret = 0;
- unsigned int events;
-
- virqfd = kzalloc(sizeof(*virqfd), GFP_KERNEL);
- if (!virqfd)
- return -ENOMEM;
-
- virqfd->pvirqfd = pvirqfd;
- virqfd->vdev = vdev;
- virqfd->handler = handler;
- virqfd->thread = thread;
- virqfd->data = data;
-
- INIT_WORK(&virqfd->shutdown, virqfd_shutdown);
- INIT_WORK(&virqfd->inject, virqfd_inject);
-
- irqfd = fdget(fd);
- if (!irqfd.file) {
- ret = -EBADF;
- goto err_fd;
- }
-
- ctx = eventfd_ctx_fileget(irqfd.file);
- if (IS_ERR(ctx)) {
- ret = PTR_ERR(ctx);
- goto err_ctx;
- }
-
- virqfd->eventfd = ctx;
-
- /*
- * virqfds can be released by closing the eventfd or directly
- * through ioctl. These are both done through a workqueue, so
- * we update the pointer to the virqfd under lock to avoid
- * pushing multiple jobs to release the same virqfd.
- */
- spin_lock_irq(&vdev->irqlock);
-
- if (*pvirqfd) {
- spin_unlock_irq(&vdev->irqlock);
- ret = -EBUSY;
- goto err_busy;
- }
- *pvirqfd = virqfd;
-
- spin_unlock_irq(&vdev->irqlock);
-
- /*
- * Install our own custom wake-up handling so we are notified via
- * a callback whenever someone signals the underlying eventfd.
- */
- init_waitqueue_func_entry(&virqfd->wait, virqfd_wakeup);
- init_poll_funcptr(&virqfd->pt, virqfd_ptable_queue_proc);
-
- events = irqfd.file->f_op->poll(irqfd.file, &virqfd->pt);
-
- /*
- * Check if there was an event already pending on the eventfd
- * before we registered and trigger it as if we didn't miss it.
- */
- if (events & POLLIN) {
- if ((!handler || handler(vdev, data)) && thread)
- schedule_work(&virqfd->inject);
- }
-
- /*
- * Do not drop the file until the irqfd is fully initialized,
- * otherwise we might race against the POLLHUP.
- */
- fdput(irqfd);
-
- return 0;
-err_busy:
- eventfd_ctx_put(ctx);
-err_ctx:
- fdput(irqfd);
-err_fd:
- kfree(virqfd);
-
- return ret;
-}
-
-static void virqfd_disable(struct vfio_pci_device *vdev,
- struct virqfd **pvirqfd)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&vdev->irqlock, flags);
-
- if (*pvirqfd) {
- virqfd_deactivate(*pvirqfd);
- *pvirqfd = NULL;
- }
-
- spin_unlock_irqrestore(&vdev->irqlock, flags);
-
- /*
- * Block until we know all outstanding shutdown jobs have completed.
- * Even if we don't queue the job, flush the wq to be sure it's
- * been released.
- */
- flush_workqueue(vfio_irqfd_cleanup_wq);
-}
-
-/*
* INTx
*/
static void vfio_send_intx_eventfd(struct vfio_pci_device *vdev, void *unused)
diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
index 671c17a..4c89e0e 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -86,9 +86,6 @@ extern ssize_t vfio_pci_vga_rw(struct vfio_pci_device *vdev, char __user *buf,
extern int vfio_pci_init_perm_bits(void);
extern void vfio_pci_uninit_perm_bits(void);

-extern int vfio_pci_virqfd_init(void);
-extern void vfio_pci_virqfd_exit(void);
-
extern int vfio_config_init(struct vfio_pci_device *vdev);
extern void vfio_config_free(struct vfio_pci_device *vdev);
#endif /* VFIO_PCI_PRIVATE_H */
diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
new file mode 100644
index 0000000..243eb61
--- /dev/null
+++ b/drivers/vfio/virqfd.c
@@ -0,0 +1,214 @@
+/*
+ * VFIO generic eventfd code for IRQFD support.
+ * Derived from drivers/vfio/pci/vfio_pci_intrs.c
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All rights reserved.
+ * Author: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+***@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.
+ */
+
+#include <linux/vfio.h>
+#include <linux/eventfd.h>
+#include <linux/file.h>
+#include <linux/slab.h>
+#include "pci/vfio_pci_private.h"
+
+static struct workqueue_struct *vfio_irqfd_cleanup_wq;
+
+int __init vfio_pci_virqfd_init(void)
+{
+ vfio_irqfd_cleanup_wq =
+ create_singlethread_workqueue("vfio-irqfd-cleanup");
+ if (!vfio_irqfd_cleanup_wq)
+ return -ENOMEM;
+
+ return 0;
+}
+
+void vfio_pci_virqfd_exit(void)
+{
+ destroy_workqueue(vfio_irqfd_cleanup_wq);
+}
+
+static void virqfd_deactivate(struct virqfd *virqfd)
+{
+ queue_work(vfio_irqfd_cleanup_wq, &virqfd->shutdown);
+}
+
+static int virqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
+{
+ struct virqfd *virqfd = container_of(wait, struct virqfd, wait);
+ unsigned long flags = (unsigned long)key;
+
+ if (flags & POLLIN) {
+ /* An event has been signaled, call function */
+ if ((!virqfd->handler ||
+ virqfd->handler(virqfd->vdev, virqfd->data)) &&
+ virqfd->thread)
+ schedule_work(&virqfd->inject);
+ }
+
+ if (flags & POLLHUP) {
+ unsigned long flags;
+ spin_lock_irqsave(&virqfd->vdev->irqlock, flags);
+
+ /*
+ * The eventfd is closing, if the virqfd has not yet been
+ * queued for release, as determined by testing whether the
+ * vdev pointer to it is still valid, queue it now. As
+ * with kvm irqfds, we know we won't race against the virqfd
+ * going away because we hold wqh->lock to get here.
+ */
+ if (*(virqfd->pvirqfd) == virqfd) {
+ *(virqfd->pvirqfd) = NULL;
+ virqfd_deactivate(virqfd);
+ }
+
+ spin_unlock_irqrestore(&virqfd->vdev->irqlock, flags);
+ }
+
+ return 0;
+}
+
+static void virqfd_ptable_queue_proc(struct file *file,
+ wait_queue_head_t *wqh, poll_table *pt)
+{
+ struct virqfd *virqfd = container_of(pt, struct virqfd, pt);
+ add_wait_queue(wqh, &virqfd->wait);
+}
+
+static void virqfd_shutdown(struct work_struct *work)
+{
+ struct virqfd *virqfd = container_of(work, struct virqfd, shutdown);
+ u64 cnt;
+
+ eventfd_ctx_remove_wait_queue(virqfd->eventfd, &virqfd->wait, &cnt);
+ flush_work(&virqfd->inject);
+ eventfd_ctx_put(virqfd->eventfd);
+
+ kfree(virqfd);
+}
+
+static void virqfd_inject(struct work_struct *work)
+{
+ struct virqfd *virqfd = container_of(work, struct virqfd, inject);
+ if (virqfd->thread)
+ virqfd->thread(virqfd->vdev, virqfd->data);
+}
+
+int virqfd_enable(struct vfio_pci_device *vdev,
+ int (*handler)(struct vfio_pci_device *, void *),
+ void (*thread)(struct vfio_pci_device *, void *),
+ void *data, struct virqfd **pvirqfd, int fd)
+{
+ struct fd irqfd;
+ struct eventfd_ctx *ctx;
+ struct virqfd *virqfd;
+ int ret = 0;
+ unsigned int events;
+
+ virqfd = kzalloc(sizeof(*virqfd), GFP_KERNEL);
+ if (!virqfd)
+ return -ENOMEM;
+
+ virqfd->pvirqfd = pvirqfd;
+ virqfd->vdev = vdev;
+ virqfd->handler = handler;
+ virqfd->thread = thread;
+ virqfd->data = data;
+
+ INIT_WORK(&virqfd->shutdown, virqfd_shutdown);
+ INIT_WORK(&virqfd->inject, virqfd_inject);
+
+ irqfd = fdget(fd);
+ if (!irqfd.file) {
+ ret = -EBADF;
+ goto err_fd;
+ }
+
+ ctx = eventfd_ctx_fileget(irqfd.file);
+ if (IS_ERR(ctx)) {
+ ret = PTR_ERR(ctx);
+ goto err_ctx;
+ }
+
+ virqfd->eventfd = ctx;
+
+ /*
+ * virqfds can be released by closing the eventfd or directly
+ * through ioctl. These are both done through a workqueue, so
+ * we update the pointer to the virqfd under lock to avoid
+ * pushing multiple jobs to release the same virqfd.
+ */
+ spin_lock_irq(&vdev->irqlock);
+
+ if (*pvirqfd) {
+ spin_unlock_irq(&vdev->irqlock);
+ ret = -EBUSY;
+ goto err_busy;
+ }
+ *pvirqfd = virqfd;
+
+ spin_unlock_irq(&vdev->irqlock);
+
+ /*
+ * Install our own custom wake-up handling so we are notified via
+ * a callback whenever someone signals the underlying eventfd.
+ */
+ init_waitqueue_func_entry(&virqfd->wait, virqfd_wakeup);
+ init_poll_funcptr(&virqfd->pt, virqfd_ptable_queue_proc);
+
+ events = irqfd.file->f_op->poll(irqfd.file, &virqfd->pt);
+
+ /*
+ * Check if there was an event already pending on the eventfd
+ * before we registered and trigger it as if we didn't miss it.
+ */
+ if (events & POLLIN) {
+ if ((!handler || handler(vdev, data)) && thread)
+ schedule_work(&virqfd->inject);
+ }
+
+ /*
+ * Do not drop the file until the irqfd is fully initialized,
+ * otherwise we might race against the POLLHUP.
+ */
+ fdput(irqfd);
+
+ return 0;
+err_busy:
+ eventfd_ctx_put(ctx);
+err_ctx:
+ fdput(irqfd);
+err_fd:
+ kfree(virqfd);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(virqfd_enable);
+
+void virqfd_disable(struct vfio_pci_device *vdev,
+ struct virqfd **pvirqfd)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&vdev->irqlock, flags);
+
+ if (*pvirqfd) {
+ virqfd_deactivate(*pvirqfd);
+ *pvirqfd = NULL;
+ }
+
+ spin_unlock_irqrestore(&vdev->irqlock, flags);
+
+ /*
+ * Block until we know all outstanding shutdown jobs have completed.
+ * Even if we don't queue the job, flush the wq to be sure it's
+ * been released.
+ */
+ flush_workqueue(vfio_irqfd_cleanup_wq);
+}
+EXPORT_SYMBOL_GPL(virqfd_disable);
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index d320411..a077c48 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -14,6 +14,8 @@

#include <linux/iommu.h>
#include <linux/mm.h>
+#include <linux/workqueue.h>
+#include <linux/poll.h>
#include <uapi/linux/vfio.h>

/**
@@ -121,4 +123,30 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
return -ENOTTY;
}
#endif /* CONFIG_EEH */
+
+/*
+ * IRQFD support
+ */
+struct virqfd {
+ struct vfio_pci_device *vdev;
+ struct eventfd_ctx *eventfd;
+ int (*handler)(struct vfio_pci_device *, void *);
+ void (*thread)(struct vfio_pci_device *, void *);
+ void *data;
+ struct work_struct inject;
+ wait_queue_t wait;
+ poll_table pt;
+ struct work_struct shutdown;
+ struct virqfd **pvirqfd;
+};
+
+extern int vfio_pci_virqfd_init(void);
+extern void vfio_pci_virqfd_exit(void);
+extern int virqfd_enable(struct vfio_pci_device *vdev,
+ int (*handler)(struct vfio_pci_device *, void *),
+ void (*thread)(struct vfio_pci_device *, void *),
+ void *data, struct virqfd **pvirqfd, int fd);
+extern void virqfd_disable(struct vfio_pci_device *vdev,
+ struct virqfd **pvirqfd);
+
#endif /* VFIO_H */
--
2.1.1
Alex Williamson
2014-10-21 17:55:26 UTC
Permalink
Post by Antonios Motakis
The virqfd functionality that is used by VFIO_PCI to implement interrupt
masking and unmasking via an eventfd, is generic enough and can be reused
by another driver. Move it to a separate file in order to allow the code
to be shared.
Also properly export virqfd_enable and virqfd_disable in the process.
Let's be friendly to the global namespace and add vfio_ prefixes to
these as we export them.
Antonios Motakis
2014-10-22 13:56:31 UTC
Permalink
On Tue, Oct 21, 2014 at 7:55 PM, Alex Williamson
Post by Alex Williamson
Post by Antonios Motakis
The virqfd functionality that is used by VFIO_PCI to implement interrupt
masking and unmasking via an eventfd, is generic enough and can be reused
by another driver. Move it to a separate file in order to allow the code
to be shared.
Also properly export virqfd_enable and virqfd_disable in the process.
Let's be friendly to the global namespace and add vfio_ prefixes to
these as we export them.
Ack.
--
Antonios Motakis
Virtual Open Systems
Antonios Motakis
2014-10-13 13:10:22 UTC
Permalink
Virqfd just needs to keep accesses to any struct *virqfd safe, but this
comes into play only when creating or destroying eventfds, so sharing
the same spinlock with the VFIO bus driver is not necessary.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/pci/vfio_pci_intrs.c | 10 +++++-----
drivers/vfio/virqfd.c | 24 +++++++++++++-----------
include/linux/vfio.h | 3 +--
3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index 3f909bb..e56c814 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -226,8 +226,8 @@ static int vfio_intx_set_signal(struct vfio_pci_device *vdev, int fd)
static void vfio_intx_disable(struct vfio_pci_device *vdev)
{
vfio_intx_set_signal(vdev, -1);
- virqfd_disable(vdev, &vdev->ctx[0].unmask);
- virqfd_disable(vdev, &vdev->ctx[0].mask);
+ virqfd_disable(&vdev->ctx[0].unmask);
+ virqfd_disable(&vdev->ctx[0].mask);
vdev->irq_type = VFIO_PCI_NUM_IRQS;
vdev->num_ctx = 0;
kfree(vdev->ctx);
@@ -377,8 +377,8 @@ static void vfio_msi_disable(struct vfio_pci_device *vdev, bool msix)
vfio_msi_set_block(vdev, 0, vdev->num_ctx, NULL, msix);

for (i = 0; i < vdev->num_ctx; i++) {
- virqfd_disable(vdev, &vdev->ctx[i].unmask);
- virqfd_disable(vdev, &vdev->ctx[i].mask);
+ virqfd_disable(&vdev->ctx[i].unmask);
+ virqfd_disable(&vdev->ctx[i].mask);
}

if (msix) {
@@ -415,7 +415,7 @@ static int vfio_pci_set_intx_unmask(struct vfio_pci_device *vdev,
vfio_send_intx_eventfd, NULL,
&vdev->ctx[0].unmask, fd);

- virqfd_disable(vdev, &vdev->ctx[0].unmask);
+ virqfd_disable(&vdev->ctx[0].unmask);
}

return 0;
diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
index 243eb61..27fa2f0 100644
--- a/drivers/vfio/virqfd.c
+++ b/drivers/vfio/virqfd.c
@@ -17,6 +17,7 @@
#include "pci/vfio_pci_private.h"

static struct workqueue_struct *vfio_irqfd_cleanup_wq;
+static spinlock_t lock;

int __init vfio_pci_virqfd_init(void)
{
@@ -25,6 +26,8 @@ int __init vfio_pci_virqfd_init(void)
if (!vfio_irqfd_cleanup_wq)
return -ENOMEM;

+ spin_lock_init(&lock);
+
return 0;
}

@@ -53,21 +56,21 @@ static int virqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)

if (flags & POLLHUP) {
unsigned long flags;
- spin_lock_irqsave(&virqfd->vdev->irqlock, flags);
+ spin_lock_irqsave(&lock, flags);

/*
* The eventfd is closing, if the virqfd has not yet been
* queued for release, as determined by testing whether the
- * vdev pointer to it is still valid, queue it now. As
+ * virqfd pointer to it is still valid, queue it now. As
* with kvm irqfds, we know we won't race against the virqfd
- * going away because we hold wqh->lock to get here.
+ * going away because we hold the lock to get here.
*/
if (*(virqfd->pvirqfd) == virqfd) {
*(virqfd->pvirqfd) = NULL;
virqfd_deactivate(virqfd);
}

- spin_unlock_irqrestore(&virqfd->vdev->irqlock, flags);
+ spin_unlock_irqrestore(&lock, flags);
}

return 0;
@@ -143,16 +146,16 @@ int virqfd_enable(struct vfio_pci_device *vdev,
* we update the pointer to the virqfd under lock to avoid
* pushing multiple jobs to release the same virqfd.
*/
- spin_lock_irq(&vdev->irqlock);
+ spin_lock_irq(&lock);

if (*pvirqfd) {
- spin_unlock_irq(&vdev->irqlock);
+ spin_unlock_irq(&lock);
ret = -EBUSY;
goto err_busy;
}
*pvirqfd = virqfd;

- spin_unlock_irq(&vdev->irqlock);
+ spin_unlock_irq(&lock);

/*
* Install our own custom wake-up handling so we are notified via
@@ -190,19 +193,18 @@ err_fd:
}
EXPORT_SYMBOL_GPL(virqfd_enable);

-void virqfd_disable(struct vfio_pci_device *vdev,
- struct virqfd **pvirqfd)
+void virqfd_disable(struct virqfd **pvirqfd)
{
unsigned long flags;

- spin_lock_irqsave(&vdev->irqlock, flags);
+ spin_lock_irqsave(&lock, flags);

if (*pvirqfd) {
virqfd_deactivate(*pvirqfd);
*pvirqfd = NULL;
}

- spin_unlock_irqrestore(&vdev->irqlock, flags);
+ spin_unlock_irqrestore(&lock, flags);

/*
* Block until we know all outstanding shutdown jobs have completed.
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index a077c48..fb6037b 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -146,7 +146,6 @@ extern int virqfd_enable(struct vfio_pci_device *vdev,
int (*handler)(struct vfio_pci_device *, void *),
void (*thread)(struct vfio_pci_device *, void *),
void *data, struct virqfd **pvirqfd, int fd);
-extern void virqfd_disable(struct vfio_pci_device *vdev,
- struct virqfd **pvirqfd);
+extern void virqfd_disable(struct virqfd **pvirqfd);

#endif /* VFIO_H */
--
2.1.1
Antonios Motakis
2014-10-13 13:10:23 UTC
Permalink
VFIO_PCI passes the VFIO device structure *vdev via eventfd to the handler
that implements masking/unmasking of IRQs via an eventfd. We can replace
it in the virqfd infrastructure with an opaque type so we can make use
of the mechanism from other VFIO bus drivers.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/pci/vfio_pci_intrs.c | 11 +++++++----
drivers/vfio/virqfd.c | 17 ++++++++---------
include/linux/vfio.h | 12 ++++++------
3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index e56c814..6ca22a8 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -27,8 +27,10 @@
/*
* INTx
*/
-static void vfio_send_intx_eventfd(struct vfio_pci_device *vdev, void *unused)
+static void vfio_send_intx_eventfd(void *opaque, void *unused)
{
+ struct vfio_pci_device *vdev = opaque;
+
if (likely(is_intx(vdev) && !vdev->virq_disabled))
eventfd_signal(vdev->ctx[0].trigger, 1);
}
@@ -71,9 +73,9 @@ void vfio_pci_intx_mask(struct vfio_pci_device *vdev)
* a signal is necessary, which can then be handled via a work queue
* or directly depending on the caller.
*/
-static int vfio_pci_intx_unmask_handler(struct vfio_pci_device *vdev,
- void *unused)
+static int vfio_pci_intx_unmask_handler(void *opaque, void *unused)
{
+ struct vfio_pci_device *vdev = opaque;
struct pci_dev *pdev = vdev->pdev;
unsigned long flags;
int ret = 0;
@@ -411,7 +413,8 @@ static int vfio_pci_set_intx_unmask(struct vfio_pci_device *vdev,
} else if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
int32_t fd = *(int32_t *)data;
if (fd >= 0)
- return virqfd_enable(vdev, vfio_pci_intx_unmask_handler,
+ return virqfd_enable((void *) vdev,
+ vfio_pci_intx_unmask_handler,
vfio_send_intx_eventfd, NULL,
&vdev->ctx[0].unmask, fd);

diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
index 27fa2f0..ac63ec0 100644
--- a/drivers/vfio/virqfd.c
+++ b/drivers/vfio/virqfd.c
@@ -14,7 +14,6 @@
#include <linux/eventfd.h>
#include <linux/file.h>
#include <linux/slab.h>
-#include "pci/vfio_pci_private.h"

static struct workqueue_struct *vfio_irqfd_cleanup_wq;
static spinlock_t lock;
@@ -49,7 +48,7 @@ static int virqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
if (flags & POLLIN) {
/* An event has been signaled, call function */
if ((!virqfd->handler ||
- virqfd->handler(virqfd->vdev, virqfd->data)) &&
+ virqfd->handler(virqfd->opaque, virqfd->data)) &&
virqfd->thread)
schedule_work(&virqfd->inject);
}
@@ -99,13 +98,13 @@ static void virqfd_inject(struct work_struct *work)
{
struct virqfd *virqfd = container_of(work, struct virqfd, inject);
if (virqfd->thread)
- virqfd->thread(virqfd->vdev, virqfd->data);
+ virqfd->thread(virqfd->opaque, virqfd->data);
}

-int virqfd_enable(struct vfio_pci_device *vdev,
- int (*handler)(struct vfio_pci_device *, void *),
- void (*thread)(struct vfio_pci_device *, void *),
- void *data, struct virqfd **pvirqfd, int fd)
+int virqfd_enable(void *opaque,
+ int (*handler)(void *, void *),
+ void (*thread)(void *, void *),
+ void *data, struct virqfd **pvirqfd, int fd)
{
struct fd irqfd;
struct eventfd_ctx *ctx;
@@ -118,7 +117,7 @@ int virqfd_enable(struct vfio_pci_device *vdev,
return -ENOMEM;

virqfd->pvirqfd = pvirqfd;
- virqfd->vdev = vdev;
+ virqfd->opaque = opaque;
virqfd->handler = handler;
virqfd->thread = thread;
virqfd->data = data;
@@ -171,7 +170,7 @@ int virqfd_enable(struct vfio_pci_device *vdev,
* before we registered and trigger it as if we didn't miss it.
*/
if (events & POLLIN) {
- if ((!handler || handler(vdev, data)) && thread)
+ if ((!handler || handler(opaque, data)) && thread)
schedule_work(&virqfd->inject);
}

diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index fb6037b..ce23a42 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -128,10 +128,10 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
* IRQFD support
*/
struct virqfd {
- struct vfio_pci_device *vdev;
+ void *opaque;
struct eventfd_ctx *eventfd;
- int (*handler)(struct vfio_pci_device *, void *);
- void (*thread)(struct vfio_pci_device *, void *);
+ int (*handler)(void *, void *);
+ void (*thread)(void *, void *);
void *data;
struct work_struct inject;
wait_queue_t wait;
@@ -142,9 +142,9 @@ struct virqfd {

extern int vfio_pci_virqfd_init(void);
extern void vfio_pci_virqfd_exit(void);
-extern int virqfd_enable(struct vfio_pci_device *vdev,
- int (*handler)(struct vfio_pci_device *, void *),
- void (*thread)(struct vfio_pci_device *, void *),
+extern int virqfd_enable(void *opaque,
+ int (*handler)(void *, void *),
+ void (*thread)(void *, void *),
void *data, struct virqfd **pvirqfd, int fd);
extern void virqfd_disable(struct virqfd **pvirqfd);
--
2.1.1
Antonios Motakis
2014-10-13 13:10:24 UTC
Permalink
Now we have finally completely decoupled virqfd from VFIO_PCI. We can
initialize it from the VFIO generic code, in order to safely use it from
multiple independent VFIO bus drivers.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/pci/vfio_pci.c | 8 --------
drivers/vfio/vfio.c | 8 ++++++++
drivers/vfio/virqfd.c | 4 ++--
include/linux/vfio.h | 4 ++--
4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index f782533..40e176d 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1034,7 +1034,6 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev)
static void __exit vfio_pci_cleanup(void)
{
pci_unregister_driver(&vfio_pci_driver);
- vfio_pci_virqfd_exit();
vfio_pci_uninit_perm_bits();
}

@@ -1047,11 +1046,6 @@ static int __init vfio_pci_init(void)
if (ret)
return ret;

- /* Start the virqfd cleanup handler */
- ret = vfio_pci_virqfd_init();
- if (ret)
- goto out_virqfd;
-
/* Register and scan for devices */
ret = pci_register_driver(&vfio_pci_driver);
if (ret)
@@ -1060,8 +1054,6 @@ static int __init vfio_pci_init(void)
return 0;

out_driver:
- vfio_pci_virqfd_exit();
-out_virqfd:
vfio_pci_uninit_perm_bits();
return ret;
}
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index f018d8d..8e84471 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1464,6 +1464,11 @@ static int __init vfio_init(void)
if (ret)
goto err_cdev_add;

+ /* Start the virqfd cleanup handler used by some VFIO bus drivers */
+ ret = vfio_virqfd_init();
+ if (ret)
+ goto err_virqfd;
+
pr_info(DRIVER_DESC " version: " DRIVER_VERSION "\n");

/*
@@ -1476,6 +1481,8 @@ static int __init vfio_init(void)

return 0;

+err_virqfd:
+ cdev_del(&vfio.group_cdev);
err_cdev_add:
unregister_chrdev_region(vfio.group_devt, MINORMASK);
err_alloc_chrdev:
@@ -1490,6 +1497,7 @@ static void __exit vfio_cleanup(void)
{
WARN_ON(!list_empty(&vfio.group_list));

+ vfio_virqfd_exit();
idr_destroy(&vfio.group_idr);
cdev_del(&vfio.group_cdev);
unregister_chrdev_region(vfio.group_devt, MINORMASK);
diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
index ac63ec0..c28882f 100644
--- a/drivers/vfio/virqfd.c
+++ b/drivers/vfio/virqfd.c
@@ -18,7 +18,7 @@
static struct workqueue_struct *vfio_irqfd_cleanup_wq;
static spinlock_t lock;

-int __init vfio_pci_virqfd_init(void)
+int __init vfio_virqfd_init(void)
{
vfio_irqfd_cleanup_wq =
create_singlethread_workqueue("vfio-irqfd-cleanup");
@@ -30,7 +30,7 @@ int __init vfio_pci_virqfd_init(void)
return 0;
}

-void vfio_pci_virqfd_exit(void)
+void vfio_virqfd_exit(void)
{
destroy_workqueue(vfio_irqfd_cleanup_wq);
}
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index ce23a42..9fa02c8 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -140,8 +140,8 @@ struct virqfd {
struct virqfd **pvirqfd;
};

-extern int vfio_pci_virqfd_init(void);
-extern void vfio_pci_virqfd_exit(void);
+extern int vfio_virqfd_init(void);
+extern void vfio_virqfd_exit(void);
extern int virqfd_enable(void *opaque,
int (*handler)(void *, void *),
void (*thread)(void *, void *),
--
2.1.1
Antonios Motakis
2014-10-13 13:10:25 UTC
Permalink
With this patch the VFIO user will be able to set an eventfd that can be
used in order to mask and unmask IRQs of platform devices.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/***@public.gmane.org>
---
drivers/vfio/platform/vfio_platform_irq.c | 46 ++++++++++++++++++++++++---
drivers/vfio/platform/vfio_platform_private.h | 2 ++
2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index 7620a17..6abb387 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -45,6 +45,15 @@ static void vfio_platform_mask(struct vfio_platform_irq *irq_ctx)
spin_unlock_irqrestore(&irq_ctx->lock, flags);
}

+static int vfio_platform_mask_handler(void *opaque, void *unused)
+{
+ struct vfio_platform_irq *irq_ctx = opaque;
+
+ vfio_platform_mask(irq_ctx);
+
+ return 0;
+}
+
static int vfio_platform_set_irq_mask(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
@@ -52,8 +61,18 @@ static int vfio_platform_set_irq_mask(struct vfio_platform_device *vdev,
if (start != 0 || count != 1)
return -EINVAL;

- if (flags & VFIO_IRQ_SET_DATA_EVENTFD)
- return -EINVAL; /* not implemented yet */
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
+ int32_t fd = *(int32_t *)data;
+
+ if (fd >= 0)
+ return virqfd_enable((void *) &vdev->irqs[index],
+ vfio_platform_mask_handler,
+ NULL, NULL,
+ &vdev->irqs[index].mask, fd);
+
+ virqfd_disable(&vdev->irqs[index].mask);
+ return 0;
+ }

if (flags & VFIO_IRQ_SET_DATA_NONE) {
vfio_platform_mask(&vdev->irqs[index]);
@@ -82,6 +101,15 @@ static void vfio_platform_unmask(struct vfio_platform_irq *irq_ctx)
spin_unlock_irqrestore(&irq_ctx->lock, flags);
}

+static int vfio_platform_unmask_handler(void *opaque, void *unused)
+{
+ struct vfio_platform_irq *irq_ctx = opaque;
+
+ vfio_platform_unmask(irq_ctx);
+
+ return 0;
+}
+
static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data)
@@ -89,8 +117,18 @@ static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
if (start != 0 || count != 1)
return -EINVAL;

- if (flags & VFIO_IRQ_SET_DATA_EVENTFD)
- return -EINVAL; /* not implemented yet */
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
+ int32_t fd = *(int32_t *)data;
+
+ if (fd >= 0)
+ return virqfd_enable((void *) &vdev->irqs[index],
+ vfio_platform_unmask_handler,
+ NULL, NULL,
+ &vdev->irqs[index].unmask, fd);
+
+ virqfd_disable(&vdev->irqs[index].unmask);
+ return 0;
+ }

if (flags & VFIO_IRQ_SET_DATA_NONE) {
vfio_platform_unmask(&vdev->irqs[index]);
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 65e80e7..23ef038 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -32,6 +32,8 @@ struct vfio_platform_irq {
struct eventfd_ctx *trigger;
bool masked;
spinlock_t lock;
+ struct virqfd *unmask;
+ struct virqfd *mask;
};

struct vfio_platform_region {
--
2.1.1
Continue reading on narkive:
Loading...