[PATCH 00/18] Make vfio_mdev type safe

[PATCH 00/18] Make vfio_mdev type safe

[Jason Gunthorpe]

Prologue
========

This is series #2 in part of a larger work that arose from the minor
remark that the mdev_parent_ops indirection shim is useless and
complicates things.

It follows the "Embed struct vfio_device in all sub-structures" already
sent, and must be applied on top of it.

A preview of the future series's is here:
  https://github.com/jgunthorpe/linux/pull/3/commits

========
This series:

vfio_mdev has a number of different objects: mdev_parent, mdev_type and
mdev_device.

Unfortunately the types of these have been erased in various places
throughout the API, and this makes it very hard to understand this code or
maintain it by the time it reaches all of the drivers.

This series puts in all the types and aligns some of the design with the
driver core standard for a driver core bus driver:

 - Replace 'struct device *' with 'struct mdev_device *
 - Replace 'struct device *' with 'struct mdev_type *' and
   mtype_get_parent_dev()
 - Replace 'struct kobject *' with 'struct mdev_type *'

Now that types are clear it is easy to spot a few places that have
duplicated information.

More significantly we can now understand how to directly fix the
obfuscated 'kobj->name' matching by realizing the the kobj is a mdev_type,
which is linked to the supported_types_list provided by the driver, and
thus the core code can directly return the array indexes all the drivers
actually want.

Jason

Jason Gunthorpe (18):
  vfio/mdev: Fix missing static's on MDEV_TYPE_ATTR's
  vfio/mdev: Add missing typesafety around mdev_device
  vfio/mdev: Simplify driver registration
  vfio/mdev: Use struct mdev_type in struct mdev_device
  vfio/mdev: Do not allow a mdev_type to have a NULL parent pointer
  vfio/mdev: Expose mdev_get/put_parent to mdev_private.h
  vfio/mdev: Add missing reference counting to mdev_type
  vfio/mdev: Reorganize mdev_device_create()
  vfio/mdev: Add missing error handling to dev_set_name()
  vfio/mdev: Remove duplicate storage of parent in mdev_device
  vfio/mdev: Add mdev/mtype_get_type_group_id()
  vfio/mtty: Use mdev_get_type_group_id()
  vfio/mdpy: Use mdev_get_type_group_id()
  vfio/mbochs: Use mdev_get_type_group_id()
  vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV
  vfio/gvt: Use mdev_get_type_group_id()
  vfio/mdev: Remove kobj from mdev_parent_ops->create()
  vfio/mdev: Correct the function signatures for the
    mdev_type_attributes

 .../driver-api/vfio-mediated-device.rst       |   9 +-
 drivers/gpu/drm/i915/Kconfig                  |   1 +
 drivers/gpu/drm/i915/gvt/gvt.c                |  41 ++---
 drivers/gpu/drm/i915/gvt/gvt.h                |   4 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c              |   7 +-
 drivers/s390/cio/vfio_ccw_ops.c               |  17 +-
 drivers/s390/crypto/vfio_ap_ops.c             |  14 +-
 drivers/vfio/mdev/mdev_core.c                 | 160 ++++++------------
 drivers/vfio/mdev/mdev_driver.c               |  19 +--
 drivers/vfio/mdev/mdev_private.h              |  40 ++---
 drivers/vfio/mdev/mdev_sysfs.c                |  59 ++++---
 drivers/vfio/mdev/vfio_mdev.c                 |  29 ++--
 drivers/vfio/vfio_iommu_type1.c               |  25 +--
 include/linux/mdev.h                          |  80 ++++++---
 samples/vfio-mdev/mbochs.c                    |  55 +++---
 samples/vfio-mdev/mdpy.c                      |  56 +++---
 samples/vfio-mdev/mtty.c                      |  66 ++------
 17 files changed, 306 insertions(+), 376 deletions(-)

-- 
2.31.0

[PATCH 17/18] vfio/mdev: Remove kobj from mdev_parent_ops->create()

[Jason Gunthorpe]

The kobj here is a type-erased version of mdev_type, which is already
stored in the struct mdev_device being passed in. It was only ever used to
compute the type_group_id, which is now extracted directly from the mdev.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c  | 2 +-
 drivers/s390/cio/vfio_ccw_ops.c   | 2 +-
 drivers/s390/crypto/vfio_ap_ops.c | 2 +-
 drivers/vfio/mdev/mdev_core.c     | 2 +-
 include/linux/mdev.h              | 3 +--
 samples/vfio-mdev/mbochs.c        | 2 +-
 samples/vfio-mdev/mdpy.c          | 2 +-
 samples/vfio-mdev/mtty.c          | 2 +-
 8 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 16e1e4a38aa1f6..6bf176e8426e63 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -689,7 +689,7 @@ static void kvmgt_put_vfio_device(void *vgpu)
 	vfio_device_put(vdev->vfio_device);
 }
 
-static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
+static int intel_vgpu_create(struct mdev_device *mdev)
 {
 	struct intel_vgpu *vgpu = NULL;
 	struct intel_vgpu_type *type;
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 68106be4ba7a19..06a82ec136080c 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -110,7 +110,7 @@ static struct attribute_group *mdev_type_groups[] = {
 	NULL,
 };
 
-static int vfio_ccw_mdev_create(struct kobject *kobj, struct mdev_device *mdev)
+static int vfio_ccw_mdev_create(struct mdev_device *mdev)
 {
 	struct vfio_ccw_private *private =
 		dev_get_drvdata(mdev_parent_dev(mdev));
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 41fc2e4135fe18..6d75ed07bcd49d 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -322,7 +322,7 @@ static void vfio_ap_matrix_init(struct ap_config_info *info,
 	matrix->adm_max = info->apxa ? info->Nd : 15;
 }
 
-static int vfio_ap_mdev_create(struct kobject *kobj, struct mdev_device *mdev)
+static int vfio_ap_mdev_create(struct mdev_device *mdev)
 {
 	struct ap_matrix_mdev *matrix_mdev;
 
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 3ba5e9464b4d20..71455812cb84cf 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -286,7 +286,7 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid)
 		goto mdev_fail;
 	}
 
-	ret = parent->ops->create(&type->kobj, mdev);
+	ret = parent->ops->create(mdev);
 	if (ret)
 		goto ops_create_fail;
 
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 41e91936522394..c3a800051d6146 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -61,7 +61,6 @@ unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj);
  * @create:		Called to allocate basic resources in parent device's
  *			driver for a particular mediated device. It is
  *			mandatory to provide create ops.
