diff mbox

[v2,1/4] vfio/mdev: Add new instances parameter for mdev create

Message ID 20180720021928.15343-2-zhenyuw@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhenyu Wang July 20, 2018, 2:19 a.m. UTC
For special mdev type which can aggregate instances for mdev device,
this extends mdev create interface by allowing extra "instances=xxx"
parameter, which is passed to mdev device model to be able to create
arbitrary bundled number of instances for target mdev device.

v2: create new create_with_instances operator for vendor driver

Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/vfio/mdev/mdev_core.c    | 18 +++++++++++++----
 drivers/vfio/mdev/mdev_private.h |  5 ++++-
 drivers/vfio/mdev/mdev_sysfs.c   | 34 ++++++++++++++++++++++++++------
 include/linux/mdev.h             | 10 ++++++++++
 4 files changed, 56 insertions(+), 11 deletions(-)

Comments

Cornelia Huck July 26, 2018, 3:37 p.m. UTC | #1
On Fri, 20 Jul 2018 10:19:25 +0800
Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> For special mdev type which can aggregate instances for mdev device,
> this extends mdev create interface by allowing extra "instances=xxx"
> parameter, which is passed to mdev device model to be able to create
> arbitrary bundled number of instances for target mdev device.
> 
> v2: create new create_with_instances operator for vendor driver
> 
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/vfio/mdev/mdev_core.c    | 18 +++++++++++++----
>  drivers/vfio/mdev/mdev_private.h |  5 ++++-
>  drivers/vfio/mdev/mdev_sysfs.c   | 34 ++++++++++++++++++++++++++------
>  include/linux/mdev.h             | 10 ++++++++++
>  4 files changed, 56 insertions(+), 11 deletions(-)
> 

(...)

> diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
> index 249472f05509..a06e5b7c69d3 100644
> --- a/drivers/vfio/mdev/mdev_sysfs.c
> +++ b/drivers/vfio/mdev/mdev_sysfs.c
> @@ -54,11 +54,15 @@ static const struct sysfs_ops mdev_type_sysfs_ops = {
>  static ssize_t create_store(struct kobject *kobj, struct device *dev,
>  			    const char *buf, size_t count)
>  {
> -	char *str;
> +	char *str, *opt = NULL;
>  	uuid_le uuid;
>  	int ret;
> +	unsigned int instances = 1;
>  
> -	if ((count < UUID_STRING_LEN) || (count > UUID_STRING_LEN + 1))
> +	if (count < UUID_STRING_LEN)
> +		return -EINVAL;
> +
> +	if (count > UUID_STRING_LEN + 1 + MDEV_CREATE_OPT_LEN)

Do you plan to have other optional parameters? If you don't, you could
probably do a quick exit here if count is between UUID_STRING_LEN + 1
and UUID_STRING_LEN + 12 (for ",instances=<one digit>")?

>  		return -EINVAL;
>  
>  	str = kstrndup(buf, count, GFP_KERNEL);

(...)

> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index b6e048e1045f..cfb702600f95 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -30,6 +30,13 @@ struct mdev_device;
>   *			@kobj: kobject of type for which 'create' is called.
>   *			@mdev: mdev_device structure on of mediated device
>   *			      that is being created
> + * @create_with_instances: Allocate aggregated instances' resources in parent device's
> + *			driver for a particular mediated device. It is optional
> + *			if doesn't support aggregated resources.

"Optional if aggregated resources are not supported"

> + *			@kobj: kobject of type for which 'create' is called.
> + *			@mdev: mdev_device structure on of mediated device
> + *			      that is being created
> + *                      @instances: number of instances to aggregate
>   *			Returns integer: success (0) or error (< 0)

You need that "Returns" line for both the old and the new ops.

>   * @remove:		Called to free resources in parent device's driver for a
>   *			a mediated device. It is mandatory to provide 'remove'
> @@ -71,6 +78,9 @@ struct mdev_parent_ops {
>  	struct attribute_group **supported_type_groups;
>  
>  	int     (*create)(struct kobject *kobj, struct mdev_device *mdev);
> +	int     (*create_with_instances)(struct kobject *kobj,
> +					 struct mdev_device *mdev,
> +					 unsigned int instances);
>  	int     (*remove)(struct mdev_device *mdev);
>  	int     (*open)(struct mdev_device *mdev);
>  	void    (*release)(struct mdev_device *mdev);
diff mbox

Patch

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 0212f0ee8aea..e69db302a4f2 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -104,12 +104,16 @@  static inline void mdev_put_parent(struct mdev_parent *parent)
 }
 
 static int mdev_device_create_ops(struct kobject *kobj,
-				  struct mdev_device *mdev)
+				  struct mdev_device *mdev,
+				  unsigned int instances)
 {
 	struct mdev_parent *parent = mdev->parent;
 	int ret;
 
-	ret = parent->ops->create(kobj, mdev);
+	if (instances > 1)
+		ret = parent->ops->create_with_instances(kobj, mdev, instances);
+	else
+		ret = parent->ops->create(kobj, mdev);
 	if (ret)
 		return ret;
 
@@ -276,7 +280,8 @@  static void mdev_device_release(struct device *dev)
 	kfree(mdev);
 }
 
-int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid)
+int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid,
+		       unsigned int instances)
 {
 	int ret;
 	struct mdev_device *mdev, *tmp;
@@ -287,6 +292,11 @@  int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid)
 	if (!parent)
 		return -EINVAL;
 
+	if (instances > 1 && !parent->ops->create_with_instances) {
+		ret = -EINVAL;
+		goto mdev_fail;
+	}
+
 	mutex_lock(&mdev_list_lock);
 
 	/* Check for duplicate */
@@ -323,7 +333,7 @@  int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid)
 		goto mdev_fail;
 	}
 
-	ret = mdev_device_create_ops(kobj, mdev);
+	ret = mdev_device_create_ops(kobj, mdev, instances);
 	if (ret)
 		goto create_fail;
 
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
index b5819b7d7ef7..c9d3fe04e273 100644
--- a/drivers/vfio/mdev/mdev_private.h
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -52,13 +52,16 @@  struct mdev_type {
 #define to_mdev_type(_kobj)		\
 	container_of(_kobj, struct mdev_type, kobj)
 
+#define MDEV_CREATE_OPT_LEN 32
+
 int  parent_create_sysfs_files(struct mdev_parent *parent);
 void parent_remove_sysfs_files(struct mdev_parent *parent);
 
 int  mdev_create_sysfs_files(struct device *dev, struct mdev_type *type);
 void mdev_remove_sysfs_files(struct device *dev, struct mdev_type *type);
 
-int  mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid);
+int  mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid,
+			unsigned int instances);
 int  mdev_device_remove(struct device *dev, bool force_remove);
 
 #endif /* MDEV_PRIVATE_H */
diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
index 249472f05509..a06e5b7c69d3 100644
--- a/drivers/vfio/mdev/mdev_sysfs.c
+++ b/drivers/vfio/mdev/mdev_sysfs.c
@@ -54,11 +54,15 @@  static const struct sysfs_ops mdev_type_sysfs_ops = {
 static ssize_t create_store(struct kobject *kobj, struct device *dev,
 			    const char *buf, size_t count)
 {
-	char *str;
+	char *str, *opt = NULL;
 	uuid_le uuid;
 	int ret;
+	unsigned int instances = 1;
 
-	if ((count < UUID_STRING_LEN) || (count > UUID_STRING_LEN + 1))
+	if (count < UUID_STRING_LEN)
+		return -EINVAL;
+
+	if (count > UUID_STRING_LEN + 1 + MDEV_CREATE_OPT_LEN)
 		return -EINVAL;
 
 	str = kstrndup(buf, count, GFP_KERNEL);
@@ -66,13 +70,31 @@  static ssize_t create_store(struct kobject *kobj, struct device *dev,
 		return -ENOMEM;
 
 	ret = uuid_le_to_bin(str, &uuid);
-	kfree(str);
-	if (ret)
+	if (ret) {
+		kfree(str);
 		return ret;
+	}
 
-	ret = mdev_device_create(kobj, dev, uuid);
-	if (ret)
+	if (count > UUID_STRING_LEN + 1) {
+		opt = str + UUID_STRING_LEN;
+		if (*opt++ != ',' ||
+		    strncmp(opt, "instances=", 10)) {
+			kfree(str);
+			return -EINVAL;
+		}
+		opt += 10;
+		if (kstrtouint(opt, 10, &instances)) {
+			kfree(str);
+			return -EINVAL;
+		}
+	}
+
+	ret = mdev_device_create(kobj, dev, uuid, instances);
+	if (ret) {
+		kfree(str);
 		return ret;
+	}
+	kfree(str);
 
 	return count;
 }
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index b6e048e1045f..cfb702600f95 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -30,6 +30,13 @@  struct mdev_device;
  *			@kobj: kobject of type for which 'create' is called.
  *			@mdev: mdev_device structure on of mediated device
  *			      that is being created
+ * @create_with_instances: Allocate aggregated instances' resources in parent device's
+ *			driver for a particular mediated device. It is optional
+ *			if doesn't support aggregated resources.
+ *			@kobj: kobject of type for which 'create' is called.
+ *			@mdev: mdev_device structure on of mediated device
+ *			      that is being created
+ *                      @instances: number of instances to aggregate
  *			Returns integer: success (0) or error (< 0)
  * @remove:		Called to free resources in parent device's driver for a
  *			a mediated device. It is mandatory to provide 'remove'
@@ -71,6 +78,9 @@  struct mdev_parent_ops {
 	struct attribute_group **supported_type_groups;
 
 	int     (*create)(struct kobject *kobj, struct mdev_device *mdev);
+	int     (*create_with_instances)(struct kobject *kobj,
+					 struct mdev_device *mdev,
+					 unsigned int instances);
 	int     (*remove)(struct mdev_device *mdev);
 	int     (*open)(struct mdev_device *mdev);
 	void    (*release)(struct mdev_device *mdev);