From: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
By introducing a separate device for parent_device, we can have
the parent list and lock removed, letting driver core and sysfs
to deal with the mutual exclusion.
Signed-off-by: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
---
drivers/vfio/mdev/mdev_core.c | 242 +++++----------------------------------
drivers/vfio/mdev/mdev_private.h | 9 +-
drivers/vfio/mdev/mdev_sysfs.c | 69 +++--------
drivers/vfio/mdev/vfio_mpci.c | 2 +-
include/linux/mdev.h | 6 +-
5 files changed, 57 insertions(+), 271 deletions(-)
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 90ff073..9138588 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -27,11 +27,6 @@
#define DRIVER_AUTHOR "NVIDIA Corporation"
#define DRIVER_DESC "Mediated device Core Driver"
-#define MDEV_CLASS_NAME "mdev"
-
-static LIST_HEAD(parent_list);
-static DEFINE_MUTEX(parent_list_lock);
-
static int mdev_add_attribute_group(struct device *dev,
const struct attribute_group **groups)
{
@@ -58,55 +53,6 @@ static struct mdev_device *find_mdev_device(struct parent_device *parent,
return NULL;
}
-/* Should be called holding parent_list_lock */
-static struct parent_device *find_parent_device(struct device *dev)
-{
- struct parent_device *parent;
-
- list_for_each_entry(parent, &parent_list, next) {
- if (parent->dev == dev)
- return parent;
- }
- return NULL;
-}
-
-static void mdev_release_parent(struct kref *kref)
-{
- struct parent_device *parent = container_of(kref, struct parent_device,
- ref);
- kfree(parent);
-}
-
-static
-inline struct parent_device *mdev_get_parent(struct parent_device *parent)
-{
- if (parent)
- kref_get(&parent->ref);
-
- return parent;
-}
-
-static inline void mdev_put_parent(struct parent_device *parent)
-{
- if (parent)
- kref_put(&parent->ref, mdev_release_parent);
-}
-
-static struct parent_device *mdev_get_parent_by_dev(struct device *dev)
-{
- struct parent_device *parent = NULL, *p;
-
- mutex_lock(&parent_list_lock);
- list_for_each_entry(p, &parent_list, next) {
- if (p->dev == dev) {
- parent = mdev_get_parent(p);
- break;
- }
- }
- mutex_unlock(&parent_list_lock);
- return parent;
-}
-
static int mdev_device_create_ops(struct mdev_device *mdev, char *mdev_params)
{
struct parent_device *parent = mdev->parent;
@@ -153,7 +99,6 @@ static void mdev_release_device(struct kref *kref)
device_unregister(&mdev->dev);
wake_up(&parent->release_done);
- mdev_put_parent(parent);
}
struct mdev_device *mdev_get_device(struct mdev_device *mdev)
@@ -173,66 +118,6 @@ void mdev_put_device(struct mdev_device *mdev)
EXPORT_SYMBOL(mdev_put_device);
/*
- * Find first mediated device from given uuid and increment refcount of
- * mediated device. Caller should call mdev_put_device() when the use of
- * mdev_device is done.
- */
-static struct mdev_device *mdev_get_first_device_by_uuid(uuid_le uuid)
-{
- struct mdev_device *mdev = NULL, *p;
- struct parent_device *parent;
-
- mutex_lock(&parent_list_lock);
- list_for_each_entry(parent, &parent_list, next) {
- mutex_lock(&parent->mdev_list_lock);
- list_for_each_entry(p, &parent->mdev_list, next) {
- if (uuid_le_cmp(p->uuid, uuid) == 0) {
- mdev = mdev_get_device(p);
- break;
- }
- }
- mutex_unlock(&parent->mdev_list_lock);
-
- if (mdev)
- break;
- }
- mutex_unlock(&parent_list_lock);
- return mdev;
-}
-
-/*
- * Find mediated device from given iommu_group and increment refcount of
- * mediated device. Caller should call mdev_put_device() when the use of
- * mdev_device is done.
- */
-struct mdev_device *mdev_get_device_by_group(struct iommu_group *group)
-{
- struct mdev_device *mdev = NULL, *p;
- struct parent_device *parent;
-
- mutex_lock(&parent_list_lock);
- list_for_each_entry(parent, &parent_list, next) {
- mutex_lock(&parent->mdev_list_lock);
- list_for_each_entry(p, &parent->mdev_list, next) {
- if (!p->group)
- continue;
-
- if (iommu_group_id(p->group) == iommu_group_id(group)) {
- mdev = mdev_get_device(p);
- break;
- }
- }
- mutex_unlock(&parent->mdev_list_lock);
-
- if (mdev)
- break;
- }
- mutex_unlock(&parent_list_lock);
- return mdev;
-}
-EXPORT_SYMBOL(mdev_get_device_by_group);
-
-/*
* mdev_register_device : Register a device
* @dev: device structure representing parent device.
* @ops: Parent device operation structure to be registered.
@@ -252,30 +137,21 @@ int mdev_register_device(struct device *dev, const struct parent_ops *ops)
if (!ops->create || !ops->destroy)
return -EINVAL;
- mutex_lock(&parent_list_lock);
-
- /* Check for duplicate */
- parent = find_parent_device(dev);
- if (parent) {
- ret = -EEXIST;
- goto add_dev_err;
- }
-
parent = kzalloc(sizeof(*parent), GFP_KERNEL);
if (!parent) {
ret = -ENOMEM;
goto add_dev_err;
}
- kref_init(&parent->ref);
- list_add(&parent->next, &parent_list);
-
- parent->dev = dev;
+ parent->dev.parent = dev;
parent->ops = ops;
mutex_init(&parent->mdev_list_lock);
INIT_LIST_HEAD(&parent->mdev_list);
init_waitqueue_head(&parent->release_done);
- mutex_unlock(&parent_list_lock);
+
+ ret = device_register(&parent->dev);
+ if (ret)
+ goto register_error;
ret = mdev_create_sysfs_files(dev);
if (ret)
@@ -291,50 +167,37 @@ int mdev_register_device(struct device *dev, const struct parent_ops *ops)
add_group_error:
mdev_remove_sysfs_files(dev);
add_sysfs_error:
- mutex_lock(&parent_list_lock);
- list_del(&parent->next);
- mutex_unlock(&parent_list_lock);
- mdev_put_parent(parent);
+ device_unregister(&parent->dev);
+register_error:
return ret;
add_dev_err:
- mutex_unlock(&parent_list_lock);
return ret;
}
EXPORT_SYMBOL(mdev_register_device);
/*
* mdev_unregister_device : Unregister a parent device
- * @dev: device structure representing parent device.
+ * @dev: device structure representing parent device which is returned by
+ * mdev_register_device.
*
* Remove device from list of registered parent devices. Give a chance to free
* existing mediated devices for given device.
*/
-
-void mdev_unregister_device(struct device *dev)
+void mdev_unregister_device(struct parent_device *parent)
{
- struct parent_device *parent;
struct mdev_device *mdev, *n;
int ret;
- mutex_lock(&parent_list_lock);
- parent = find_parent_device(dev);
-
- if (!parent) {
- mutex_unlock(&parent_list_lock);
- return;
- }
- dev_info(dev, "MDEV: Unregistering\n");
+ dev_info(&parent->dev, "MDEV: Unregistering\n");
/*
* Remove parent from the list and remove create and destroy sysfs
* files so that no new mediated device could be created for this parent
*/
- list_del(&parent->next);
- mdev_remove_sysfs_files(dev);
- mutex_unlock(&parent_list_lock);
+ mdev_remove_sysfs_files(&parent->dev);
- mdev_remove_attribute_group(dev,
+ mdev_remove_attribute_group(&parent->dev,
parent->ops->dev_attr_groups);
mutex_lock(&parent->mdev_list_lock);
@@ -350,14 +213,12 @@ void mdev_unregister_device(struct device *dev)
ret = wait_event_interruptible_timeout(parent->release_done,
list_empty(&parent->mdev_list), HZ * 10);
if (ret == -ERESTARTSYS) {
- dev_warn(dev, "Mediated devices are in use, task"
+ dev_warn(&parent->dev, "Mediated devices are in use, task"
" \"%s\" (%d) "
"blocked until all are released",
current->comm, task_pid_nr(current));
}
} while (ret <= 0);
-
- mdev_put_parent(parent);
}
EXPORT_SYMBOL(mdev_unregister_device);
@@ -377,11 +238,7 @@ int mdev_device_create(struct device *dev, uuid_le uuid, uint32_t instance,
{
int ret;
struct mdev_device *mdev;
- struct parent_device *parent;
-
- parent = mdev_get_parent_by_dev(dev);
- if (!parent)
- return -EINVAL;
+ struct parent_device *parent = dev_to_parent_dev(dev);
mutex_lock(&parent->mdev_list_lock);
/* Check for duplicate */
@@ -403,6 +260,7 @@ int mdev_device_create(struct device *dev, uuid_le uuid, uint32_t instance,
kref_init(&mdev->ref);
mdev->dev.parent = dev;
+ mdev->dev.class = &mdev_class;
mdev->dev.bus = &mdev_bus_type;
mdev->dev.release = mdev_device_release;
dev_set_name(&mdev->dev, "%pUl-%d", uuid.b, instance);
@@ -429,19 +287,14 @@ create_failed:
create_err:
mutex_unlock(&parent->mdev_list_lock);
- mdev_put_parent(parent);
return ret;
}
int mdev_device_destroy(struct device *dev, uuid_le uuid, uint32_t instance)
{
struct mdev_device *mdev;
- struct parent_device *parent;
int ret;
-
- parent = mdev_get_parent_by_dev(dev);
- if (!parent)
- return -EINVAL;
+ struct parent_device *parent = dev_to_parent_dev(dev);
mutex_lock(&parent->mdev_list_lock);
mdev = find_mdev_device(parent, uuid, instance);
@@ -457,12 +310,10 @@ int mdev_device_destroy(struct device *dev, uuid_le uuid, uint32_t instance)
mutex_unlock(&parent->mdev_list_lock);
mdev_put_device(mdev);
- mdev_put_parent(parent);
return ret;
destroy_err:
mutex_unlock(&parent->mdev_list_lock);
- mdev_put_parent(parent);
return ret;
}
@@ -575,72 +426,37 @@ EXPORT_SYMBOL(mdev_del_phys_mapping);
void mdev_device_supported_config(struct device *dev, char *str)
{
- struct parent_device *parent;
-
- parent = mdev_get_parent_by_dev(dev);
+ struct parent_device *parent = dev_to_parent_dev(dev);
if (parent) {
if (parent->ops->supported_config)
- parent->ops->supported_config(parent->dev, str);
- mdev_put_parent(parent);
+ parent->ops->supported_config(&parent->dev, str);
}
}
-int mdev_device_start(uuid_le uuid)
+int mdev_device_start(struct device *dev, bool start)
{
int ret = 0;
- struct mdev_device *mdev;
- struct parent_device *parent;
+ struct mdev_device *mdev = dev_to_mdev(dev);
+ struct parent_device *parent = dev_to_parent_dev(dev->parent);
- mdev = mdev_get_first_device_by_uuid(uuid);
- if (!mdev)
- return -EINVAL;
-
- parent = mdev->parent;
-
- if (parent->ops->start)
+ mdev_get_device(mdev);
+ if (start && parent->ops->start)
ret = parent->ops->start(mdev->uuid);
-
- if (ret)
- pr_err("mdev_start failed %d\n", ret);
- else
- kobject_uevent(&mdev->dev.kobj, KOBJ_ONLINE);
-
- mdev_put_device(mdev);
-
- return ret;
-}
-
-int mdev_device_stop(uuid_le uuid)
-{
- int ret = 0;
- struct mdev_device *mdev;
- struct parent_device *parent;
-
- mdev = mdev_get_first_device_by_uuid(uuid);
- if (!mdev)
- return -EINVAL;
-
- parent = mdev->parent;
-
- if (parent->ops->stop)
+ else if (!start && parent->ops->stop)
ret = parent->ops->stop(mdev->uuid);
if (ret)
- pr_err("mdev stop failed %d\n", ret);
+ pr_err("mdev %s failed %d\n", start ? "start" : "stop", ret);
else
- kobject_uevent(&mdev->dev.kobj, KOBJ_OFFLINE);
+ kobject_uevent(&mdev->dev.kobj,
+ start ? KOBJ_ONLINE : KOBJ_OFFLINE);
mdev_put_device(mdev);
+
return ret;
}
-static struct class mdev_class = {
- .name = MDEV_CLASS_NAME,
- .owner = THIS_MODULE,
- .class_attrs = mdev_class_attrs,
-};
-
static int __init mdev_init(void)
{
int ret;
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
index ee2db61..3fce40f 100644
--- a/drivers/vfio/mdev/mdev_private.h
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -13,12 +13,16 @@
#ifndef MDEV_PRIVATE_H
#define MDEV_PRIVATE_H
+#define dev_to_parent_dev(_dev) container_of((_dev), \
+ struct parent_device, dev)
+#define dev_to_mdev(_dev) container_of((_dev), struct mdev_device, dev)
+
int mdev_bus_register(void);
void mdev_bus_unregister(void);
/* Function prototypes for mdev_sysfs */
-extern struct class_attribute mdev_class_attrs[];
+extern struct class mdev_class;
int mdev_create_sysfs_files(struct device *dev);
void mdev_remove_sysfs_files(struct device *dev);
@@ -27,7 +31,6 @@ int mdev_device_create(struct device *dev, uuid_le uuid, uint32_t instance,
char *mdev_params);
int mdev_device_destroy(struct device *dev, uuid_le uuid, uint32_t instance);
void mdev_device_supported_config(struct device *dev, char *str);
-int mdev_device_start(uuid_le uuid);
-int mdev_device_stop(uuid_le uuid);
+int mdev_device_start(struct device *dev, bool start);
#endif /* MDEV_PRIVATE_H */
diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
index e0457e6..3080edc 100644
--- a/drivers/vfio/mdev/mdev_sysfs.c
+++ b/drivers/vfio/mdev/mdev_sysfs.c
@@ -171,65 +171,34 @@ destroy_error:
return ret;
}
-ssize_t mdev_start_store(struct class *class, struct class_attribute *attr,
- const char *buf, size_t count)
+static ssize_t start_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t len)
{
- char *uuid_str, *ptr;
- uuid_le uuid;
- int ret;
-
- ptr = uuid_str = kstrndup(buf, count, GFP_KERNEL);
-
- if (!uuid_str)
- return -ENOMEM;
+ int start, ret;
- ret = uuid_le_to_bin(uuid_str, &uuid);
- if (ret) {
- pr_err("mdev_start: UUID parse error %s\n", buf);
- goto start_error;
- }
+ ret = kstrtoint(buf, 10, &start);
+ if (ret)
+ goto exit;
- ret = mdev_device_start(uuid);
+ ret = mdev_device_start(dev, !!start);
if (ret == 0)
- ret = count;
-
-start_error:
- kfree(ptr);
+ ret = len;
+exit:
return ret;
}
+static DEVICE_ATTR_WO(start);
-ssize_t mdev_stop_store(struct class *class, struct class_attribute *attr,
- const char *buf, size_t count)
-{
- char *uuid_str, *ptr;
- uuid_le uuid;
- int ret;
-
- ptr = uuid_str = kstrndup(buf, count, GFP_KERNEL);
-
- if (!uuid_str)
- return -ENOMEM;
-
- ret = uuid_le_to_bin(uuid_str, &uuid);
- if (ret) {
- pr_err("mdev_stop: UUID parse error %s\n", buf);
- goto stop_error;
- }
-
- ret = mdev_device_stop(uuid);
- if (ret == 0)
- ret = count;
-
-stop_error:
- kfree(ptr);
- return ret;
+static struct attribute *mdev_device_attrs[] = {
+ &dev_attr_start.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(mdev_device);
-}
+#define MDEV_CLASS_NAME "mdev"
-struct class_attribute mdev_class_attrs[] = {
- __ATTR_WO(mdev_start),
- __ATTR_WO(mdev_stop),
- __ATTR_NULL
+struct class mdev_class = {
+ .name = MDEV_CLASS_NAME,
+ .dev_groups = mdev_device_groups,
};
int mdev_create_sysfs_files(struct device *dev)
diff --git a/drivers/vfio/mdev/vfio_mpci.c b/drivers/vfio/mdev/vfio_mpci.c
index 9da94b7..88c0ba6 100644
--- a/drivers/vfio/mdev/vfio_mpci.c
+++ b/drivers/vfio/mdev/vfio_mpci.c
@@ -420,7 +420,7 @@ static int mdev_dev_mmio_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
virtaddr = vma->vm_start;
req_size = vma->vm_end - vma->vm_start;
- pdev = to_pci_dev(parent->dev);
+ pdev = to_pci_dev(parent->dev.parent);
index = VFIO_PCI_OFFSET_TO_INDEX(vma->vm_pgoff << PAGE_SHIFT);
pgoff = pci_resource_start(pdev, index) >> PAGE_SHIFT;
}
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 0b41f30..42da41b 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -159,12 +159,10 @@ struct parent_ops {
*/
struct parent_device {
- struct device *dev;
+ struct device dev;
const struct parent_ops *ops;
/* internal */
- struct kref ref;
- struct list_head next;
struct list_head mdev_list;
struct mutex mdev_list_lock;
wait_queue_head_t release_done;
@@ -214,7 +212,7 @@ extern struct bus_type mdev_bus_type;
extern int mdev_register_device(struct device *dev,
const struct parent_ops *ops);
-extern void mdev_unregister_device(struct device *dev);
+extern void mdev_unregister_device(struct parent_device *parent);
extern int mdev_register_driver(struct mdev_driver *drv, struct module *owner);
extern void mdev_unregister_driver(struct mdev_driver *drv);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
