From 6c7f98b334a32df5cac8abac8983ac4ce17cab57 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Mon, 11 Apr 2022 16:13:59 +0200 Subject: vfio/mdev: Remove vfio_mdev.c Now that all mdev drivers directly create their own mdev_device driver and directly register with the vfio core's vfio_device_ops this is all dead code. Delete vfio_mdev.c and the mdev_parent_ops members that are connected to it. Signed-off-by: Jason Gunthorpe Signed-off-by: Christoph Hellwig Signed-off-by: Zhi Wang Link: http://patchwork.freedesktop.org/patch/msgid/20220411141403.86980-31-hch@lst.de Reviewed-by: Kirti Wankhede Reviewed-by: Zhi Wang --- drivers/vfio/mdev/Makefile | 2 +- drivers/vfio/mdev/mdev_core.c | 40 ++--------- drivers/vfio/mdev/mdev_driver.c | 10 --- drivers/vfio/mdev/mdev_private.h | 2 - drivers/vfio/mdev/vfio_mdev.c | 152 --------------------------------------- 5 files changed, 5 insertions(+), 201 deletions(-) delete mode 100644 drivers/vfio/mdev/vfio_mdev.c (limited to 'drivers/vfio') diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile index ff9ecd802125..7c236ba1b90e 100644 --- a/drivers/vfio/mdev/Makefile +++ b/drivers/vfio/mdev/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o vfio_mdev.o +mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o obj-$(CONFIG_VFIO_MDEV) += mdev.o diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index b314101237fe..8b1e86b9e8bc 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -89,17 +89,10 @@ void mdev_release_parent(struct kref *kref) static void mdev_device_remove_common(struct mdev_device *mdev) { struct mdev_parent *parent = mdev->type->parent; - int ret; mdev_remove_sysfs_files(mdev); device_del(&mdev->dev); lockdep_assert_held(&parent->unreg_sem); - if (parent->ops->remove) { - ret = parent->ops->remove(mdev); - if (ret) - dev_err(&mdev->dev, "Remove failed: err=%d\n", ret); - } - /* Balances with device_initialize() */ put_device(&mdev->dev); } @@ -131,7 +124,7 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops) /* check for mandatory ops */ if (!ops || !ops->supported_type_groups) return -EINVAL; - if (!ops->device_driver && (!ops->create || !ops->remove)) + if (!ops->device_driver) return -EINVAL; dev = get_device(dev); @@ -297,18 +290,10 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) goto out_put_device; } - if (parent->ops->create) { - ret = parent->ops->create(mdev); - if (ret) - goto out_unlock; - } - ret = device_add(&mdev->dev); if (ret) - goto out_remove; + goto out_unlock; - if (!drv) - drv = &vfio_mdev_driver; ret = device_driver_attach(&drv->driver, &mdev->dev); if (ret) goto out_del; @@ -325,9 +310,6 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) out_del: device_del(&mdev->dev); -out_remove: - if (parent->ops->remove) - parent->ops->remove(mdev); out_unlock: up_read(&parent->unreg_sem); out_put_device: @@ -370,28 +352,14 @@ int mdev_device_remove(struct mdev_device *mdev) static int __init mdev_init(void) { - int rc; - - rc = mdev_bus_register(); - if (rc) - return rc; - rc = mdev_register_driver(&vfio_mdev_driver); - if (rc) - goto err_bus; - return 0; -err_bus: - mdev_bus_unregister(); - return rc; + return bus_register(&mdev_bus_type); } static void __exit mdev_exit(void) { - mdev_unregister_driver(&vfio_mdev_driver); - if (mdev_bus_compat_class) class_compat_unregister(mdev_bus_compat_class); - - mdev_bus_unregister(); + bus_unregister(&mdev_bus_type); } subsys_initcall(mdev_init) diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c index 7927ed4f1711..9c2af59809e2 100644 --- a/drivers/vfio/mdev/mdev_driver.c +++ b/drivers/vfio/mdev/mdev_driver.c @@ -74,13 +74,3 @@ void mdev_unregister_driver(struct mdev_driver *drv) driver_unregister(&drv->driver); } EXPORT_SYMBOL(mdev_unregister_driver); - -int mdev_bus_register(void) -{ - return bus_register(&mdev_bus_type); -} - -void mdev_bus_unregister(void) -{ - bus_unregister(&mdev_bus_type); -} diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index afbad7b0a14a..6999c89db7b1 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -37,8 +37,6 @@ struct mdev_type { #define to_mdev_type(_kobj) \ container_of(_kobj, struct mdev_type, kobj) -extern struct mdev_driver vfio_mdev_driver; - int parent_create_sysfs_files(struct mdev_parent *parent); void parent_remove_sysfs_files(struct mdev_parent *parent); diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c deleted file mode 100644 index a90e24b0c851..000000000000 --- a/drivers/vfio/mdev/vfio_mdev.c +++ /dev/null @@ -1,152 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * VFIO based driver for Mediated device - * - * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. - * Author: Neo Jia - * Kirti Wankhede - */ - -#include -#include -#include -#include -#include -#include -#include - -#include "mdev_private.h" - -static int vfio_mdev_open_device(struct vfio_device *core_vdev) -{ - struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->type->parent; - - if (unlikely(!parent->ops->open_device)) - return 0; - - return parent->ops->open_device(mdev); -} - -static void vfio_mdev_close_device(struct vfio_device *core_vdev) -{ - struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->type->parent; - - if (likely(parent->ops->close_device)) - parent->ops->close_device(mdev); -} - -static long vfio_mdev_unlocked_ioctl(struct vfio_device *core_vdev, - unsigned int cmd, unsigned long arg) -{ - struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->type->parent; - - if (unlikely(!parent->ops->ioctl)) - return 0; - - return parent->ops->ioctl(mdev, cmd, arg); -} - -static ssize_t vfio_mdev_read(struct vfio_device *core_vdev, char __user *buf, - size_t count, loff_t *ppos) -{ - struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->type->parent; - - if (unlikely(!parent->ops->read)) - return -EINVAL; - - return parent->ops->read(mdev, buf, count, ppos); -} - -static ssize_t vfio_mdev_write(struct vfio_device *core_vdev, - const char __user *buf, size_t count, - loff_t *ppos) -{ - struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->type->parent; - - if (unlikely(!parent->ops->write)) - return -EINVAL; - - return parent->ops->write(mdev, buf, count, ppos); -} - -static int vfio_mdev_mmap(struct vfio_device *core_vdev, - struct vm_area_struct *vma) -{ - struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->type->parent; - - if (unlikely(!parent->ops->mmap)) - return -EINVAL; - - return parent->ops->mmap(mdev, vma); -} - -static void vfio_mdev_request(struct vfio_device *core_vdev, unsigned int count) -{ - struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->type->parent; - - if (parent->ops->request) - parent->ops->request(mdev, count); - else if (count == 0) - dev_notice(mdev_dev(mdev), - "No mdev vendor driver request callback support, blocked until released by user\n"); -} - -static const struct vfio_device_ops vfio_mdev_dev_ops = { - .name = "vfio-mdev", - .open_device = vfio_mdev_open_device, - .close_device = vfio_mdev_close_device, - .ioctl = vfio_mdev_unlocked_ioctl, - .read = vfio_mdev_read, - .write = vfio_mdev_write, - .mmap = vfio_mdev_mmap, - .request = vfio_mdev_request, -}; - -static int vfio_mdev_probe(struct mdev_device *mdev) -{ - struct vfio_device *vdev; - int ret; - - vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); - if (!vdev) - return -ENOMEM; - - vfio_init_group_dev(vdev, &mdev->dev, &vfio_mdev_dev_ops); - ret = vfio_register_emulated_iommu_dev(vdev); - if (ret) - goto out_uninit; - - dev_set_drvdata(&mdev->dev, vdev); - return 0; - -out_uninit: - vfio_uninit_group_dev(vdev); - kfree(vdev); - return ret; -} - -static void vfio_mdev_remove(struct mdev_device *mdev) -{ - struct vfio_device *vdev = dev_get_drvdata(&mdev->dev); - - vfio_unregister_group_dev(vdev); - vfio_uninit_group_dev(vdev); - kfree(vdev); -} - -struct mdev_driver vfio_mdev_driver = { - .driver = { - .name = "vfio_mdev", - .owner = THIS_MODULE, - .mod_name = KBUILD_MODNAME, - }, - .probe = vfio_mdev_probe, - .remove = vfio_mdev_remove, -}; -- cgit v1.2.3-71-gd317 From e6486939d8ea22569af942a1888aa3824f59cc5e Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Mon, 11 Apr 2022 16:14:00 +0200 Subject: vfio/mdev: Remove mdev_parent_ops dev_attr_groups This is only used by one sample to print a fixed string that is pointless. In general, having a device driver attach sysfs attributes to the parent is horrific. This should never happen, and always leads to some kind of liftime bug as it become very difficult for the sysfs attribute to go back to any data owned by the device driver. Remove the general mechanism to create this abuse. Signed-off-by: Jason Gunthorpe Signed-off-by: Christoph Hellwig Signed-off-by: Zhi Wang Link: http://patchwork.freedesktop.org/patch/msgid/20220411141403.86980-32-hch@lst.de Reviewed-by: Kirti Wankhede Reviewed-by: Zhi Wang --- drivers/vfio/mdev/mdev_sysfs.c | 12 ++---------- include/linux/mdev.h | 2 -- samples/vfio-mdev/mtty.c | 30 +----------------------------- 3 files changed, 3 insertions(+), 41 deletions(-) (limited to 'drivers/vfio') diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index f5cf1931c54e..66eef08833a4 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -197,7 +197,6 @@ void parent_remove_sysfs_files(struct mdev_parent *parent) remove_mdev_supported_type(type); } - sysfs_remove_groups(&parent->dev->kobj, parent->ops->dev_attr_groups); kset_unregister(parent->mdev_types_kset); } @@ -213,17 +212,10 @@ int parent_create_sysfs_files(struct mdev_parent *parent) INIT_LIST_HEAD(&parent->type_list); - ret = sysfs_create_groups(&parent->dev->kobj, - parent->ops->dev_attr_groups); - if (ret) - goto create_err; - ret = add_mdev_supported_type_groups(parent); if (ret) - sysfs_remove_groups(&parent->dev->kobj, - parent->ops->dev_attr_groups); - else - return ret; + goto create_err; + return 0; create_err: kset_unregister(parent->mdev_types_kset); diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 192aed656116..69df1fa2cb6f 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -36,7 +36,6 @@ struct device *mtype_get_parent_dev(struct mdev_type *mtype); * * @owner: The module owner. * @device_driver: Which device driver to probe() on newly created devices - * @dev_attr_groups: Attributes of the parent device. * @mdev_attr_groups: Attributes of the mediated device. * @supported_type_groups: Attributes to define supported types. It is mandatory * to provide supported types. @@ -47,7 +46,6 @@ struct device *mtype_get_parent_dev(struct mdev_type *mtype); struct mdev_parent_ops { struct module *owner; struct mdev_driver *device_driver; - const struct attribute_group **dev_attr_groups; const struct attribute_group **mdev_attr_groups; struct attribute_group **supported_type_groups; }; diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index a0e1a469bd47..4f227dc26785 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -1207,38 +1207,11 @@ static long mtty_ioctl(struct vfio_device *vdev, unsigned int cmd, return -ENOTTY; } -static ssize_t -sample_mtty_dev_show(struct device *dev, struct device_attribute *attr, - char *buf) -{ - return sprintf(buf, "This is phy device\n"); -} - -static DEVICE_ATTR_RO(sample_mtty_dev); - -static struct attribute *mtty_dev_attrs[] = { - &dev_attr_sample_mtty_dev.attr, - NULL, -}; - -static const struct attribute_group mtty_dev_group = { - .name = "mtty_dev", - .attrs = mtty_dev_attrs, -}; - -static const struct attribute_group *mtty_dev_groups[] = { - &mtty_dev_group, - NULL, -}; - static ssize_t sample_mdev_dev_show(struct device *dev, struct device_attribute *attr, char *buf) { - if (mdev_from_dev(dev)) - return sprintf(buf, "This is MDEV %s\n", dev_name(dev)); - - return sprintf(buf, "\n"); + return sprintf(buf, "This is MDEV %s\n", dev_name(dev)); } static DEVICE_ATTR_RO(sample_mdev_dev); @@ -1333,7 +1306,6 @@ static struct mdev_driver mtty_driver = { static const struct mdev_parent_ops mdev_fops = { .owner = THIS_MODULE, .device_driver = &mtty_driver, - .dev_attr_groups = mtty_dev_groups, .supported_type_groups = mdev_type_groups, }; -- cgit v1.2.3-71-gd317 From 6b42f491e17ce13f5ff7f2d1f49c73a0f4c47b20 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Mon, 11 Apr 2022 16:14:01 +0200 Subject: vfio/mdev: Remove mdev_parent_ops The last useful member in this struct is the supported_type_groups, move it to the mdev_driver and delete mdev_parent_ops. Replace it with mdev_driver as an argument to mdev_register_device() Signed-off-by: Jason Gunthorpe Signed-off-by: Christoph Hellwig Signed-off-by: Zhi Wang Link: http://patchwork.freedesktop.org/patch/msgid/20220411141403.86980-33-hch@lst.de Reviewed-by: Kirti Wankhede Reviewed-by: Zhi Wang --- Documentation/driver-api/vfio-mediated-device.rst | 24 +++++----------------- drivers/gpu/drm/i915/gvt/kvmgt.c | 7 +------ drivers/s390/cio/vfio_ccw_ops.c | 7 +------ drivers/s390/crypto/vfio_ap_ops.c | 9 ++------ drivers/vfio/mdev/mdev_core.c | 13 +++++------- drivers/vfio/mdev/mdev_private.h | 2 +- drivers/vfio/mdev/mdev_sysfs.c | 6 +++--- include/linux/mdev.h | 25 ++++------------------- samples/vfio-mdev/mbochs.c | 9 ++------ samples/vfio-mdev/mdpy.c | 9 ++------ samples/vfio-mdev/mtty.c | 9 ++------ 11 files changed, 28 insertions(+), 92 deletions(-) (limited to 'drivers/vfio') diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst index 5a6e18a651a1..784bbeb22adc 100644 --- a/Documentation/driver-api/vfio-mediated-device.rst +++ b/Documentation/driver-api/vfio-mediated-device.rst @@ -105,6 +105,7 @@ structure to represent a mediated device's driver:: struct mdev_driver { int (*probe) (struct mdev_device *dev); void (*remove) (struct mdev_device *dev); + struct attribute_group **supported_type_groups; struct device_driver driver; }; @@ -119,30 +120,15 @@ to register and unregister itself with the core driver: extern void mdev_unregister_driver(struct mdev_driver *drv); -The mediated bus driver is responsible for adding mediated devices to the VFIO -group when devices are bound to the driver and removing mediated devices from -the VFIO when devices are unbound from the driver. - - -Physical Device Driver Interface --------------------------------- - -The physical device driver interface provides the mdev_parent_ops[3] structure -to define the APIs to manage work in the mediated core driver that is related -to the physical device. - -The structures in the mdev_parent_ops structure are as follows: - -* dev_attr_groups: attributes of the parent device -* mdev_attr_groups: attributes of the mediated device -* supported_config: attributes to define supported configurations -* device_driver: device driver to bind for mediated device instances +The mediated bus driver's probe function should create a vfio_device on top of +the mdev_device and connect it to an appropriate implementation of +vfio_device_ops. When a driver wants to add the GUID creation sysfs to an existing device it has probe'd to then it should call:: extern int mdev_register_device(struct device *dev, - const struct mdev_parent_ops *ops); + struct mdev_driver *mdev_driver); This will provide the 'mdev_supported_types/XX/create' files which can then be used to trigger the creation of a mdev_device. The created mdev_device will be diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index f033031c676d..0787ba5c301f 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -1723,12 +1723,7 @@ static struct mdev_driver intel_vgpu_mdev_driver = { }, .probe = intel_vgpu_probe, .remove = intel_vgpu_remove, -}; - -static const struct mdev_parent_ops intel_vgpu_mdev_ops = { - .owner = THIS_MODULE, .supported_type_groups = gvt_vgpu_type_groups, - .device_driver = &intel_vgpu_mdev_driver, }; int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn) @@ -2131,7 +2126,7 @@ static int intel_gvt_init_device(struct drm_i915_private *i915) if (ret) goto out_destroy_idle_vgpu; - ret = mdev_register_device(i915->drm.dev, &intel_vgpu_mdev_ops); + ret = mdev_register_device(i915->drm.dev, &intel_vgpu_mdev_driver); if (ret) goto out_cleanup_vgpu_type_groups; diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index d8589afac272..c4d60cdbf247 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -656,17 +656,12 @@ struct mdev_driver vfio_ccw_mdev_driver = { }, .probe = vfio_ccw_mdev_probe, .remove = vfio_ccw_mdev_remove, -}; - -static const struct mdev_parent_ops vfio_ccw_mdev_ops = { - .owner = THIS_MODULE, - .device_driver = &vfio_ccw_mdev_driver, .supported_type_groups = mdev_type_groups, }; int vfio_ccw_mdev_reg(struct subchannel *sch) { - return mdev_register_device(&sch->dev, &vfio_ccw_mdev_ops); + return mdev_register_device(&sch->dev, &vfio_ccw_mdev_driver); } void vfio_ccw_mdev_unreg(struct subchannel *sch) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 6e08d04b605d..ee0a3bf8f476 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1496,12 +1496,7 @@ static struct mdev_driver vfio_ap_matrix_driver = { }, .probe = vfio_ap_mdev_probe, .remove = vfio_ap_mdev_remove, -}; - -static const struct mdev_parent_ops vfio_ap_matrix_ops = { - .owner = THIS_MODULE, - .device_driver = &vfio_ap_matrix_driver, - .supported_type_groups = vfio_ap_mdev_type_groups, + .supported_type_groups = vfio_ap_mdev_type_groups, }; int vfio_ap_mdev_register(void) @@ -1514,7 +1509,7 @@ int vfio_ap_mdev_register(void) if (ret) return ret; - ret = mdev_register_device(&matrix_dev->device, &vfio_ap_matrix_ops); + ret = mdev_register_device(&matrix_dev->device, &vfio_ap_matrix_driver); if (ret) goto err_driver; return 0; diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 8b1e86b9e8bc..19a20ff420b7 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -109,12 +109,12 @@ static int mdev_device_remove_cb(struct device *dev, void *data) /* * mdev_register_device : Register a device * @dev: device structure representing parent device. - * @ops: Parent device operation structure to be registered. + * @mdev_driver: Device driver to bind to the newly created mdev * * Add device to list of registered parent devices. * Returns a negative value on error, otherwise 0. */ -int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops) +int mdev_register_device(struct device *dev, struct mdev_driver *mdev_driver) { int ret; struct mdev_parent *parent; @@ -122,9 +122,7 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops) char *envp[] = { env_string, NULL }; /* check for mandatory ops */ - if (!ops || !ops->supported_type_groups) - return -EINVAL; - if (!ops->device_driver) + if (!mdev_driver->supported_type_groups) return -EINVAL; dev = get_device(dev); @@ -151,7 +149,7 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops) init_rwsem(&parent->unreg_sem); parent->dev = dev; - parent->ops = ops; + parent->mdev_driver = mdev_driver; if (!mdev_bus_compat_class) { mdev_bus_compat_class = class_compat_register("mdev_bus"); @@ -249,7 +247,7 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) int ret; struct mdev_device *mdev, *tmp; struct mdev_parent *parent = type->parent; - struct mdev_driver *drv = parent->ops->device_driver; + struct mdev_driver *drv = parent->mdev_driver; mutex_lock(&mdev_list_lock); @@ -271,7 +269,6 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) mdev->dev.parent = parent->dev; mdev->dev.bus = &mdev_bus_type; mdev->dev.release = mdev_device_release; - mdev->dev.groups = parent->ops->mdev_attr_groups; mdev->type = type; /* Pairs with the put in mdev_device_release() */ kobject_get(&type->kobj); diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index 6999c89db7b1..5a7ffd4e770f 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -15,7 +15,7 @@ void mdev_bus_unregister(void); struct mdev_parent { struct device *dev; - const struct mdev_parent_ops *ops; + struct mdev_driver *mdev_driver; struct kref ref; struct list_head next; struct kset *mdev_types_kset; diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 66eef08833a4..5a3873d1a275 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -97,7 +97,7 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, { struct mdev_type *type; struct attribute_group *group = - parent->ops->supported_type_groups[type_group_id]; + parent->mdev_driver->supported_type_groups[type_group_id]; int ret; if (!group->name) { @@ -154,7 +154,7 @@ attr_create_failed: static void remove_mdev_supported_type(struct mdev_type *type) { struct attribute_group *group = - type->parent->ops->supported_type_groups[type->type_group_id]; + type->parent->mdev_driver->supported_type_groups[type->type_group_id]; sysfs_remove_files(&type->kobj, (const struct attribute **)group->attrs); @@ -168,7 +168,7 @@ static int add_mdev_supported_type_groups(struct mdev_parent *parent) { int i; - for (i = 0; parent->ops->supported_type_groups[i]; i++) { + for (i = 0; parent->mdev_driver->supported_type_groups[i]; i++) { struct mdev_type *type; type = add_mdev_supported_type(parent, i); diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 69df1fa2cb6f..1f6f57a3c316 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -30,26 +30,6 @@ unsigned int mdev_get_type_group_id(struct mdev_device *mdev); 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 - * register the device to mdev module. - * - * @owner: The module owner. - * @device_driver: Which device driver to probe() on newly created devices - * @mdev_attr_groups: Attributes of the mediated device. - * @supported_type_groups: Attributes to define supported types. It is mandatory - * to provide supported types. - * - * Parent device that support mediated device should be registered with mdev - * module with mdev_parent_ops structure. - **/ -struct mdev_parent_ops { - struct module *owner; - struct mdev_driver *device_driver; - const struct attribute_group **mdev_attr_groups; - struct attribute_group **supported_type_groups; -}; - /* interface for exporting mdev supported type attributes */ struct mdev_type_attribute { struct attribute attr; @@ -74,12 +54,15 @@ struct mdev_type_attribute mdev_type_attr_##_name = \ * struct mdev_driver - Mediated device driver * @probe: called when new device created * @remove: called when device removed + * @supported_type_groups: Attributes to define supported types. It is mandatory + * to provide supported types. * @driver: device driver structure * **/ struct mdev_driver { int (*probe)(struct mdev_device *dev); void (*remove)(struct mdev_device *dev); + struct attribute_group **supported_type_groups; struct device_driver driver; }; @@ -98,7 +81,7 @@ static inline const guid_t *mdev_uuid(struct mdev_device *mdev) extern struct bus_type mdev_bus_type; -int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops); +int mdev_register_device(struct device *dev, struct mdev_driver *mdev_driver); void mdev_unregister_device(struct device *dev); int mdev_register_driver(struct mdev_driver *drv); diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index e90c8552cc31..344c2901a82b 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -1412,12 +1412,7 @@ static struct mdev_driver mbochs_driver = { }, .probe = mbochs_probe, .remove = mbochs_remove, -}; - -static const struct mdev_parent_ops mdev_fops = { - .owner = THIS_MODULE, - .device_driver = &mbochs_driver, - .supported_type_groups = mdev_type_groups, + .supported_type_groups = mdev_type_groups, }; static const struct file_operations vd_fops = { @@ -1462,7 +1457,7 @@ static int __init mbochs_dev_init(void) if (ret) goto err_class; - ret = mdev_register_device(&mbochs_dev, &mdev_fops); + ret = mdev_register_device(&mbochs_dev, &mbochs_driver); if (ret) goto err_device; diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c index fe5d43e797b6..e8c46eb2e246 100644 --- a/samples/vfio-mdev/mdpy.c +++ b/samples/vfio-mdev/mdpy.c @@ -723,12 +723,7 @@ static struct mdev_driver mdpy_driver = { }, .probe = mdpy_probe, .remove = mdpy_remove, -}; - -static const struct mdev_parent_ops mdev_fops = { - .owner = THIS_MODULE, - .device_driver = &mdpy_driver, - .supported_type_groups = mdev_type_groups, + .supported_type_groups = mdev_type_groups, }; static const struct file_operations vd_fops = { @@ -771,7 +766,7 @@ static int __init mdpy_dev_init(void) if (ret) goto err_class; - ret = mdev_register_device(&mdpy_dev, &mdev_fops); + ret = mdev_register_device(&mdpy_dev, &mdpy_driver); if (ret) goto err_device; diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index 4f227dc26785..f42a59ed2e3f 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -1301,12 +1301,7 @@ static struct mdev_driver mtty_driver = { }, .probe = mtty_probe, .remove = mtty_remove, -}; - -static const struct mdev_parent_ops mdev_fops = { - .owner = THIS_MODULE, - .device_driver = &mtty_driver, - .supported_type_groups = mdev_type_groups, + .supported_type_groups = mdev_type_groups, }; static void mtty_device_release(struct device *dev) @@ -1357,7 +1352,7 @@ static int __init mtty_dev_init(void) if (ret) goto err_class; - ret = mdev_register_device(&mtty_dev.dev, &mdev_fops); + ret = mdev_register_device(&mtty_dev.dev, &mtty_driver); if (ret) goto err_device; return 0; -- cgit v1.2.3-71-gd317 From 2aa72ec97ce9eb092cb69efd26f5eb2469e61734 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Mon, 11 Apr 2022 16:14:02 +0200 Subject: vfio/mdev: Use the driver core to create the 'remove' file The device creator is supposed to use the dev.groups value to add sysfs files before device_add is called, not call sysfs_create_files() after device_add() returns. This creates a race with uevent delivery where the extra attribute will not be visible. This was being done because the groups had been co-opted by the mdev driver, now that prior patches have moved the driver's groups to the struct device_driver the dev.group is properly free for use here. Signed-off-by: Jason Gunthorpe Signed-off-by: Christoph Hellwig Signed-off-by: Zhi Wang Link: http://patchwork.freedesktop.org/patch/msgid/20220411141403.86980-34-hch@lst.de Reviewed-by: Kirti Wankhede Reviewed-by: Zhi Wang --- drivers/vfio/mdev/mdev_core.c | 1 + drivers/vfio/mdev/mdev_private.h | 2 ++ drivers/vfio/mdev/mdev_sysfs.c | 19 ++++++++++--------- 3 files changed, 13 insertions(+), 9 deletions(-) (limited to 'drivers/vfio') diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 19a20ff420b7..b8b9e7911e55 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -269,6 +269,7 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) mdev->dev.parent = parent->dev; mdev->dev.bus = &mdev_bus_type; mdev->dev.release = mdev_device_release; + mdev->dev.groups = mdev_device_groups; mdev->type = type; /* Pairs with the put in mdev_device_release() */ kobject_get(&type->kobj); diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index 5a7ffd4e770f..7c9fc79f3d83 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -32,6 +32,8 @@ struct mdev_type { unsigned int type_group_id; }; +extern const struct attribute_group *mdev_device_groups[]; + #define to_mdev_type_attr(_attr) \ container_of(_attr, struct mdev_type_attribute, attr) #define to_mdev_type(_kobj) \ diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 5a3873d1a275..0ccfeb3dda24 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -244,11 +244,20 @@ static ssize_t remove_store(struct device *dev, struct device_attribute *attr, static DEVICE_ATTR_WO(remove); -static const struct attribute *mdev_device_attrs[] = { +static struct attribute *mdev_device_attrs[] = { &dev_attr_remove.attr, NULL, }; +static const struct attribute_group mdev_device_group = { + .attrs = mdev_device_attrs, +}; + +const struct attribute_group *mdev_device_groups[] = { + &mdev_device_group, + NULL +}; + int mdev_create_sysfs_files(struct mdev_device *mdev) { struct mdev_type *type = mdev->type; @@ -262,15 +271,8 @@ int mdev_create_sysfs_files(struct mdev_device *mdev) ret = sysfs_create_link(kobj, &type->kobj, "mdev_type"); if (ret) goto type_link_failed; - - ret = sysfs_create_files(kobj, mdev_device_attrs); - if (ret) - goto create_files_failed; - return ret; -create_files_failed: - sysfs_remove_link(kobj, "mdev_type"); type_link_failed: sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev)); return ret; @@ -280,7 +282,6 @@ void mdev_remove_sysfs_files(struct mdev_device *mdev) { struct kobject *kobj = &mdev->dev.kobj; - sysfs_remove_files(kobj, mdev_device_attrs); sysfs_remove_link(kobj, "mdev_type"); sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev)); } -- cgit v1.2.3-71-gd317