- *			@kobj: kobject of type for which 'create' is called.
  *			@mdev: mdev_device structure on of mediated device
  *			      that is being created
  *			Returns integer: success (0) or error (< 0)
@@ -107,7 +106,7 @@ struct mdev_parent_ops {
 	const struct attribute_group **mdev_attr_groups;
 	struct attribute_group **supported_type_groups;
 
-	int     (*create)(struct kobject *kobj, struct mdev_device *mdev);
+	int     (*create)(struct mdev_device *mdev);
 	int     (*remove)(struct mdev_device *mdev);
 	int     (*open)(struct mdev_device *mdev);
 	void    (*release)(struct mdev_device *mdev);
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
index a1af30df10a2ee..ac4d0dc2490705 100644
--- a/samples/vfio-mdev/mbochs.c
+++ b/samples/vfio-mdev/mbochs.c
@@ -506,7 +506,7 @@ static int mbochs_reset(struct mdev_device *mdev)
 	return 0;
 }
 
-static int mbochs_create(struct kobject *kobj, struct mdev_device *mdev)
+static int mbochs_create(struct mdev_device *mdev)
 {
 	const struct mbochs_type *type =
 		&mbochs_types[mdev_get_type_group_id(mdev)];
diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c
index 08c15f9f06a880..da88fd7dd42329 100644
--- a/samples/vfio-mdev/mdpy.c
+++ b/samples/vfio-mdev/mdpy.c
@@ -216,7 +216,7 @@ static int mdpy_reset(struct mdev_device *mdev)
 	return 0;
 }
 
-static int mdpy_create(struct kobject *kobj, struct mdev_device *mdev)
+static int mdpy_create(struct mdev_device *mdev)
 {
 	const struct mdpy_type *type =
 		&mdpy_types[mdev_get_type_group_id(mdev)];
diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c
index 191a587a8d5ab1..f2e36c06ac6aa2 100644
--- a/samples/vfio-mdev/mtty.c
+++ b/samples/vfio-mdev/mtty.c
@@ -708,7 +708,7 @@ static ssize_t mdev_access(struct mdev_device *mdev, u8 *buf, size_t count,
 	return ret;
 }
 
-static int mtty_create(struct kobject *kobj, struct mdev_device *mdev)
+static int mtty_create(struct mdev_device *mdev)
 {
 	struct mdev_state *mdev_state;
 	int nr_ports = mdev_get_type_group_id(mdev) + 1;
-- 
2.31.0

[PATCH 18/18] vfio/mdev: Correct the function signatures for the mdev_type_attributes

[Jason Gunthorpe]

The driver core standard is to pass in the properly typed object, the
properly typed attribute and the buffer data. It stems from the root
kobject method:

  ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,..)

Each subclass of kobject should provide their own function with the same
signature but more specific types, eg struct device uses:

  ssize_t (*show)(struct device *dev, struct device_attribute *attr,..)

In this case the existing signature is:

  ssize_t (*show)(struct kobject *kobj, struct device *dev,..)

Where kobj is a 'struct mdev_type *' and dev is 'mdev_type->parent->dev'.

Change the mdev_type related sysfs attribute functions to:

  ssize_t (*show)(struct mdev_type *mtype, struct mdev_type_attribute *attr,..)

In order to restore type safety and match the driver core standard

There are no current users of 'attr', but if it is ever needed it would be
hard to add in retroactively, so do it now.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/gpu/drm/i915/gvt/gvt.c    | 21 +++++++++++----------
 drivers/s390/cio/vfio_ccw_ops.c   | 15 +++++++++------
 drivers/s390/crypto/vfio_ap_ops.c | 12 +++++++-----
 drivers/vfio/mdev/mdev_core.c     | 14 ++++++++++++--
 drivers/vfio/mdev/mdev_sysfs.c    | 11 ++++++-----
 include/linux/mdev.h              | 11 +++++++----
 samples/vfio-mdev/mbochs.c        | 26 +++++++++++++++-----------
 samples/vfio-mdev/mdpy.c          | 24 ++++++++++++++----------
 samples/vfio-mdev/mtty.c          | 18 +++++++++---------
 9 files changed, 90 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 4b47a18e9dfa0f..3703814a669b46 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -54,14 +54,15 @@ intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id)
 	return &gvt->types[type_group_id];
 }
 
-static ssize_t available_instances_show(struct kobject *kobj,
-					struct device *dev, char *buf)
+static ssize_t available_instances_show(struct mdev_type *mtype,
+					struct mdev_type_attribute *attr,
+					char *buf)
 {
 	struct intel_vgpu_type *type;
 	unsigned int num = 0;
-	void *gvt = kdev_to_i915(dev)->gvt;
+	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
 
-	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(kobj));
+	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
 	if (!type)
 		num = 0;
 	else
@@ -70,19 +71,19 @@ static ssize_t available_instances_show(struct kobject *kobj,
 	return sprintf(buf, "%u\n", num);
 }
 
-static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
-		char *buf)
+static ssize_t device_api_show(struct mdev_type *mtype,
+			       struct mdev_type_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
 }
 
-static ssize_t description_show(struct kobject *kobj, struct device *dev,
-		char *buf)
+static ssize_t description_show(struct mdev_type *mtype,
+				struct mdev_type_attribute *attr, char *buf)
 {
 	struct intel_vgpu_type *type;
-	void *gvt = kdev_to_i915(dev)->gvt;
+	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
 
-	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(kobj));
+	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
 	if (!type)
 		return 0;
 
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 06a82ec136080c..74fe21eceb66cc 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -71,23 +71,26 @@ static int vfio_ccw_mdev_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t name_show(struct mdev_type *mtype,
+			 struct mdev_type_attribute *attr, char *buf)
 {
 	return sprintf(buf, "I/O subchannel (Non-QDIO)\n");
 }
 static MDEV_TYPE_ATTR_RO(name);
 
-static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
-			       char *buf)
+static ssize_t device_api_show(struct mdev_type *mtype,
+			       struct mdev_type_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%s\n", VFIO_DEVICE_API_CCW_STRING);
 }
 static MDEV_TYPE_ATTR_RO(device_api);
 
-static ssize_t available_instances_show(struct kobject *kobj,
-					struct device *dev, char *buf)
+static ssize_t available_instances_show(struct mdev_type *mtype,
+					struct mdev_type_attribute *attr,
+					char *buf)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(dev);
+	struct vfio_ccw_private *private =
+		dev_get_drvdata(mtype_get_parent_dev(mtype));
 
 	return sprintf(buf, "%d\n", atomic_read(&private->avail));
 }
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 6d75ed07bcd49d..cdc5edb0074690 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -366,15 +366,17 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
 	return 0;
 }
 
-static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t name_show(struct mdev_type *mtype,
+			 struct mdev_type_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
 }
 
 static MDEV_TYPE_ATTR_RO(name);
 
-static ssize_t available_instances_show(struct kobject *kobj,
-					struct device *dev, char *buf)
+static ssize_t available_instances_show(struct mdev_type *mtype,
+					struct mdev_type_attribute *attr,
+					char *buf)
 {
 	return sprintf(buf, "%d\n",
 		       atomic_read(&matrix_dev->available_instances));
@@ -382,8 +384,8 @@ static ssize_t available_instances_show(struct kobject *kobj,
 
 static MDEV_TYPE_ATTR_RO(available_instances);
 
-static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
-			       char *buf)
+static ssize_t device_api_show(struct mdev_type *mtype,
+			       struct mdev_type_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%s\n", VFIO_DEVICE_API_AP_STRING);
 }
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 71455812cb84cf..9ef1d5bed8069f 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -47,12 +47,22 @@ EXPORT_SYMBOL(mdev_get_type_group_id);
  * Used in mdev_type_attribute sysfs functions to return the index in the
  * supported_type_groups that the sysfs is called from.
  */
-unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj)
+unsigned int mtype_get_type_group_id(struct mdev_type *mtype)
 {
-	return container_of(mtype_kobj, struct mdev_type, kobj)->type_group_id;
+	return mtype->type_group_id;
 }
 EXPORT_SYMBOL(mtype_get_type_group_id);
 
+/*
+ * Used in mdev_type_attribute sysfs functions to return the parent struct
+ * device
+ */
+struct device *mtype_get_parent_dev(struct mdev_type *mtype)
+{
+	return mtype->parent->dev;
+}
+EXPORT_SYMBOL(mtype_get_parent_dev);
+
 /* Should be called holding parent_list_lock */
 static struct mdev_parent *__find_parent_device(struct device *dev)
 {
diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
index 91ecccdc2f2ec6..9b0f1a8757a0df 100644
--- a/drivers/vfio/mdev/mdev_sysfs.c
+++ b/drivers/vfio/mdev/mdev_sysfs.c
@@ -26,7 +26,7 @@ static ssize_t mdev_type_attr_show(struct kobject *kobj,
 	ssize_t ret = -EIO;
 
 	if (attr->show)
-		ret = attr->show(kobj, type->parent->dev, buf);
+		ret = attr->show(type, attr, buf);
 	return ret;
 }
 
@@ -39,7 +39,7 @@ static ssize_t mdev_type_attr_store(struct kobject *kobj,
 	ssize_t ret = -EIO;
 
 	if (attr->store)
-		ret = attr->store(&type->kobj, type->parent->dev, buf, count);
+		ret = attr->store(type, attr, buf, count);
 	return ret;
 }
 
@@ -48,8 +48,9 @@ static const struct sysfs_ops mdev_type_sysfs_ops = {
 	.store = mdev_type_attr_store,
 };
 
-static ssize_t create_store(struct kobject *kobj, struct device *dev,
-			    const char *buf, size_t count)
+static ssize_t create_store(struct mdev_type *mtype,
+			    struct mdev_type_attribute *attr, const char *buf,
+			    size_t count)
 {
 	char *str;
 	guid_t uuid;
@@ -67,7 +68,7 @@ static ssize_t create_store(struct kobject *kobj, struct device *dev,
 	if (ret)
 		return ret;
 
-	ret = mdev_device_create(to_mdev_type(kobj), &uuid);
+	ret = mdev_device_create(mtype, &uuid);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index c3a800051d6146..1fb34ea394ad46 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -47,7 +47,8 @@ static inline struct device *mdev_get_iommu_device(struct mdev_device *mdev)
 }
 
 unsigned int mdev_get_type_group_id(struct mdev_device *mdev);
-unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj);
+unsigned int mtype_get_type_group_id(struct mdev_type *mtype);
+struct device *mtype_get_parent_dev(struct mdev_type *mtype);
 
 /**
  * struct mdev_parent_ops - Structure to be registered for each parent device to
@@ -123,9 +124,11 @@ struct mdev_parent_ops {
 /* interface for exporting mdev supported type attributes */
 struct mdev_type_attribute {
 	struct attribute attr;
-	ssize_t (*show)(struct kobject *kobj, struct device *dev, char *buf);
-	ssize_t (*store)(struct kobject *kobj, struct device *dev,
-			 const char *buf, size_t count);
+	ssize_t (*show)(struct mdev_type *mtype,
+			struct mdev_type_attribute *attr, char *buf);
+	ssize_t (*store)(struct mdev_type *mtype,
+			 struct mdev_type_attribute *attr, const char *buf,
+			 size_t count);
 };
 
 #define MDEV_TYPE_ATTR(_name, _mode, _show, _store)		\
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
index ac4d0dc2490705..861c76914e7639 100644
--- a/samples/vfio-mdev/mbochs.c
+++ b/samples/vfio-mdev/mbochs.c
@@ -1330,37 +1330,41 @@ static const struct attribute_group *mdev_dev_groups[] = {
 	NULL,
 };
 
-static ssize_t
-name_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t name_show(struct mdev_type *mtype,
+			 struct mdev_type_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%s\n", kobj->name);
+	const struct mbochs_type *type =
+		&mbochs_types[mtype_get_type_group_id(mtype)];
+
+	return sprintf(buf, "%s\n", type->name);
 }
 static MDEV_TYPE_ATTR_RO(name);
 
-static ssize_t
-description_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t description_show(struct mdev_type *mtype,
+				struct mdev_type_attribute *attr, char *buf)
 {
 	const struct mbochs_type *type =
-		&mbochs_types[mtype_get_type_group_id(kobj)];
+		&mbochs_types[mtype_get_type_group_id(mtype)];
 
 	return sprintf(buf, "virtual display, %d MB video memory\n",
 		       type ? type->mbytes  : 0);
 }
 static MDEV_TYPE_ATTR_RO(description);
 
-static ssize_t
-available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t available_instances_show(struct mdev_type *mtype,
+					struct mdev_type_attribute *attr,
+					char *buf)
 {
 	const struct mbochs_type *type =
-		&mbochs_types[mtype_get_type_group_id(kobj)];
+		&mbochs_types[mtype_get_type_group_id(mtype)];
 	int count = (max_mbytes - mbochs_used_mbytes) / type->mbytes;
 
 	return sprintf(buf, "%d\n", count);
 }
 static MDEV_TYPE_ATTR_RO(available_instances);
 
-static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
-			       char *buf)
+static ssize_t device_api_show(struct mdev_type *mtype,
+			       struct mdev_type_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
 }
diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c
index da88fd7dd42329..885b88ea20e234 100644
--- a/samples/vfio-mdev/mdpy.c
+++ b/samples/vfio-mdev/mdpy.c
@@ -652,18 +652,21 @@ static const struct attribute_group *mdev_dev_groups[] = {
 	NULL,
 };
 
-static ssize_t
-name_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t name_show(struct mdev_type *mtype,
+			 struct mdev_type_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%s\n", kobj->name);
+	const struct mdpy_type *type =
+		&mdpy_types[mtype_get_type_group_id(mtype)];
+
+	return sprintf(buf, "%s\n", type->name);
 }
 static MDEV_TYPE_ATTR_RO(name);
 
-static ssize_t
-description_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t description_show(struct mdev_type *mtype,
+				struct mdev_type_attribute *attr, char *buf)
 {
 	const struct mdpy_type *type =
-		&mdpy_types[mtype_get_type_group_id(kobj)];
+		&mdpy_types[mtype_get_type_group_id(mtype)];
 
 	return sprintf(buf, "virtual display, %dx%d framebuffer\n",
 		       type ? type->width  : 0,
@@ -671,15 +674,16 @@ description_show(struct kobject *kobj, struct device *dev, char *buf)
 }
 static MDEV_TYPE_ATTR_RO(description);
 
-static ssize_t
-available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t available_instances_show(struct mdev_type *mtype,
+					struct mdev_type_attribute *attr,
+					char *buf)
 {
 	return sprintf(buf, "%d\n", max_devices - mdpy_count);
 }
 static MDEV_TYPE_ATTR_RO(available_instances);
 
-static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
-			       char *buf)
+static ssize_t device_api_show(struct mdev_type *mtype,
+			       struct mdev_type_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
 }
diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c
index f2e36c06ac6aa2..b9b24be4abdab7 100644
--- a/samples/vfio-mdev/mtty.c
+++ b/samples/vfio-mdev/mtty.c
@@ -1292,23 +1292,24 @@ static const struct attribute_group *mdev_dev_groups[] = {
 	NULL,
 };
 
-static ssize_t
-name_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t name_show(struct mdev_type *mtype,
+			 struct mdev_type_attribute *attr, char *buf)
 {
 	static const char *name_str[2] = { "Single port serial",
 					   "Dual port serial" };
 
 	return sysfs_emit(buf, "%s\n",
-			  name_str[mtype_get_type_group_id(kobj)]);
+			  name_str[mtype_get_type_group_id(mtype)]);
 }
 
 static MDEV_TYPE_ATTR_RO(name);
 
-static ssize_t
-available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
+static ssize_t available_instances_show(struct mdev_type *mtype,
+					struct mdev_type_attribute *attr,
+					char *buf)
 {
 	struct mdev_state *mds;
-	unsigned int ports = mtype_get_type_group_id(kobj) + 1;
+	unsigned int ports = mtype_get_type_group_id(mtype) + 1;
 	int used = 0;
 
 	list_for_each_entry(mds, &mdev_devices_list, next)
@@ -1319,9 +1320,8 @@ available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
 
 static MDEV_TYPE_ATTR_RO(available_instances);
 
-
-static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
-			       char *buf)
+static ssize_t device_api_show(struct mdev_type *mtype,
+			       struct mdev_type_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
 }
-- 
2.31.0

Re: [PATCH 17/18] vfio/mdev: Remove kobj from mdev_parent_ops->create()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 18/18] vfio/mdev: Correct the function signatures for the mdev_type_attributes

[Christoph Hellwig]

On Tue, Mar 23, 2021 at 02:55:35PM -0300, Jason Gunthorpe wrote:
> The driver core standard is to pass in the properly typed object, the
> properly typed attribute and the buffer data. It stems from the root
> kobject method:
> 
>   ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,..)
> 
> Each subclass of kobject should provide their own function with the same
> signature but more specific types, eg struct device uses:
> 
>   ssize_t (*show)(struct device *dev, struct device_attribute *attr,..)
> 
> In this case the existing signature is:
> 
>   ssize_t (*show)(struct kobject *kobj, struct device *dev,..)
> 
> Where kobj is a 'struct mdev_type *' and dev is 'mdev_type->parent->dev'.
> 
> Change the mdev_type related sysfs attribute functions to:
> 
>   ssize_t (*show)(struct mdev_type *mtype, struct mdev_type_attribute *attr,..)
> 
> In order to restore type safety and match the driver core standard
> 
> There are no current users of 'attr', but if it is ever needed it would be
> hard to add in retroactively, so do it now.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/gvt.c    | 21 +++++++++++----------
>  drivers/s390/cio/vfio_ccw_ops.c   | 15 +++++++++------
>  drivers/s390/crypto/vfio_ap_ops.c | 12 +++++++-----
>  drivers/vfio/mdev/mdev_core.c     | 14 ++++++++++++--
>  drivers/vfio/mdev/mdev_sysfs.c    | 11 ++++++-----
>  include/linux/mdev.h              | 11 +++++++----
>  samples/vfio-mdev/mbochs.c        | 26 +++++++++++++++-----------
>  samples/vfio-mdev/mdpy.c          | 24 ++++++++++++++----------
>  samples/vfio-mdev/mtty.c          | 18 +++++++++---------
>  9 files changed, 90 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 4b47a18e9dfa0f..3703814a669b46 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -54,14 +54,15 @@ intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id)
>  	return &gvt->types[type_group_id];
>  }
>  
> -static ssize_t available_instances_show(struct kobject *kobj,
> -					struct device *dev, char *buf)
> +static ssize_t available_instances_show(struct mdev_type *mtype,
> +					struct mdev_type_attribute *attr,
> +					char *buf)
>  {
>  	struct intel_vgpu_type *type;
>  	unsigned int num = 0;
> -	void *gvt = kdev_to_i915(dev)->gvt;
> +	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
>  
> -	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(kobj));
> +	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));

Somewhere in this series you should probably
switch intel_gvt_find_vgpu_type to only get the mtype, as it can trivially
deduct the gvt from it (which also seems to have lost its type somewhere..)

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

