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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> 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@xxxxxxxxxx>

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

> ---
>  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





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux