From patchwork Tue Aug 16 08:14:13 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jike Song X-Patchwork-Id: 659520 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sD4x86ssGz9t2S for ; Tue, 16 Aug 2016 18:18:04 +1000 (AEST) Received: from localhost ([::1]:40683 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZZZS-0003KV-KH for incoming@patchwork.ozlabs.org; Tue, 16 Aug 2016 04:18:02 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44280) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZZY8-0002QH-Qk for qemu-devel@nongnu.org; Tue, 16 Aug 2016 04:16:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bZZY2-0005HR-JE for qemu-devel@nongnu.org; Tue, 16 Aug 2016 04:16:39 -0400 Received: from mga02.intel.com ([134.134.136.20]:26029) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZZY2-0005HC-4I for qemu-devel@nongnu.org; Tue, 16 Aug 2016 04:16:34 -0400 Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP; 16 Aug 2016 01:16:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.28,529,1464678000"; d="scan'208"; a="1026170507" Received: from kvmgt1.bj.intel.com ([10.238.154.158]) by fmsmga001.fm.intel.com with ESMTP; 16 Aug 2016 01:16:07 -0700 From: Jike Song To: alex.williamson@redhat.com, kwankhede@nvidia.com, cjia@nvidia.com Date: Tue, 16 Aug 2016 16:14:13 +0800 Message-Id: <1471335257-6226-2-git-send-email-jike.song@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1471335257-6226-1-git-send-email-jike.song@intel.com> References: <1471335257-6226-1-git-send-email-jike.song@intel.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 134.134.136.20 Subject: [Qemu-devel] [RFC v6-based v1 1/5] mdev: create separate device for parent_device X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kevin.tian@intel.com, guangrong.xiao@linux.intel.com, kvm@vger.kernel.org, qemu-devel@nongnu.org, zhenyuw@linux.intel.com, jike.song@intel.com, kraxel@redhat.com, pbonzini@redhat.com, bjsdjshi@linux.vnet.ibm.com Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Xiao Guangrong 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 --- 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);