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

Tian, Kevin
2021-03-26T07:04:52Z

> 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