From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753960AbcJTH07 (ORCPT ); Thu, 20 Oct 2016 03:26:59 -0400 Received: from mga11.intel.com ([192.55.52.93]:55619 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752796AbcJTH05 (ORCPT ); Thu, 20 Oct 2016 03:26:57 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,517,1473145200"; d="scan'208";a="1047338084" Message-ID: <58087109.8010308@intel.com> Date: Thu, 20 Oct 2016 15:23:53 +0800 From: Jike Song User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 MIME-Version: 1.0 To: Kirti Wankhede CC: alex.williamson@redhat.com, pbonzini@redhat.com, kraxel@redhat.com, cjia@nvidia.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, kevin.tian@intel.com, bjsdjshi@linux.vnet.ibm.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v9 01/12] vfio: Mediated device Core driver References: <1476739332-4911-1-git-send-email-kwankhede@nvidia.com> <1476739332-4911-2-git-send-email-kwankhede@nvidia.com> In-Reply-To: <1476739332-4911-2-git-send-email-kwankhede@nvidia.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/18/2016 05:22 AM, Kirti Wankhede wrote: > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c > new file mode 100644 > index 000000000000..7db5ec164aeb > --- /dev/null > +++ b/drivers/vfio/mdev/mdev_core.c > @@ -0,0 +1,372 @@ > +/* > + * Mediated device Core Driver > + * > + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. > + * Author: Neo Jia > + * Kirti Wankhede > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > + > +#include "mdev_private.h" > + > +#define DRIVER_VERSION "0.1" > +#define DRIVER_AUTHOR "NVIDIA Corporation" > +#define DRIVER_DESC "Mediated device Core Driver" > + > +static LIST_HEAD(parent_list); > +static DEFINE_MUTEX(parent_list_lock); > +static struct class_compat *mdev_bus_compat_class; > + > + > +/* > + * mdev_register_device : Register a device > + * @dev: device structure representing parent device. > + * @ops: Parent device operation structure to be registered. > + * > + * Add device to list of registered parent devices. > + * Returns a negative value on error, otherwise 0. > + */ > +int mdev_register_device(struct device *dev, const struct parent_ops *ops) > +{ > + int ret = 0; > + struct parent_device *parent; > + > + /* check for mandatory ops */ > + if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups) > + return -EINVAL; > + > + dev = get_device(dev); > + if (!dev) > + 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); > + > + parent->dev = dev; > + parent->ops = ops; > + > + ret = parent_create_sysfs_files(parent); > + if (ret) { > + mutex_unlock(&parent_list_lock); > + mdev_put_parent(parent); > + return ret; > + } > + > + ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL); > + if (ret) > + dev_warn(dev, "Failed to create compatibility class link\n"); > + > + list_add(&parent->next, &parent_list); > + mutex_unlock(&parent_list_lock); > + > + dev_info(dev, "MDEV: Registered\n"); > + return 0; > + > +add_dev_err: > + mutex_unlock(&parent_list_lock); > + put_device(dev); > + return ret; > +} > +EXPORT_SYMBOL(mdev_register_device); > +static int __init mdev_init(void) > +{ > + int ret; > + > + ret = mdev_bus_register(); > + if (ret) { > + pr_err("Failed to register mdev bus\n"); > + return ret; > + } > + > + mdev_bus_compat_class = class_compat_register("mdev_bus"); > + if (!mdev_bus_compat_class) { > + mdev_bus_unregister(); > + return -ENOMEM; > + } > + > + /* > + * Attempt to load known vfio_mdev. This gives us a working environment > + * without the user needing to explicitly load vfio_mdev driver. > + */ > + request_module_nowait("vfio_mdev"); > + > + return ret; > +} > + > +static void __exit mdev_exit(void) > +{ > + class_compat_unregister(mdev_bus_compat_class); > + mdev_bus_unregister(); > +} > + > +module_init(mdev_init) > +module_exit(mdev_exit) Hi Kirti, There is a possible issue: mdev_bus_register is called from mdev_init, a module_init, equal to device_initcall if builtin to vmlinux; however, the vendor driver, say i915.ko for intel case, have to call mdev_register_device from its module_init: at that time, mdev_init is still not called. Not sure if this issue exists with nvidia.ko. Though in most cases we are expecting users select mdev as a standalone module, we still won't break builtin case. Hi Alex, do you have any suggestion here? -- Thanks, Jike From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46702) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bx7kk-0005BI-TX for qemu-devel@nongnu.org; Thu, 20 Oct 2016 03:27:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bx7kg-0000qG-T9 for qemu-devel@nongnu.org; Thu, 20 Oct 2016 03:27:02 -0400 Received: from mga05.intel.com ([192.55.52.43]:28496) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bx7kg-0000pl-I6 for qemu-devel@nongnu.org; Thu, 20 Oct 2016 03:26:58 -0400 Message-ID: <58087109.8010308@intel.com> Date: Thu, 20 Oct 2016 15:23:53 +0800 From: Jike Song MIME-Version: 1.0 References: <1476739332-4911-1-git-send-email-kwankhede@nvidia.com> <1476739332-4911-2-git-send-email-kwankhede@nvidia.com> In-Reply-To: <1476739332-4911-2-git-send-email-kwankhede@nvidia.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v9 01/12] vfio: Mediated device Core driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kirti Wankhede Cc: alex.williamson@redhat.com, pbonzini@redhat.com, kraxel@redhat.com, cjia@nvidia.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, kevin.tian@intel.com, bjsdjshi@linux.vnet.ibm.com, linux-kernel@vger.kernel.org On 10/18/2016 05:22 AM, Kirti Wankhede wrote: > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c > new file mode 100644 > index 000000000000..7db5ec164aeb > --- /dev/null > +++ b/drivers/vfio/mdev/mdev_core.c > @@ -0,0 +1,372 @@ > +/* > + * Mediated device Core Driver > + * > + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. > + * Author: Neo Jia > + * Kirti Wankhede > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > + > +#include "mdev_private.h" > + > +#define DRIVER_VERSION "0.1" > +#define DRIVER_AUTHOR "NVIDIA Corporation" > +#define DRIVER_DESC "Mediated device Core Driver" > + > +static LIST_HEAD(parent_list); > +static DEFINE_MUTEX(parent_list_lock); > +static struct class_compat *mdev_bus_compat_class; > + > + > +/* > + * mdev_register_device : Register a device > + * @dev: device structure representing parent device. > + * @ops: Parent device operation structure to be registered. > + * > + * Add device to list of registered parent devices. > + * Returns a negative value on error, otherwise 0. > + */ > +int mdev_register_device(struct device *dev, const struct parent_ops *ops) > +{ > + int ret = 0; > + struct parent_device *parent; > + > + /* check for mandatory ops */ > + if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups) > + return -EINVAL; > + > + dev = get_device(dev); > + if (!dev) > + 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); > + > + parent->dev = dev; > + parent->ops = ops; > + > + ret = parent_create_sysfs_files(parent); > + if (ret) { > + mutex_unlock(&parent_list_lock); > + mdev_put_parent(parent); > + return ret; > + } > + > + ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL); > + if (ret) > + dev_warn(dev, "Failed to create compatibility class link\n"); > + > + list_add(&parent->next, &parent_list); > + mutex_unlock(&parent_list_lock); > + > + dev_info(dev, "MDEV: Registered\n"); > + return 0; > + > +add_dev_err: > + mutex_unlock(&parent_list_lock); > + put_device(dev); > + return ret; > +} > +EXPORT_SYMBOL(mdev_register_device); > +static int __init mdev_init(void) > +{ > + int ret; > + > + ret = mdev_bus_register(); > + if (ret) { > + pr_err("Failed to register mdev bus\n"); > + return ret; > + } > + > + mdev_bus_compat_class = class_compat_register("mdev_bus"); > + if (!mdev_bus_compat_class) { > + mdev_bus_unregister(); > + return -ENOMEM; > + } > + > + /* > + * Attempt to load known vfio_mdev. This gives us a working environment > + * without the user needing to explicitly load vfio_mdev driver. > + */ > + request_module_nowait("vfio_mdev"); > + > + return ret; > +} > + > +static void __exit mdev_exit(void) > +{ > + class_compat_unregister(mdev_bus_compat_class); > + mdev_bus_unregister(); > +} > + > +module_init(mdev_init) > +module_exit(mdev_exit) Hi Kirti, There is a possible issue: mdev_bus_register is called from mdev_init, a module_init, equal to device_initcall if builtin to vmlinux; however, the vendor driver, say i915.ko for intel case, have to call mdev_register_device from its module_init: at that time, mdev_init is still not called. Not sure if this issue exists with nvidia.ko. Though in most cases we are expecting users select mdev as a standalone module, we still won't break builtin case. Hi Alex, do you have any suggestion here? -- Thanks, Jike