[PATCH 0/2] vfio: convert to misc driver

[PATCH 0/2] vfio: convert to misc driver

[Alex Williamson]

The control device is pretty easily converted to a misc driver with
static minor number which allows us to support auto load.  I've
submitted a request to lanana for the minor number reservation, so
this is mostly FYI until that's approved.  Comments and reviews
welcome.  Thanks,

Alex

---

Alex Williamson (2):
      misc: Reserve minor for VFIO
      vfio: Convert control interface to misc driver


 Documentation/devices.txt  |    1 +
 drivers/vfio/vfio.c        |   70 +++++++++++++++++++++-----------------------
 include/linux/miscdevice.h |    1 +
 3 files changed, 35 insertions(+), 37 deletions(-)

[PATCH 1/2] misc: Reserve minor for VFIO

[Alex Williamson]

VFIO currently allocates it's own dynamic chardev range, reserving the
first minor for the control part of the interface (/dev/vfio/vfio) and
the remainder for VFIO groups (/dev/vfio/$GROUP).  This works, but it
doesn't support auto loading.  For instance when libvirt checks for
VFIO support it looks for /dev/vfio/vfio, which currently doesn't
exist unless the vfio module is loaded.  By converting the control
device to a misc driver and reserving a static minor, we can enable
auto loading.

Reserving the minor is a prerequist to that conversion.  Minor 196
appears to be unused by anything currently in the kernel.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 Documentation/devices.txt  |    1 +
 include/linux/miscdevice.h |    1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devices.txt b/Documentation/devices.txt
index 80b7241..10378cc 100644
--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -409,6 +409,7 @@ Your cooperation is appreciated.
 		193 = /dev/d7s		SPARC 7-segment display
 		194 = /dev/zkshim	Zero-Knowledge network shim control
 		195 = /dev/elographics/e2201	Elographics touchscreen E271-2201
+		196 = /dev/vfio/vfio	VFIO userspace driver interface
 		198 = /dev/sexec	Signed executable interface
 		199 = /dev/scanners/cuecat :CueCat barcode scanner
 		200 = /dev/net/tun	TAP/TUN network device
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index f7eaf2d..3737f72 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -30,6 +30,7 @@
 #define STORE_QUEUE_MINOR	155
 #define I2O_MINOR		166
 #define MICROCODE_MINOR		184
+#define VFIO_MINOR		196
 #define TUN_MINOR		200
 #define CUSE_MINOR		203
 #define MWAVE_MINOR		219	/* ACP/Mwave Modem */

[PATCH 2/2] vfio: Convert control interface to misc driver

[Alex Williamson]

This change allows us to support module auto loading using devname
support in userspace tools.  With this, /dev/vfio/vfio will always
be present and opening it will cause the vfio module to load.  This
should avoid needing to configure the system to statically load
vfio in order to get libvirt to correctly detect support for it.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio.c |   70 ++++++++++++++++++++++++---------------------------
 1 file changed, 33 insertions(+), 37 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 1eab4ac..21271d8 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -22,6 +22,7 @@
 #include <linux/idr.h>
 #include <linux/iommu.h>
 #include <linux/list.h>
+#include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/rwsem.h>
@@ -45,9 +46,7 @@ static struct vfio {
 	struct idr			group_idr;
 	struct mutex			group_lock;
 	struct cdev			group_cdev;
-	struct device			*dev;
-	dev_t				devt;
-	struct cdev			cdev;
+	dev_t				group_devt;
 	wait_queue_head_t		release_q;
 } vfio;
 
@@ -142,8 +141,7 @@ EXPORT_SYMBOL_GPL(vfio_unregister_iommu_driver);
  */
 static int vfio_alloc_group_minor(struct vfio_group *group)
 {
-	/* index 0 is used by /dev/vfio/vfio */
-	return idr_alloc(&vfio.group_idr, group, 1, MINORMASK + 1, GFP_KERNEL);
+	return idr_alloc(&vfio.group_idr, group, 0, MINORMASK + 1, GFP_KERNEL);
 }
 
 static void vfio_free_group_minor(int minor)
@@ -243,7 +241,8 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group)
 		}
 	}
 