RE: [PATCH 17/18] vfio/mdev: Remove kobj from mdev_parent_ops->create()

[Tian, Kevin]

> From: Jason Gunthorpe
> Sent: Wednesday, March 24, 2021 1:56 AM
> 
> The kobj here is a type-erased version of mdev_type, which is already
> stored in the struct mdev_device being passed in. It was only ever used to
> compute the type_group_id, which is now extracted directly from the mdev.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 2 +-
>  drivers/s390/cio/vfio_ccw_ops.c   | 2 +-
>  drivers/s390/crypto/vfio_ap_ops.c | 2 +-
>  drivers/vfio/mdev/mdev_core.c     | 2 +-
>  include/linux/mdev.h              | 3 +--
>  samples/vfio-mdev/mbochs.c        | 2 +-
>  samples/vfio-mdev/mdpy.c          | 2 +-
>  samples/vfio-mdev/mtty.c          | 2 +-
>  8 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 16e1e4a38aa1f6..6bf176e8426e63 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -689,7 +689,7 @@ static void kvmgt_put_vfio_device(void *vgpu)
>  	vfio_device_put(vdev->vfio_device);
>  }
> 
> -static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
> +static int intel_vgpu_create(struct mdev_device *mdev)
>  {
>  	struct intel_vgpu *vgpu = NULL;
>  	struct intel_vgpu_type *type;
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c
> b/drivers/s390/cio/vfio_ccw_ops.c
> index 68106be4ba7a19..06a82ec136080c 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -110,7 +110,7 @@ static struct attribute_group *mdev_type_groups[] =
> {
>  	NULL,
>  };
> 
> -static int vfio_ccw_mdev_create(struct kobject *kobj, struct mdev_device
> *mdev)
> +static int vfio_ccw_mdev_create(struct mdev_device *mdev)
>  {
>  	struct vfio_ccw_private *private =
>  		dev_get_drvdata(mdev_parent_dev(mdev));
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
> b/drivers/s390/crypto/vfio_ap_ops.c
> index 41fc2e4135fe18..6d75ed07bcd49d 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -322,7 +322,7 @@ static void vfio_ap_matrix_init(struct ap_config_info
> *info,
>  	matrix->adm_max = info->apxa ? info->Nd : 15;
>  }
> 
> -static int vfio_ap_mdev_create(struct kobject *kobj, struct mdev_device
> *mdev)
> +static int vfio_ap_mdev_create(struct mdev_device *mdev)
>  {
>  	struct ap_matrix_mdev *matrix_mdev;
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c
> b/drivers/vfio/mdev/mdev_core.c
> index 3ba5e9464b4d20..71455812cb84cf 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -286,7 +286,7 @@ int mdev_device_create(struct mdev_type *type,
> const guid_t *uuid)
>  		goto mdev_fail;
>  	}
> 
> -	ret = parent->ops->create(&type->kobj, mdev);
> +	ret = parent->ops->create(mdev);
>  	if (ret)
>  		goto ops_create_fail;
> 
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index 41e91936522394..c3a800051d6146 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -61,7 +61,6 @@ unsigned int mtype_get_type_group_id(struct kobject
> *mtype_kobj);
>   * @create:		Called to allocate basic resources in parent device's
>   *			driver for a particular mediated device. It is
>   *			mandatory to provide create ops.
> - *			@kobj: kobject of type for which 'create' is called.
>   *			@mdev: mdev_device structure on of mediated
> device
>   *			      that is being created
>   *			Returns integer: success (0) or error (< 0)
> @@ -107,7 +106,7 @@ struct mdev_parent_ops {
>  	const struct attribute_group **mdev_attr_groups;
>  	struct attribute_group **supported_type_groups;
> 
> -	int     (*create)(struct kobject *kobj, struct mdev_device *mdev);
> +	int     (*create)(struct mdev_device *mdev);
>  	int     (*remove)(struct mdev_device *mdev);
>  	int     (*open)(struct mdev_device *mdev);
>  	void    (*release)(struct mdev_device *mdev);
> diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
> index a1af30df10a2ee..ac4d0dc2490705 100644
> --- a/samples/vfio-mdev/mbochs.c
> +++ b/samples/vfio-mdev/mbochs.c
> @@ -506,7 +506,7 @@ static int mbochs_reset(struct mdev_device *mdev)
>  	return 0;
>  }
> 
> -static int mbochs_create(struct kobject *kobj, struct mdev_device *mdev)
> +static int mbochs_create(struct mdev_device *mdev)
>  {
>  	const struct mbochs_type *type =
>  		&mbochs_types[mdev_get_type_group_id(mdev)];
> diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c
> index 08c15f9f06a880..da88fd7dd42329 100644
> --- a/samples/vfio-mdev/mdpy.c
> +++ b/samples/vfio-mdev/mdpy.c
> @@ -216,7 +216,7 @@ static int mdpy_reset(struct mdev_device *mdev)
>  	return 0;
>  }
> 
> -static int mdpy_create(struct kobject *kobj, struct mdev_device *mdev)
> +static int mdpy_create(struct mdev_device *mdev)
>  {
>  	const struct mdpy_type *type =
>  		&mdpy_types[mdev_get_type_group_id(mdev)];
> diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c
> index 191a587a8d5ab1..f2e36c06ac6aa2 100644
> --- a/samples/vfio-mdev/mtty.c
> +++ b/samples/vfio-mdev/mtty.c
> @@ -708,7 +708,7 @@ static ssize_t mdev_access(struct mdev_device
> *mdev, u8 *buf, size_t count,
>  	return ret;
>  }
> 
> -static int mtty_create(struct kobject *kobj, struct mdev_device *mdev)
> +static int mtty_create(struct mdev_device *mdev)
>  {
>  	struct mdev_state *mdev_state;
>  	int nr_ports = mdev_get_type_group_id(mdev) + 1;
> --
> 2.31.0
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

RE: [PATCH 18/18] vfio/mdev: Correct the function signatures for the mdev_type_attributes

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, March 24, 2021 1:56 AM
> 
> The driver core standard is to pass in the properly typed object, the
> properly typed attribute and the buffer data. It stems from the root
> kobject method:
> 
>   ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,..)
> 
> Each subclass of kobject should provide their own function with the same
> signature but more specific types, eg struct device uses:
> 
>   ssize_t (*show)(struct device *dev, struct device_attribute *attr,..)
> 
> In this case the existing signature is:
> 
>   ssize_t (*show)(struct kobject *kobj, struct device *dev,..)
> 
> Where kobj is a 'struct mdev_type *' and dev is 'mdev_type->parent->dev'.
> 
> Change the mdev_type related sysfs attribute functions to:
> 
>   ssize_t (*show)(struct mdev_type *mtype, struct mdev_type_attribute
> *attr,..)
> 
> In order to restore type safety and match the driver core standard
> 
> There are no current users of 'attr', but if it is ever needed it would be
> hard to add in retroactively, so do it now.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> ---
>  drivers/gpu/drm/i915/gvt/gvt.c    | 21 +++++++++++----------
>  drivers/s390/cio/vfio_ccw_ops.c   | 15 +++++++++------
>  drivers/s390/crypto/vfio_ap_ops.c | 12 +++++++-----
>  drivers/vfio/mdev/mdev_core.c     | 14 ++++++++++++--
>  drivers/vfio/mdev/mdev_sysfs.c    | 11 ++++++-----
>  include/linux/mdev.h              | 11 +++++++----
>  samples/vfio-mdev/mbochs.c        | 26 +++++++++++++++-----------
>  samples/vfio-mdev/mdpy.c          | 24 ++++++++++++++----------
>  samples/vfio-mdev/mtty.c          | 18 +++++++++---------
>  9 files changed, 90 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 4b47a18e9dfa0f..3703814a669b46 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -54,14 +54,15 @@ intel_gvt_find_vgpu_type(struct intel_gvt *gvt,
> unsigned int type_group_id)
>  	return &gvt->types[type_group_id];
>  }
> 
> -static ssize_t available_instances_show(struct kobject *kobj,
> -					struct device *dev, char *buf)
> +static ssize_t available_instances_show(struct mdev_type *mtype,
> +					struct mdev_type_attribute *attr,
> +					char *buf)
>  {
>  	struct intel_vgpu_type *type;
>  	unsigned int num = 0;
> -	void *gvt = kdev_to_i915(dev)->gvt;
> +	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
> 
> -	type = intel_gvt_find_vgpu_type(gvt,
> mtype_get_type_group_id(kobj));
> +	type = intel_gvt_find_vgpu_type(gvt,
> mtype_get_type_group_id(mtype));
>  	if (!type)
>  		num = 0;
>  	else
> @@ -70,19 +71,19 @@ static ssize_t available_instances_show(struct kobject
> *kobj,
>  	return sprintf(buf, "%u\n", num);
>  }
> 
> -static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
> -		char *buf)
> +static ssize_t device_api_show(struct mdev_type *mtype,
> +			       struct mdev_type_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
>  }
> 
> -static ssize_t description_show(struct kobject *kobj, struct device *dev,
> -		char *buf)
> +static ssize_t description_show(struct mdev_type *mtype,
> +				struct mdev_type_attribute *attr, char *buf)
>  {
>  	struct intel_vgpu_type *type;
> -	void *gvt = kdev_to_i915(dev)->gvt;
> +	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
> 
> -	type = intel_gvt_find_vgpu_type(gvt,
> mtype_get_type_group_id(kobj));
> +	type = intel_gvt_find_vgpu_type(gvt,
> mtype_get_type_group_id(mtype));
>  	if (!type)
>  		return 0;
> 
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c
> b/drivers/s390/cio/vfio_ccw_ops.c
> index 06a82ec136080c..74fe21eceb66cc 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -71,23 +71,26 @@ static int vfio_ccw_mdev_notifier(struct
> notifier_block *nb,
>  	return NOTIFY_DONE;
>  }
> 
> -static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t name_show(struct mdev_type *mtype,
> +			 struct mdev_type_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "I/O subchannel (Non-QDIO)\n");
>  }
>  static MDEV_TYPE_ATTR_RO(name);
> 
> -static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
> -			       char *buf)
> +static ssize_t device_api_show(struct mdev_type *mtype,
> +			       struct mdev_type_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "%s\n", VFIO_DEVICE_API_CCW_STRING);
>  }
>  static MDEV_TYPE_ATTR_RO(device_api);
> 
> -static ssize_t available_instances_show(struct kobject *kobj,
> -					struct device *dev, char *buf)
> +static ssize_t available_instances_show(struct mdev_type *mtype,
> +					struct mdev_type_attribute *attr,
> +					char *buf)
>  {
> -	struct vfio_ccw_private *private = dev_get_drvdata(dev);
> +	struct vfio_ccw_private *private =
> +		dev_get_drvdata(mtype_get_parent_dev(mtype));
> 
>  	return sprintf(buf, "%d\n", atomic_read(&private->avail));
>  }
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
> b/drivers/s390/crypto/vfio_ap_ops.c
> index 6d75ed07bcd49d..cdc5edb0074690 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -366,15 +366,17 @@ static int vfio_ap_mdev_remove(struct
> mdev_device *mdev)
>  	return 0;
>  }
> 
> -static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t name_show(struct mdev_type *mtype,
> +			 struct mdev_type_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
>  }
> 
>  static MDEV_TYPE_ATTR_RO(name);
> 
> -static ssize_t available_instances_show(struct kobject *kobj,
> -					struct device *dev, char *buf)
> +static ssize_t available_instances_show(struct mdev_type *mtype,
> +					struct mdev_type_attribute *attr,
> +					char *buf)
>  {
>  	return sprintf(buf, "%d\n",
>  		       atomic_read(&matrix_dev->available_instances));
> @@ -382,8 +384,8 @@ static ssize_t available_instances_show(struct kobject
> *kobj,
> 
>  static MDEV_TYPE_ATTR_RO(available_instances);
> 
> -static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
> -			       char *buf)
> +static ssize_t device_api_show(struct mdev_type *mtype,
> +			       struct mdev_type_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "%s\n", VFIO_DEVICE_API_AP_STRING);
>  }
> diff --git a/drivers/vfio/mdev/mdev_core.c
> b/drivers/vfio/mdev/mdev_core.c
> index 71455812cb84cf..9ef1d5bed8069f 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -47,12 +47,22 @@ EXPORT_SYMBOL(mdev_get_type_group_id);
>   * Used in mdev_type_attribute sysfs functions to return the index in the
>   * supported_type_groups that the sysfs is called from.
>   */
> -unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj)
> +unsigned int mtype_get_type_group_id(struct mdev_type *mtype)
>  {
> -	return container_of(mtype_kobj, struct mdev_type, kobj)-
> >type_group_id;
> +	return mtype->type_group_id;
>  }
>  EXPORT_SYMBOL(mtype_get_type_group_id);
> 
> +/*
> + * Used in mdev_type_attribute sysfs functions to return the parent struct
> + * device
> + */
> +struct device *mtype_get_parent_dev(struct mdev_type *mtype)
> +{
> +	return mtype->parent->dev;
> +}
> +EXPORT_SYMBOL(mtype_get_parent_dev);
> +
>  /* Should be called holding parent_list_lock */
>  static struct mdev_parent *__find_parent_device(struct device *dev)
>  {
> diff --git a/drivers/vfio/mdev/mdev_sysfs.c
> b/drivers/vfio/mdev/mdev_sysfs.c
> index 91ecccdc2f2ec6..9b0f1a8757a0df 100644
> --- a/drivers/vfio/mdev/mdev_sysfs.c
> +++ b/drivers/vfio/mdev/mdev_sysfs.c
> @@ -26,7 +26,7 @@ static ssize_t mdev_type_attr_show(struct kobject
> *kobj,
>  	ssize_t ret = -EIO;
> 
>  	if (attr->show)
> -		ret = attr->show(kobj, type->parent->dev, buf);
> +		ret = attr->show(type, attr, buf);
>  	return ret;
>  }
> 
> @@ -39,7 +39,7 @@ static ssize_t mdev_type_attr_store(struct kobject *kobj,
>  	ssize_t ret = -EIO;
> 
>  	if (attr->store)
> -		ret = attr->store(&type->kobj, type->parent->dev, buf, count);
> +		ret = attr->store(type, attr, buf, count);
>  	return ret;
>  }
> 
> @@ -48,8 +48,9 @@ static const struct sysfs_ops mdev_type_sysfs_ops = {
>  	.store = mdev_type_attr_store,
>  };
> 
> -static ssize_t create_store(struct kobject *kobj, struct device *dev,
> -			    const char *buf, size_t count)
> +static ssize_t create_store(struct mdev_type *mtype,
> +			    struct mdev_type_attribute *attr, const char *buf,
> +			    size_t count)
>  {
>  	char *str;
>  	guid_t uuid;
> @@ -67,7 +68,7 @@ static ssize_t create_store(struct kobject *kobj, struct
> device *dev,
>  	if (ret)
>  		return ret;
> 
> -	ret = mdev_device_create(to_mdev_type(kobj), &uuid);
> +	ret = mdev_device_create(mtype, &uuid);
>  	if (ret)
>  		return ret;
> 
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index c3a800051d6146..1fb34ea394ad46 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -47,7 +47,8 @@ static inline struct device
> *mdev_get_iommu_device(struct mdev_device *mdev)
>  }
> 
>  unsigned int mdev_get_type_group_id(struct mdev_device *mdev);
> -unsigned int mtype_get_type_group_id(struct kobject *mtype_kobj);
> +unsigned int mtype_get_type_group_id(struct mdev_type *mtype);
> +struct device *mtype_get_parent_dev(struct mdev_type *mtype);
> 
>  /**
>   * struct mdev_parent_ops - Structure to be registered for each parent
> device to
> @@ -123,9 +124,11 @@ struct mdev_parent_ops {
>  /* interface for exporting mdev supported type attributes */
>  struct mdev_type_attribute {
>  	struct attribute attr;
> -	ssize_t (*show)(struct kobject *kobj, struct device *dev, char *buf);
> -	ssize_t (*store)(struct kobject *kobj, struct device *dev,
> -			 const char *buf, size_t count);
> +	ssize_t (*show)(struct mdev_type *mtype,
> +			struct mdev_type_attribute *attr, char *buf);
> +	ssize_t (*store)(struct mdev_type *mtype,
> +			 struct mdev_type_attribute *attr, const char *buf,
> +			 size_t count);
>  };
> 
>  #define MDEV_TYPE_ATTR(_name, _mode, _show, _store)		\
> diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
> index ac4d0dc2490705..861c76914e7639 100644
> --- a/samples/vfio-mdev/mbochs.c
> +++ b/samples/vfio-mdev/mbochs.c
> @@ -1330,37 +1330,41 @@ static const struct attribute_group
> *mdev_dev_groups[] = {
>  	NULL,
>  };
> 
> -static ssize_t
> -name_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t name_show(struct mdev_type *mtype,
> +			 struct mdev_type_attribute *attr, char *buf)
>  {
> -	return sprintf(buf, "%s\n", kobj->name);
> +	const struct mbochs_type *type =
> +		&mbochs_types[mtype_get_type_group_id(mtype)];
> +
> +	return sprintf(buf, "%s\n", type->name);
>  }
>  static MDEV_TYPE_ATTR_RO(name);
> 
> -static ssize_t
> -description_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t description_show(struct mdev_type *mtype,
> +				struct mdev_type_attribute *attr, char *buf)
>  {
>  	const struct mbochs_type *type =
> -		&mbochs_types[mtype_get_type_group_id(kobj)];
> +		&mbochs_types[mtype_get_type_group_id(mtype)];
> 
>  	return sprintf(buf, "virtual display, %d MB video memory\n",
>  		       type ? type->mbytes  : 0);
>  }
>  static MDEV_TYPE_ATTR_RO(description);
> 
> -static ssize_t
> -available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t available_instances_show(struct mdev_type *mtype,
> +					struct mdev_type_attribute *attr,
> +					char *buf)
>  {
>  	const struct mbochs_type *type =
> -		&mbochs_types[mtype_get_type_group_id(kobj)];
> +		&mbochs_types[mtype_get_type_group_id(mtype)];
>  	int count = (max_mbytes - mbochs_used_mbytes) / type->mbytes;
> 
>  	return sprintf(buf, "%d\n", count);
>  }
>  static MDEV_TYPE_ATTR_RO(available_instances);
> 
> -static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
> -			       char *buf)
> +static ssize_t device_api_show(struct mdev_type *mtype,
> +			       struct mdev_type_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
>  }
> diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c
> index da88fd7dd42329..885b88ea20e234 100644
> --- a/samples/vfio-mdev/mdpy.c
> +++ b/samples/vfio-mdev/mdpy.c
> @@ -652,18 +652,21 @@ static const struct attribute_group
> *mdev_dev_groups[] = {
>  	NULL,
>  };
> 
> -static ssize_t
> -name_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t name_show(struct mdev_type *mtype,
> +			 struct mdev_type_attribute *attr, char *buf)
>  {
> -	return sprintf(buf, "%s\n", kobj->name);
> +	const struct mdpy_type *type =
> +		&mdpy_types[mtype_get_type_group_id(mtype)];
> +
> +	return sprintf(buf, "%s\n", type->name);
>  }
>  static MDEV_TYPE_ATTR_RO(name);
> 
> -static ssize_t
> -description_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t description_show(struct mdev_type *mtype,
> +				struct mdev_type_attribute *attr, char *buf)
>  {
>  	const struct mdpy_type *type =
> -		&mdpy_types[mtype_get_type_group_id(kobj)];
> +		&mdpy_types[mtype_get_type_group_id(mtype)];
> 
>  	return sprintf(buf, "virtual display, %dx%d framebuffer\n",
>  		       type ? type->width  : 0,
> @@ -671,15 +674,16 @@ description_show(struct kobject *kobj, struct
> device *dev, char *buf)
>  }
>  static MDEV_TYPE_ATTR_RO(description);
> 
> -static ssize_t
> -available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t available_instances_show(struct mdev_type *mtype,
> +					struct mdev_type_attribute *attr,
> +					char *buf)
>  {
>  	return sprintf(buf, "%d\n", max_devices - mdpy_count);
>  }
>  static MDEV_TYPE_ATTR_RO(available_instances);
> 
> -static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
> -			       char *buf)
> +static ssize_t device_api_show(struct mdev_type *mtype,
> +			       struct mdev_type_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
>  }
> diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c
> index f2e36c06ac6aa2..b9b24be4abdab7 100644
> --- a/samples/vfio-mdev/mtty.c
> +++ b/samples/vfio-mdev/mtty.c
> @@ -1292,23 +1292,24 @@ static const struct attribute_group
> *mdev_dev_groups[] = {
>  	NULL,
>  };
> 
> -static ssize_t
> -name_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t name_show(struct mdev_type *mtype,
> +			 struct mdev_type_attribute *attr, char *buf)
>  {
>  	static const char *name_str[2] = { "Single port serial",
>  					   "Dual port serial" };
> 
>  	return sysfs_emit(buf, "%s\n",
> -			  name_str[mtype_get_type_group_id(kobj)]);
> +			  name_str[mtype_get_type_group_id(mtype)]);
>  }
> 
>  static MDEV_TYPE_ATTR_RO(name);
> 
> -static ssize_t
> -available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
> +static ssize_t available_instances_show(struct mdev_type *mtype,
> +					struct mdev_type_attribute *attr,
> +					char *buf)
>  {
>  	struct mdev_state *mds;
> -	unsigned int ports = mtype_get_type_group_id(kobj) + 1;
> +	unsigned int ports = mtype_get_type_group_id(mtype) + 1;
>  	int used = 0;
> 
>  	list_for_each_entry(mds, &mdev_devices_list, next)
> @@ -1319,9 +1320,8 @@ available_instances_show(struct kobject *kobj,
> struct device *dev, char *buf)
> 
>  static MDEV_TYPE_ATTR_RO(available_instances);
> 
> -
> -static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
> -			       char *buf)
> +static ssize_t device_api_show(struct mdev_type *mtype,
> +			       struct mdev_type_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
>  }
> --
> 2.31.0

Re: [PATCH 17/18] vfio/mdev: Remove kobj from mdev_parent_ops->create()

[Cornelia Huck]

On Tue, 23 Mar 2021 14:55:34 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> The kobj here is a type-erased version of mdev_type, which is already
> stored in the struct mdev_device being passed in. It was only ever used to
> compute the type_group_id, which is now extracted directly from the mdev.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 2 +-
>  drivers/s390/cio/vfio_ccw_ops.c   | 2 +-
>  drivers/s390/crypto/vfio_ap_ops.c | 2 +-
>  drivers/vfio/mdev/mdev_core.c     | 2 +-
>  include/linux/mdev.h              | 3 +--
>  samples/vfio-mdev/mbochs.c        | 2 +-
>  samples/vfio-mdev/mdpy.c          | 2 +-
>  samples/vfio-mdev/mtty.c          | 2 +-
>  8 files changed, 8 insertions(+), 9 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 18/18] vfio/mdev: Correct the function signatures for the mdev_type_attributes

[Cornelia Huck]

On Tue, 23 Mar 2021 14:55:35 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> The driver core standard is to pass in the properly typed object, the
> properly typed attribute and the buffer data. It stems from the root
> kobject method:
> 
>   ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,..)
> 
> Each subclass of kobject should provide their own function with the same
> signature but more specific types, eg struct device uses:
> 
>   ssize_t (*show)(struct device *dev, struct device_attribute *attr,..)
> 
> In this case the existing signature is:
> 
>   ssize_t (*show)(struct kobject *kobj, struct device *dev,..)
> 
> Where kobj is a 'struct mdev_type *' and dev is 'mdev_type->parent->dev'.
> 
> Change the mdev_type related sysfs attribute functions to:
> 
>   ssize_t (*show)(struct mdev_type *mtype, struct mdev_type_attribute *attr,..)
> 
> In order to restore type safety and match the driver core standard
> 
> There are no current users of 'attr', but if it is ever needed it would be
> hard to add in retroactively, so do it now.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/gvt.c    | 21 +++++++++++----------
>  drivers/s390/cio/vfio_ccw_ops.c   | 15 +++++++++------
>  drivers/s390/crypto/vfio_ap_ops.c | 12 +++++++-----
>  drivers/vfio/mdev/mdev_core.c     | 14 ++++++++++++--
>  drivers/vfio/mdev/mdev_sysfs.c    | 11 ++++++-----
>  include/linux/mdev.h              | 11 +++++++----
>  samples/vfio-mdev/mbochs.c        | 26 +++++++++++++++-----------
>  samples/vfio-mdev/mdpy.c          | 24 ++++++++++++++----------
>  samples/vfio-mdev/mtty.c          | 18 +++++++++---------
>  9 files changed, 90 insertions(+), 62 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 18/18] vfio/mdev: Correct the function signatures for the mdev_type_attributes

[Jason Gunthorpe]

On Tue, Mar 23, 2021 at 08:31:03PM +0100, Christoph Hellwig wrote:

> > -	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(kobj));
> > +	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
> 
> Somewhere in this series you should probably switch
> intel_gvt_find_vgpu_type to only get the mtype, as it can trivially
> deduct the gvt from it (which also seems to have lost its type
> somewhere..)

I look at just this minor change for a bit and it just is a mess.

This only exists like this because the gvt_type_attrs[] are in the
wrong file and I already tried to fix that and gave up.

Deleting the intel_gvt_ops looks like precondition to do any big
improvement in here :\

Jason