-	dev = device_create(vfio.class, NULL, MKDEV(MAJOR(vfio.devt), minor),
+	dev = device_create(vfio.class, NULL,
+			    MKDEV(MAJOR(vfio.group_devt), minor),
 			    group, "%d", iommu_group_id(iommu_group));
 	if (IS_ERR(dev)) {
 		vfio_free_group_minor(minor);
@@ -268,7 +267,7 @@ static void vfio_group_release(struct kref *kref)
 
 	WARN_ON(!list_empty(&group->device_list));
 
-	device_destroy(vfio.class, MKDEV(MAJOR(vfio.devt), group->minor));
+	device_destroy(vfio.class, MKDEV(MAJOR(vfio.group_devt), group->minor));
 	list_del(&group->vfio_next);
 	vfio_free_group_minor(group->minor);
 	vfio_group_unlock_and_free(group);
@@ -1419,12 +1418,17 @@ EXPORT_SYMBOL_GPL(vfio_external_user_iommu_id);
  */
 static char *vfio_devnode(struct device *dev, umode_t *mode)
 {
-	if (mode && (MINOR(dev->devt) == 0))
-		*mode = S_IRUGO | S_IWUGO;
-
 	return kasprintf(GFP_KERNEL, "vfio/%s", dev_name(dev));
 }
 
+static struct miscdevice vfio_dev = {
+	.minor = VFIO_MINOR,
+	.name = "vfio",
+	.fops = &vfio_fops,
+	.nodename = "vfio/vfio",
+	.mode = S_IRUGO | S_IWUGO,
+};
+
 static int __init vfio_init(void)
 {
 	int ret;
@@ -1436,6 +1440,13 @@ static int __init vfio_init(void)
 	INIT_LIST_HEAD(&vfio.iommu_drivers_list);
 	init_waitqueue_head(&vfio.release_q);
 
+	ret = misc_register(&vfio_dev);
+	if (ret) {
+		pr_err("vfio: misc device register failed\n");
+		return ret;
+	}
+
+	/* /dev/vfio/$GROUP */
 	vfio.class = class_create(THIS_MODULE, "vfio");
 	if (IS_ERR(vfio.class)) {
 		ret = PTR_ERR(vfio.class);
@@ -1444,27 +1455,14 @@ static int __init vfio_init(void)
 
 	vfio.class->devnode = vfio_devnode;
 
-	ret = alloc_chrdev_region(&vfio.devt, 0, MINORMASK, "vfio");
-	if (ret)
-		goto err_base_chrdev;
-
-	cdev_init(&vfio.cdev, &vfio_fops);
-	ret = cdev_add(&vfio.cdev, vfio.devt, 1);
+	ret = alloc_chrdev_region(&vfio.group_devt, 0, MINORMASK, "vfio");
 	if (ret)
-		goto err_base_cdev;
+		goto err_alloc_chrdev;
 
-	vfio.dev = device_create(vfio.class, NULL, vfio.devt, NULL, "vfio");
-	if (IS_ERR(vfio.dev)) {
-		ret = PTR_ERR(vfio.dev);
-		goto err_base_dev;
-	}
-
-	/* /dev/vfio/$GROUP */
 	cdev_init(&vfio.group_cdev, &vfio_group_fops);
-	ret = cdev_add(&vfio.group_cdev,
-		       MKDEV(MAJOR(vfio.devt), 1), MINORMASK - 1);
+	ret = cdev_add(&vfio.group_cdev, vfio.group_devt, MINORMASK);
 	if (ret)
-		goto err_groups_cdev;
+		goto err_cdev_add;
 
 	pr_info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
 
@@ -1478,16 +1476,13 @@ static int __init vfio_init(void)
 
 	return 0;
 
-err_groups_cdev:
-	device_destroy(vfio.class, vfio.devt);
-err_base_dev:
-	cdev_del(&vfio.cdev);
-err_base_cdev:
-	unregister_chrdev_region(vfio.devt, MINORMASK);
-err_base_chrdev:
+err_cdev_add:
+	unregister_chrdev_region(vfio.group_devt, MINORMASK);
+err_alloc_chrdev:
 	class_destroy(vfio.class);
 	vfio.class = NULL;
 err_class:
+	misc_deregister(&vfio_dev);
 	return ret;
 }
 
@@ -1497,11 +1492,10 @@ static void __exit vfio_cleanup(void)
 
 	idr_destroy(&vfio.group_idr);
 	cdev_del(&vfio.group_cdev);
-	device_destroy(vfio.class, vfio.devt);
-	cdev_del(&vfio.cdev);
-	unregister_chrdev_region(vfio.devt, MINORMASK);
+	unregister_chrdev_region(vfio.group_devt, MINORMASK);
 	class_destroy(vfio.class);
 	vfio.class = NULL;
+	misc_deregister(&vfio_dev);
 }
 
 module_init(vfio_init);
@@ -1511,3 +1505,5 @@ MODULE_VERSION(DRIVER_VERSION);
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_ALIAS_MISCDEV(VFIO_MINOR);
+MODULE_ALIAS("devname:vfio/vfio");

Re: [PATCH 1/2] misc: Reserve minor for VFIO

[One Thousand Gnomes]

> VFIO currently allocates it's own dynamic chardev range, reserving the
> first minor for the control part of the interface (/dev/vfio/vfio) and
> the remainder for VFIO groups (/dev/vfio/$GROUP).  This works, but it
> doesn't support auto loading.  For instance when libvirt checks for
> VFIO support it looks for /dev/vfio/vfio, which currently doesn't
> exist unless the vfio module is loaded.  By converting the control
> device to a misc driver and reserving a static minor, we can enable
> auto loading.

Looks sensible to me. I'm not sure if Lanana even really goes anywhere
any more however. Linus kept complaining about static device numbering
despite the fact it's still essential for a few odd cases like this.

Alan