Prologue ========
This is series #3 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 applies on top of Alex's current tree and requires the prior two series.
This series achieves the removal of vfio_mdev.c. The future patches are all focused on leveraging the changes made in the prior series to simplify the API and device operation.
A preview of the future series's is here: https://github.com/jgunthorpe/linux/pull/3/commits
========
The mdev bus's core part for managing the lifecycle of devices is mostly as one would expect for a driver core bus subsystem.
However instead of having a normal 'struct device_driver' and binding the actual mdev drivers through the standard driver core mechanisms it open codes this with the struct mdev_parent_ops and provides a single driver that shims between the VFIO core and the actual device driver.
Make every one of the mdev drivers implement an actual struct mdev_driver and directly call vfio_register_group_dev() in the probe() function for the mdev.
Squash what is left of the mdev_parent_ops into the mdev_driver and remap create(), remove() and mdev_attr_groups to their driver core equivalents. Arrange to bind the created mdev_device to the mdev_driver that is provided by the end driver.
The actual execution flow doesn't change much, eg what was parent_ops->create is now device_driver->probe and it is called at almost the exact same time - except under the normal control of the driver core.
This allows deleting the entire mdev_drvdata, and tidying some of the sysfs. Many places in the drivers start using container_of()
This cleanly splits the mdev sysfs GUID lifecycle management stuff from the vfio_device implementation part, the only VFIO special part of mdev that remains is the mdev specific iommu intervention.
Thanks, Jason
Jason Gunthorpe (12): vfio/mdev: Remove CONFIG_VFIO_MDEV_DEVICE vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind vfio/mtty: Convert to use vfio_register_group_dev() vfio/mdpy: Convert to use vfio_register_group_dev() vfio/mbochs: Convert to use vfio_register_group_dev() vfio/ap_ops: Convert to use vfio_register_group_dev() vfio/ccw: Convert to use vfio_register_group_dev() vfio/gvt: Convert to use vfio_register_group_dev() vfio/mdev: Remove mdev_parent_ops dev_attr_groups vfio/mdev: Remove mdev_parent_ops vfio/mdev: Use the driver core to create the 'remove' file vfio/mdev: Remove mdev drvdata
.../driver-api/vfio-mediated-device.rst | 55 ++--- Documentation/s390/vfio-ap.rst | 1 - arch/s390/Kconfig | 2 +- drivers/gpu/drm/i915/Kconfig | 2 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 210 +++++++++-------- drivers/s390/cio/vfio_ccw_drv.c | 21 +- drivers/s390/cio/vfio_ccw_ops.c | 136 ++++++----- drivers/s390/cio/vfio_ccw_private.h | 5 + drivers/s390/crypto/vfio_ap_ops.c | 138 ++++++----- drivers/s390/crypto/vfio_ap_private.h | 2 + drivers/vfio/mdev/Kconfig | 7 - drivers/vfio/mdev/Makefile | 1 - drivers/vfio/mdev/mdev_core.c | 65 ++++-- drivers/vfio/mdev/mdev_driver.c | 10 +- drivers/vfio/mdev/mdev_private.h | 4 +- drivers/vfio/mdev/mdev_sysfs.c | 37 ++- drivers/vfio/mdev/vfio_mdev.c | 180 --------------- drivers/vfio/vfio.c | 6 +- include/linux/mdev.h | 86 +------ include/linux/vfio.h | 4 + samples/Kconfig | 6 +- samples/vfio-mdev/mbochs.c | 166 +++++++------ samples/vfio-mdev/mdpy.c | 162 +++++++------ samples/vfio-mdev/mtty.c | 218 +++++++----------- 24 files changed, 649 insertions(+), 875 deletions(-) delete mode 100644 drivers/vfio/mdev/vfio_mdev.c
For some reason the vfio_mdev shim mdev_driver has its own module and kconfig. As the next patch requires access to it from mdev.ko merge the two modules together and remove VFIO_MDEV_DEVICE.
A later patch deletes this driver entirely.
This also fixes a misuse of kconfig in the samples which prevented the samples from being built in.
Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- Documentation/s390/vfio-ap.rst | 1 - arch/s390/Kconfig | 2 +- drivers/gpu/drm/i915/Kconfig | 2 +- drivers/vfio/mdev/Kconfig | 7 ------- drivers/vfio/mdev/Makefile | 3 +-- drivers/vfio/mdev/mdev_core.c | 16 ++++++++++++++-- drivers/vfio/mdev/mdev_private.h | 2 ++ drivers/vfio/mdev/vfio_mdev.c | 24 +----------------------- samples/Kconfig | 6 +++--- 9 files changed, 23 insertions(+), 40 deletions(-)
diff --git a/Documentation/s390/vfio-ap.rst b/Documentation/s390/vfio-ap.rst index e15436599086b7..f57ae621f33e89 100644 --- a/Documentation/s390/vfio-ap.rst +++ b/Documentation/s390/vfio-ap.rst @@ -514,7 +514,6 @@ These are the steps: * S390_AP_IOMMU * VFIO * VFIO_MDEV - * VFIO_MDEV_DEVICE * KVM
If using make menuconfig select the following to build the vfio_ap module:: diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index c1ff874e6c2e63..dc7928e37fa409 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -773,7 +773,7 @@ config VFIO_CCW config VFIO_AP def_tristate n prompt "VFIO support for AP devices" - depends on S390_AP_IOMMU && VFIO_MDEV_DEVICE && KVM + depends on S390_AP_IOMMU && VFIO_MDEV && KVM depends on ZCRYPT help This driver grants access to Adjunct Processor (AP) devices diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 483e9ff8ca1d23..388bc41aa1a75b 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -125,7 +125,7 @@ config DRM_I915_GVT_KVMGT tristate "Enable KVM/VFIO support for Intel GVT-g" depends on DRM_I915_GVT depends on KVM - depends on VFIO_MDEV && VFIO_MDEV_DEVICE + depends on VFIO_MDEV default n help Choose this option if you want to enable KVMGT support for diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig index 5da27f2100f9bd..763c877a1318bc 100644 --- a/drivers/vfio/mdev/Kconfig +++ b/drivers/vfio/mdev/Kconfig @@ -9,10 +9,3 @@ config VFIO_MDEV See Documentation/driver-api/vfio-mediated-device.rst for more details.
If you don't know what do here, say N. - -config VFIO_MDEV_DEVICE - tristate "VFIO driver for Mediated devices" - depends on VFIO && VFIO_MDEV - default n - help - VFIO based driver for Mediated devices. diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile index 101516fdf3753e..ff9ecd80212503 100644 --- a/drivers/vfio/mdev/Makefile +++ b/drivers/vfio/mdev/Makefile @@ -1,6 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only
-mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o +mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o vfio_mdev.o
obj-$(CONFIG_VFIO_MDEV) += mdev.o -obj-$(CONFIG_VFIO_MDEV_DEVICE) += vfio_mdev.o diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 2a85d6fcb7ddd0..ff8c1a84516698 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -360,11 +360,24 @@ int mdev_device_remove(struct mdev_device *mdev)
static int __init mdev_init(void) { - return mdev_bus_register(); + 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; }
static void __exit mdev_exit(void) { + mdev_unregister_driver(&vfio_mdev_driver); + if (mdev_bus_compat_class) class_compat_unregister(mdev_bus_compat_class);
@@ -378,4 +391,3 @@ MODULE_VERSION(DRIVER_VERSION); MODULE_LICENSE("GPL v2"); MODULE_AUTHOR(DRIVER_AUTHOR); MODULE_DESCRIPTION(DRIVER_DESC); -MODULE_SOFTDEP("post: vfio_mdev"); diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index a656cfe0346c33..5461b67582289f 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -37,6 +37,8 @@ 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 index 922729071c5a8e..d5b4eede47c1a5 100644 --- a/drivers/vfio/mdev/vfio_mdev.c +++ b/drivers/vfio/mdev/vfio_mdev.c @@ -17,10 +17,6 @@
#include "mdev_private.h"
-#define DRIVER_VERSION "0.1" -#define DRIVER_AUTHOR "NVIDIA Corporation" -#define DRIVER_DESC "VFIO based driver for Mediated device" - static int vfio_mdev_open(struct vfio_device *core_vdev) { struct mdev_device *mdev = to_mdev_device(core_vdev->dev); @@ -151,7 +147,7 @@ static void vfio_mdev_remove(struct mdev_device *mdev) kfree(vdev); }
-static struct mdev_driver vfio_mdev_driver = { +struct mdev_driver vfio_mdev_driver = { .driver = { .name = "vfio_mdev", .owner = THIS_MODULE, @@ -160,21 +156,3 @@ static struct mdev_driver vfio_mdev_driver = { .probe = vfio_mdev_probe, .remove = vfio_mdev_remove, }; - -static int __init vfio_mdev_init(void) -{ - return mdev_register_driver(&vfio_mdev_driver); -} - -static void __exit vfio_mdev_exit(void) -{ - mdev_unregister_driver(&vfio_mdev_driver); -} - -module_init(vfio_mdev_init) -module_exit(vfio_mdev_exit) - -MODULE_VERSION(DRIVER_VERSION); -MODULE_LICENSE("GPL v2"); -MODULE_AUTHOR(DRIVER_AUTHOR); -MODULE_DESCRIPTION(DRIVER_DESC); diff --git a/samples/Kconfig b/samples/Kconfig index e76cdfc50e257d..2a4876e2ce0d03 100644 --- a/samples/Kconfig +++ b/samples/Kconfig @@ -147,14 +147,14 @@ config SAMPLE_UHID
config SAMPLE_VFIO_MDEV_MTTY tristate "Build VFIO mtty example mediated device sample code -- loadable modules only" - depends on VFIO_MDEV_DEVICE && m + depends on VFIO_MDEV help Build a virtual tty sample driver for use as a VFIO mediated device
config SAMPLE_VFIO_MDEV_MDPY tristate "Build VFIO mdpy example mediated device sample code -- loadable modules only" - depends on VFIO_MDEV_DEVICE && m + depends on VFIO_MDEV help Build a virtual display sample driver for use as a VFIO mediated device. It is a simple framebuffer and supports @@ -171,7 +171,7 @@ config SAMPLE_VFIO_MDEV_MDPY_FB
config SAMPLE_VFIO_MDEV_MBOCHS tristate "Build VFIO mdpy example mediated device sample code -- loadable modules only" - depends on VFIO_MDEV_DEVICE && m + depends on VFIO_MDEV select DMA_SHARED_BUFFER help Build a virtual display sample driver for use as a VFIO
On 4/23/21 4:02 PM, Jason Gunthorpe wrote:
@@ -171,7 +171,7 @@ config SAMPLE_VFIO_MDEV_MDPY_FB
config SAMPLE_VFIO_MDEV_MBOCHS tristate "Build VFIO mdpy example mediated device sample code -- loadable modules only"
You can drop the ending of the prompt string.
- depends on VFIO_MDEV_DEVICE && m
- depends on VFIO_MDEV select DMA_SHARED_BUFFER help Build a virtual display sample driver for use as a VFIO
On Fri, Apr 23, 2021 at 05:08:10PM -0700, Randy Dunlap wrote:
On 4/23/21 4:02 PM, Jason Gunthorpe wrote:
@@ -171,7 +171,7 @@ config SAMPLE_VFIO_MDEV_MDPY_FB
config SAMPLE_VFIO_MDEV_MBOCHS tristate "Build VFIO mdpy example mediated device sample code -- loadable modules only"
You can drop the ending of the prompt string.
Hum, I see this whole sample kconfig file is filled with this '&& m' pattern, I wonder if there is a reason?
I think I will put the '&& m' back, I thought it was some kconfig misunderstanding as it is very strange to see a naked '&& M'.
Thanks Jason
On 4/26/21 11:26 AM, Jason Gunthorpe wrote:
On Fri, Apr 23, 2021 at 05:08:10PM -0700, Randy Dunlap wrote:
On 4/23/21 4:02 PM, Jason Gunthorpe wrote:
@@ -171,7 +171,7 @@ config SAMPLE_VFIO_MDEV_MDPY_FB
config SAMPLE_VFIO_MDEV_MBOCHS tristate "Build VFIO mdpy example mediated device sample code -- loadable modules only"
You can drop the ending of the prompt string.
Hum, I see this whole sample kconfig file is filled with this '&& m' pattern, I wonder if there is a reason?
I think I will put the '&& m' back, I thought it was some kconfig misunderstanding as it is very strange to see a naked '&& M'.
It just limits those kconfig items to being =m or not set, i.e., even though they are tristate, setting to =y is not allowed. I guess the thinking is that samples don't need to reside in system memory for very long. However, if you want this one to be capable of =y, like your patch, you can still remove the end of the prompt string.
While there is a confusing mess of pointers and structs in this driver, the struct kvmgt_vdev (which in turn is 1:1 with a struct intel_vgpu) is what holds the vfio_device. Replace all the drvdata's and weird derivations of vgpu and vdev with container_of() or vdev->vgpu.
Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- .../driver-api/vfio-mediated-device.rst | 19 -- drivers/gpu/drm/i915/gvt/kvmgt.c | 208 ++++++++++-------- drivers/vfio/mdev/Makefile | 2 +- drivers/vfio/mdev/mdev_core.c | 47 +--- drivers/vfio/mdev/mdev_driver.c | 11 +- drivers/vfio/mdev/mdev_private.h | 2 - drivers/vfio/mdev/vfio_mdev.c | 158 ------------- drivers/vfio/vfio.c | 6 +- include/linux/mdev.h | 52 ----- include/linux/vfio.h | 4 + 10 files changed, 126 insertions(+), 383 deletions(-) delete mode 100644 drivers/vfio/mdev/vfio_mdev.c
diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst index 1779b85f014e2f..5f866b17c93e69 100644 --- a/Documentation/driver-api/vfio-mediated-device.rst +++ b/Documentation/driver-api/vfio-mediated-device.rst @@ -137,25 +137,6 @@ The structures in the mdev_parent_ops structure are as follows: * mdev_attr_groups: attributes of the mediated device * supported_config: attributes to define supported configurations
-The functions in the mdev_parent_ops structure are as follows: - -* create: allocate basic resources in a driver for a mediated device -* remove: free resources in a driver when a mediated device is destroyed - -(Note that mdev-core provides no implicit serialization of create/remove -callbacks per mdev parent device, per mdev type, or any other categorization. -Vendor drivers are expected to be fully asynchronous in this respect or -provide their own internal resource protection.) - -The callbacks in the mdev_parent_ops structure are as follows: - -* open: open callback of mediated device -* close: close callback of mediated device -* ioctl: ioctl callback of mediated device -* read : read emulation callback -* write: write emulation callback -* mmap: mmap emulation callback - A driver should use the mdev_parent_ops structure in the function call to register itself with the mdev core driver::
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 6bf176e8426e63..85ef300087e091 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -50,6 +50,7 @@ #include "gvt.h"
static const struct intel_gvt_ops *intel_gvt_ops; +static const struct vfio_device_ops intel_vgpu_dev_ops;
/* helper macros copied from vfio-pci */ #define VFIO_PCI_OFFSET_SHIFT 40 @@ -109,8 +110,8 @@ struct gvt_dma { };
struct kvmgt_vdev { + struct vfio_device vfio_device; struct intel_vgpu *vgpu; - struct mdev_device *mdev; struct vfio_region *region; int num_regions; struct eventfd_ctx *intx_trigger; @@ -130,7 +131,6 @@ struct kvmgt_vdev { struct kvm *kvm; struct work_struct release_work; atomic_t released; - struct vfio_device *vfio_device; struct vfio_group *vfio_group; };
@@ -144,7 +144,7 @@ static inline bool handle_valid(unsigned long handle) return !!(handle & ~0xff); }
-static int kvmgt_guest_init(struct mdev_device *mdev); +static int kvmgt_guest_init(struct kvmgt_vdev *vdev); static void intel_vgpu_release_work(struct work_struct *work); static bool kvmgt_guest_exit(struct kvmgt_guest_info *info);
@@ -611,12 +611,7 @@ static int kvmgt_get_vfio_device(void *p_vgpu) struct intel_vgpu *vgpu = (struct intel_vgpu *)p_vgpu; struct kvmgt_vdev *vdev = kvmgt_vdev(vgpu);
- vdev->vfio_device = vfio_device_get_from_dev( - mdev_dev(vdev->mdev)); - if (!vdev->vfio_device) { - gvt_vgpu_err("failed to get vfio device\n"); - return -ENODEV; - } + vfio_device_get(&vdev->vfio_device); return 0; }
@@ -683,16 +678,14 @@ static void kvmgt_put_vfio_device(void *vgpu) { struct kvmgt_vdev *vdev = kvmgt_vdev((struct intel_vgpu *)vgpu);
- if (WARN_ON(!vdev->vfio_device)) - return; - - vfio_device_put(vdev->vfio_device); + vfio_device_put(&vdev->vfio_device); }
-static int intel_vgpu_create(struct mdev_device *mdev) +static int intel_vgpu_probe(struct mdev_device *mdev) { struct intel_vgpu *vgpu = NULL; struct intel_vgpu_type *type; + struct kvmgt_vdev *vdev; struct device *pdev; void *gvt; int ret; @@ -702,40 +695,40 @@ static int intel_vgpu_create(struct mdev_device *mdev)
type = intel_gvt_ops->gvt_find_vgpu_type(gvt, mdev_get_type_group_id(mdev)); - if (!type) { - ret = -EINVAL; - goto out; - } + if (!type) + return -EINVAL;
vgpu = intel_gvt_ops->vgpu_create(gvt, type); if (IS_ERR_OR_NULL(vgpu)) { - ret = vgpu == NULL ? -EFAULT : PTR_ERR(vgpu); gvt_err("failed to create intel vgpu: %d\n", ret); - goto out; + return vgpu == NULL ? -EFAULT : PTR_ERR(vgpu); }
- INIT_WORK(&kvmgt_vdev(vgpu)->release_work, intel_vgpu_release_work); + vdev = kvmgt_vdev(vgpu); + INIT_WORK(&vdev->release_work, intel_vgpu_release_work); + vfio_init_group_dev(&vdev->vfio_device, &mdev->dev, + &intel_vgpu_dev_ops);
- kvmgt_vdev(vgpu)->mdev = mdev; - mdev_set_drvdata(mdev, vgpu); + ret = vfio_register_group_dev(&vdev->vfio_device); + if (ret) { + intel_gvt_ops->vgpu_destroy(vgpu); + return ret; + } + dev_set_drvdata(&mdev->dev, vdev);
gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n", dev_name(mdev_dev(mdev))); - ret = 0; - -out: - return ret; + return 0; }
-static int intel_vgpu_remove(struct mdev_device *mdev) +static void intel_vgpu_remove(struct mdev_device *mdev) { - struct intel_vgpu *vgpu = mdev_get_drvdata(mdev); - - if (handle_valid(vgpu->handle)) - return -EBUSY; + struct kvmgt_vdev *vdev = dev_get_drvdata(&mdev->dev); + struct intel_vgpu *vgpu = vdev->vgpu;
+ if (WARN_ON(handle_valid(vgpu->handle))) + return; intel_gvt_ops->vgpu_destroy(vgpu); - return 0; }
static int intel_vgpu_iommu_notifier(struct notifier_block *nb, @@ -788,10 +781,11 @@ static int intel_vgpu_group_notifier(struct notifier_block *nb, return NOTIFY_OK; }
-static int intel_vgpu_open(struct mdev_device *mdev) +static int intel_vgpu_open(struct vfio_device *vfio_dev) { - struct intel_vgpu *vgpu = mdev_get_drvdata(mdev); - struct kvmgt_vdev *vdev = kvmgt_vdev(vgpu); + struct kvmgt_vdev *vdev = + container_of(vfio_dev, struct kvmgt_vdev, vfio_device); + struct intel_vgpu *vgpu = vdev->vgpu; unsigned long events; int ret; struct vfio_group *vfio_group; @@ -800,7 +794,7 @@ static int intel_vgpu_open(struct mdev_device *mdev) vdev->group_notifier.notifier_call = intel_vgpu_group_notifier;
events = VFIO_IOMMU_NOTIFY_DMA_UNMAP; - ret = vfio_register_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY, &events, + ret = vfio_register_notifier(vfio_dev->dev, VFIO_IOMMU_NOTIFY, &events, &vdev->iommu_notifier); if (ret != 0) { gvt_vgpu_err("vfio_register_notifier for iommu failed: %d\n", @@ -809,7 +803,7 @@ static int intel_vgpu_open(struct mdev_device *mdev) }
events = VFIO_GROUP_NOTIFY_SET_KVM; - ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY, &events, + ret = vfio_register_notifier(vfio_dev->dev, VFIO_GROUP_NOTIFY, &events, &vdev->group_notifier); if (ret != 0) { gvt_vgpu_err("vfio_register_notifier for group failed: %d\n", @@ -817,7 +811,7 @@ static int intel_vgpu_open(struct mdev_device *mdev) goto undo_iommu; }
- vfio_group = vfio_group_get_external_user_from_dev(mdev_dev(mdev)); + vfio_group = vfio_group_get_external_user_from_dev(vfio_dev->dev); if (IS_ERR_OR_NULL(vfio_group)) { ret = !vfio_group ? -EFAULT : PTR_ERR(vfio_group); gvt_vgpu_err("vfio_group_get_external_user_from_dev failed\n"); @@ -833,11 +827,11 @@ static int intel_vgpu_open(struct mdev_device *mdev) goto undo_group; }
- ret = kvmgt_guest_init(mdev); + ret = kvmgt_guest_init(vdev); if (ret) goto undo_group;
- intel_gvt_ops->vgpu_activate(vgpu); + intel_gvt_ops->vgpu_activate(vdev->vgpu);
atomic_set(&vdev->released, 0); return ret; @@ -847,11 +841,11 @@ static int intel_vgpu_open(struct mdev_device *mdev) vdev->vfio_group = NULL;
undo_register: - vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY, + vfio_unregister_notifier(vfio_dev->dev, VFIO_GROUP_NOTIFY, &vdev->group_notifier);
undo_iommu: - vfio_unregister_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY, + vfio_unregister_notifier(vfio_dev->dev, VFIO_IOMMU_NOTIFY, &vdev->iommu_notifier); out: return ret; @@ -884,12 +878,12 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu)
intel_gvt_ops->vgpu_release(vgpu);
- ret = vfio_unregister_notifier(mdev_dev(vdev->mdev), VFIO_IOMMU_NOTIFY, + ret = vfio_unregister_notifier(vdev->vfio_device.dev, VFIO_IOMMU_NOTIFY, &vdev->iommu_notifier); drm_WARN(&i915->drm, ret, "vfio_unregister_notifier for iommu failed: %d\n", ret);
- ret = vfio_unregister_notifier(mdev_dev(vdev->mdev), VFIO_GROUP_NOTIFY, + ret = vfio_unregister_notifier(vdev->vfio_device.dev, VFIO_GROUP_NOTIFY, &vdev->group_notifier); drm_WARN(&i915->drm, ret, "vfio_unregister_notifier for group failed: %d\n", ret); @@ -907,11 +901,12 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu) vgpu->handle = 0; }
-static void intel_vgpu_release(struct mdev_device *mdev) +static void intel_vgpu_release(struct vfio_device *vfio_dev) { - struct intel_vgpu *vgpu = mdev_get_drvdata(mdev); + struct kvmgt_vdev *vdev = + container_of(vfio_dev, struct kvmgt_vdev, vfio_device);
- __intel_vgpu_release(vgpu); + __intel_vgpu_release(vdev->vgpu); }
static void intel_vgpu_release_work(struct work_struct *work) @@ -997,11 +992,10 @@ static int intel_vgpu_aperture_rw(struct intel_vgpu *vgpu, u64 off, return 0; }
-static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf, +static ssize_t intel_vgpu_rw(struct kvmgt_vdev *vdev, char *buf, size_t count, loff_t *ppos, bool is_write) { - struct intel_vgpu *vgpu = mdev_get_drvdata(mdev); - struct kvmgt_vdev *vdev = kvmgt_vdev(vgpu); + struct intel_vgpu *vgpu = vdev->vgpu; unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); u64 pos = *ppos & VFIO_PCI_OFFSET_MASK; int ret = -EINVAL; @@ -1047,9 +1041,9 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf, return ret == 0 ? count : ret; }
-static bool gtt_entry(struct mdev_device *mdev, loff_t *ppos) +static bool gtt_entry(struct kvmgt_vdev *vdev, loff_t *ppos) { - struct intel_vgpu *vgpu = mdev_get_drvdata(mdev); + struct intel_vgpu *vgpu = vdev->vgpu; unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); struct intel_gvt *gvt = vgpu->gvt; int offset; @@ -1066,9 +1060,11 @@ static bool gtt_entry(struct mdev_device *mdev, loff_t *ppos) true : false; }
-static ssize_t intel_vgpu_read(struct mdev_device *mdev, char __user *buf, +static ssize_t intel_vgpu_read(struct vfio_device *vfio_dev, char __user *buf, size_t count, loff_t *ppos) { + struct kvmgt_vdev *vdev = + container_of(vfio_dev, struct kvmgt_vdev, vfio_device); unsigned int done = 0; int ret;
@@ -1077,10 +1073,10 @@ static ssize_t intel_vgpu_read(struct mdev_device *mdev, char __user *buf,
/* Only support GGTT entry 8 bytes read */ if (count >= 8 && !(*ppos % 8) && - gtt_entry(mdev, ppos)) { + gtt_entry(vdev, ppos)) { u64 val;
- ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val), + ret = intel_vgpu_rw(vdev, (char *)&val, sizeof(val), ppos, false); if (ret <= 0) goto read_err; @@ -1092,7 +1088,7 @@ static ssize_t intel_vgpu_read(struct mdev_device *mdev, char __user *buf, } else if (count >= 4 && !(*ppos % 4)) { u32 val;
- ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val), + ret = intel_vgpu_rw(vdev, (char *)&val, sizeof(val), ppos, false); if (ret <= 0) goto read_err; @@ -1104,7 +1100,7 @@ static ssize_t intel_vgpu_read(struct mdev_device *mdev, char __user *buf, } else if (count >= 2 && !(*ppos % 2)) { u16 val;
- ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val), + ret = intel_vgpu_rw(vdev, (char *)&val, sizeof(val), ppos, false); if (ret <= 0) goto read_err; @@ -1116,7 +1112,7 @@ static ssize_t intel_vgpu_read(struct mdev_device *mdev, char __user *buf, } else { u8 val;
- ret = intel_vgpu_rw(mdev, &val, sizeof(val), ppos, + ret = intel_vgpu_rw(vdev, &val, sizeof(val), ppos, false); if (ret <= 0) goto read_err; @@ -1139,10 +1135,12 @@ static ssize_t intel_vgpu_read(struct mdev_device *mdev, char __user *buf, return -EFAULT; }
-static ssize_t intel_vgpu_write(struct mdev_device *mdev, +static ssize_t intel_vgpu_write(struct vfio_device *vfio_dev, const char __user *buf, size_t count, loff_t *ppos) { + struct kvmgt_vdev *vdev = + container_of(vfio_dev, struct kvmgt_vdev, vfio_device); unsigned int done = 0; int ret;
@@ -1151,13 +1149,13 @@ static ssize_t intel_vgpu_write(struct mdev_device *mdev,
/* Only support GGTT entry 8 bytes write */ if (count >= 8 && !(*ppos % 8) && - gtt_entry(mdev, ppos)) { + gtt_entry(vdev, ppos)) { u64 val;
if (copy_from_user(&val, buf, sizeof(val))) goto write_err;
- ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val), + ret = intel_vgpu_rw(vdev, (char *)&val, sizeof(val), ppos, true); if (ret <= 0) goto write_err; @@ -1169,7 +1167,7 @@ static ssize_t intel_vgpu_write(struct mdev_device *mdev, if (copy_from_user(&val, buf, sizeof(val))) goto write_err;
- ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val), + ret = intel_vgpu_rw(vdev, (char *)&val, sizeof(val), ppos, true); if (ret <= 0) goto write_err; @@ -1181,7 +1179,7 @@ static ssize_t intel_vgpu_write(struct mdev_device *mdev, if (copy_from_user(&val, buf, sizeof(val))) goto write_err;
- ret = intel_vgpu_rw(mdev, (char *)&val, + ret = intel_vgpu_rw(vdev, (char *)&val, sizeof(val), ppos, true); if (ret <= 0) goto write_err; @@ -1193,7 +1191,7 @@ static ssize_t intel_vgpu_write(struct mdev_device *mdev, if (copy_from_user(&val, buf, sizeof(val))) goto write_err;
- ret = intel_vgpu_rw(mdev, &val, sizeof(val), + ret = intel_vgpu_rw(vdev, &val, sizeof(val), ppos, true); if (ret <= 0) goto write_err; @@ -1212,13 +1210,16 @@ static ssize_t intel_vgpu_write(struct mdev_device *mdev, return -EFAULT; }
-static int intel_vgpu_mmap(struct mdev_device *mdev, struct vm_area_struct *vma) +static int intel_vgpu_mmap(struct vfio_device *vfio_dev, + struct vm_area_struct *vma) { + struct kvmgt_vdev *vdev = + container_of(vfio_dev, struct kvmgt_vdev, vfio_device); unsigned int index; u64 virtaddr; unsigned long req_size, pgoff, req_start; pgprot_t pg_prot; - struct intel_vgpu *vgpu = mdev_get_drvdata(mdev); + struct intel_vgpu *vgpu = vdev->vgpu;
index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); if (index >= VFIO_PCI_ROM_REGION_INDEX) @@ -1341,11 +1342,12 @@ static int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags, return func(vgpu, index, start, count, flags, data); }
-static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, +static long intel_vgpu_ioctl(struct vfio_device *vfio_dev, unsigned int cmd, unsigned long arg) { - struct intel_vgpu *vgpu = mdev_get_drvdata(mdev); - struct kvmgt_vdev *vdev = kvmgt_vdev(vgpu); + struct kvmgt_vdev *vdev = + container_of(vfio_dev, struct kvmgt_vdev, vfio_device); + struct intel_vgpu *vgpu = vdev->vgpu; unsigned long minsz;
gvt_dbg_core("vgpu%d ioctl, cmd: %d\n", vgpu->id, cmd); @@ -1624,14 +1626,10 @@ static ssize_t vgpu_id_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct mdev_device *mdev = mdev_from_dev(dev); + struct kvmgt_vdev *vdev = dev_get_drvdata(dev); + struct intel_vgpu *vgpu = vdev->vgpu;
- if (mdev) { - struct intel_vgpu *vgpu = (struct intel_vgpu *) - mdev_get_drvdata(mdev); - return sprintf(buf, "%d\n", vgpu->id); - } - return sprintf(buf, "\n"); + return sprintf(buf, "%d\n", vgpu->id); }
static DEVICE_ATTR_RO(vgpu_id); @@ -1651,18 +1649,28 @@ static const struct attribute_group *intel_vgpu_groups[] = { NULL, };
-static struct mdev_parent_ops intel_vgpu_ops = { - .mdev_attr_groups = intel_vgpu_groups, - .create = intel_vgpu_create, - .remove = intel_vgpu_remove, +static const struct vfio_device_ops intel_vgpu_dev_ops = { + .open = intel_vgpu_open, + .release = intel_vgpu_release, + .read = intel_vgpu_read, + .write = intel_vgpu_write, + .mmap = intel_vgpu_mmap, + .ioctl = intel_vgpu_ioctl, +};
- .open = intel_vgpu_open, - .release = intel_vgpu_release, +static struct mdev_driver intel_vgpu_mdev_driver = { + .driver = { + .name = "intel_vgpu_mdev", + .owner = THIS_MODULE, + .mod_name = KBUILD_MODNAME, + .dev_groups = intel_vgpu_groups, + }, + .probe = intel_vgpu_probe, + .remove = intel_vgpu_remove, +};
- .read = intel_vgpu_read, - .write = intel_vgpu_write, - .mmap = intel_vgpu_mmap, - .ioctl = intel_vgpu_ioctl, +static struct mdev_parent_ops intel_vgpu_ops = { + .device_driver = &intel_vgpu_mdev_driver, };
static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops) @@ -1806,18 +1814,12 @@ static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu, struct kvm *kvm) return ret; }
-static int kvmgt_guest_init(struct mdev_device *mdev) +static int kvmgt_guest_init(struct kvmgt_vdev *vdev) { struct kvmgt_guest_info *info; - struct intel_vgpu *vgpu; - struct kvmgt_vdev *vdev; + struct intel_vgpu *vgpu = vdev->vgpu; struct kvm *kvm;
- vgpu = mdev_get_drvdata(mdev); - if (handle_valid(vgpu->handle)) - return -EEXIST; - - vdev = kvmgt_vdev(vgpu); kvm = vdev->kvm; if (!kvm || kvm->mm != current->mm) { gvt_vgpu_err("KVM is required to use Intel vGPU\n"); @@ -2125,13 +2127,25 @@ static const struct intel_gvt_mpt kvmgt_mpt = {
static int __init kvmgt_init(void) { - if (intel_gvt_register_hypervisor(&kvmgt_mpt) < 0) - return -ENODEV; + int ret; + + ret = mdev_register_driver(&intel_vgpu_mdev_driver); + if (ret) + return ret; + + if (intel_gvt_register_hypervisor(&kvmgt_mpt) < 0) { + ret = -ENODEV; + goto err_driver; + } return 0; +err_driver: + mdev_unregister_driver(&intel_vgpu_mdev_driver); + return ret; }
static void __exit kvmgt_exit(void) { + mdev_unregister_driver(&intel_vgpu_mdev_driver); intel_gvt_unregister_hypervisor(); }
diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile index ff9ecd80212503..7c236ba1b90eb1 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 51b8a9fcf866ad..f95d01b57fb168 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,17 +124,13 @@ 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); if (!dev) return -EINVAL;
- /* Not mandatory, but its absence could be a problem */ - if (!ops->request) - dev_info(dev, "Driver cannot be asked to release device\n"); - mutex_lock(&parent_list_lock);
/* Check for duplicate */ @@ -263,15 +252,12 @@ static void mdev_device_release(struct device *dev) */ static int mdev_bind_driver(struct mdev_device *mdev) { - struct mdev_driver *drv = mdev->type->parent->ops->device_driver; int ret;
- if (!drv) - drv = &vfio_mdev_driver; - while (1) { device_lock(&mdev->dev); - if (mdev->dev.driver == &drv->driver) { + if (mdev->dev.driver == + &mdev->type->parent->ops->device_driver->driver) { ret = 0; goto out_unlock; } @@ -337,15 +323,9 @@ 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;
ret = mdev_bind_driver(mdev); if (ret) @@ -363,9 +343,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: @@ -408,27 +385,13 @@ 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 mdev_bus_register(); }
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(); }
diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c index 6e96c023d7823d..0012a9ee7cb0a4 100644 --- a/drivers/vfio/mdev/mdev_driver.c +++ b/drivers/vfio/mdev/mdev_driver.c @@ -74,15 +74,8 @@ static int mdev_remove(struct device *dev) static int mdev_match(struct device *dev, struct device_driver *drv) { struct mdev_device *mdev = to_mdev_device(dev); - struct mdev_driver *target = mdev->type->parent->ops->device_driver; - - /* - * The ops specify the device driver to connect, fall back to the old - * shim driver if the driver hasn't been converted. - */ - if (!target) - target = &vfio_mdev_driver; - return drv == &target->driver; + + return drv == &mdev->type->parent->ops->device_driver->driver; }
struct bus_type mdev_bus_type = { diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index 5461b67582289f..a656cfe0346c33 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 d5b4eede47c1a5..00000000000000 --- a/drivers/vfio/mdev/vfio_mdev.c +++ /dev/null @@ -1,158 +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 cjia@nvidia.com - * Kirti Wankhede kwankhede@nvidia.com - */ - -#include <linux/init.h> -#include <linux/module.h> -#include <linux/device.h> -#include <linux/kernel.h> -#include <linux/slab.h> -#include <linux/vfio.h> -#include <linux/mdev.h> - -#include "mdev_private.h" - -static int vfio_mdev_open(struct vfio_device *core_vdev) -{ - struct mdev_device *mdev = to_mdev_device(core_vdev->dev); - struct mdev_parent *parent = mdev->type->parent; - - int ret; - - if (unlikely(!parent->ops->open)) - return -EINVAL; - - if (!try_module_get(THIS_MODULE)) - return -ENODEV; - - ret = parent->ops->open(mdev); - if (ret) - module_put(THIS_MODULE); - - return ret; -} - -static void vfio_mdev_release(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->release)) - parent->ops->release(mdev); - - module_put(THIS_MODULE); -} - -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 -EINVAL; - - 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 = vfio_mdev_open, - .release = vfio_mdev_release, - .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_group_dev(vdev); - if (ret) { - kfree(vdev); - return ret; - } - dev_set_drvdata(&mdev->dev, vdev); - return 0; -} - -static void vfio_mdev_remove(struct mdev_device *mdev) -{ - struct vfio_device *vdev = dev_get_drvdata(&mdev->dev); - - vfio_unregister_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, -}; diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 5e631c359ef23c..59bbdf6634f934 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -747,7 +747,7 @@ void vfio_init_group_dev(struct vfio_device *device, struct device *dev, device->dev = dev; device->ops = ops; } -EXPORT_SYMBOL_GPL(vfio_init_group_dev); +EXPORT_SYMBOL(vfio_init_group_dev);
int vfio_register_group_dev(struct vfio_device *device) { @@ -796,7 +796,7 @@ int vfio_register_group_dev(struct vfio_device *device)
return 0; } -EXPORT_SYMBOL_GPL(vfio_register_group_dev); +EXPORT_SYMBOL(vfio_register_group_dev);
/** * Get a reference to the vfio_device for a device. Even if the @@ -927,7 +927,7 @@ void vfio_unregister_group_dev(struct vfio_device *device) /* Matches the get in vfio_register_group_dev() */ vfio_group_put(group); } -EXPORT_SYMBOL_GPL(vfio_unregister_group_dev); +EXPORT_SYMBOL(vfio_unregister_group_dev);
/** * VFIO base fd, /dev/vfio/vfio diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 49cc4f65120d57..ea48c401e4fa63 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -61,45 +61,6 @@ struct device *mtype_get_parent_dev(struct mdev_type *mtype); * @mdev_attr_groups: Attributes of the mediated device. * @supported_type_groups: Attributes to define supported types. It is mandatory * to provide supported types. - * @create: Called to allocate basic resources in parent device's - * driver for a particular mediated device. It is - * mandatory to provide create ops. - * @mdev: mdev_device structure on of mediated device - * that is being created - * Returns integer: success (0) or error (< 0) - * @remove: Called to free resources in parent device's driver for - * a mediated device. It is mandatory to provide 'remove' - * ops. - * @mdev: mdev_device device structure which is being - * destroyed - * Returns integer: success (0) or error (< 0) - * @open: Open mediated device. - * @mdev: mediated device. - * Returns integer: success (0) or error (< 0) - * @release: release mediated device - * @mdev: mediated device. - * @read: Read emulation callback - * @mdev: mediated device structure - * @buf: read buffer - * @count: number of bytes to read - * @ppos: address. - * Retuns number on bytes read on success or error. - * @write: Write emulation callback - * @mdev: mediated device structure - * @buf: write buffer - * @count: number of bytes to be written - * @ppos: address. - * Retuns number on bytes written on success or error. - * @ioctl: IOCTL callback - * @mdev: mediated device structure - * @cmd: ioctl command - * @arg: arguments to ioctl - * @mmap: mmap callback - * @mdev: mediated device structure - * @vma: vma structure - * @request: request callback to release device - * @mdev: mediated device structure - * @count: request sequence number * Parent device that support mediated device should be registered with mdev * module with mdev_parent_ops structure. **/ @@ -109,19 +70,6 @@ struct mdev_parent_ops { const struct attribute_group **dev_attr_groups; const struct attribute_group **mdev_attr_groups; struct attribute_group **supported_type_groups; - - int (*create)(struct mdev_device *mdev); - int (*remove)(struct mdev_device *mdev); - int (*open)(struct mdev_device *mdev); - void (*release)(struct mdev_device *mdev); - ssize_t (*read)(struct mdev_device *mdev, char __user *buf, - size_t count, loff_t *ppos); - ssize_t (*write)(struct mdev_device *mdev, const char __user *buf, - size_t count, loff_t *ppos); - long (*ioctl)(struct mdev_device *mdev, unsigned int cmd, - unsigned long arg); - int (*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma); - void (*request)(struct mdev_device *mdev, unsigned int count); };
/* interface for exporting mdev supported type attributes */ diff --git a/include/linux/vfio.h b/include/linux/vfio.h index a2c5b30e1763ba..c5e08be4c56395 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -64,6 +64,10 @@ void vfio_init_group_dev(struct vfio_device *device, struct device *dev, int vfio_register_group_dev(struct vfio_device *device); void vfio_unregister_group_dev(struct vfio_device *device); extern struct vfio_device *vfio_device_get_from_dev(struct device *dev); +static inline void vfio_device_get(struct vfio_device *device) +{ + refcount_inc(&device->refcount); +} extern void vfio_device_put(struct vfio_device *device);
/* events for the backend driver notify callback */
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 jgg@nvidia.com --- .../driver-api/vfio-mediated-device.rst | 36 +++++++------------ drivers/gpu/drm/i915/gvt/kvmgt.c | 8 ++--- 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_driver.c | 2 +- drivers/vfio/mdev/mdev_private.h | 2 +- drivers/vfio/mdev/mdev_sysfs.c | 6 ++-- include/linux/mdev.h | 24 +++---------- samples/vfio-mdev/mbochs.c | 9 ++--- samples/vfio-mdev/mdpy.c | 9 ++--- samples/vfio-mdev/mtty.c | 9 ++--- 12 files changed, 38 insertions(+), 96 deletions(-)
diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst index 5f866b17c93e69..b7cf357243d269 100644 --- a/Documentation/driver-api/vfio-mediated-device.rst +++ b/Documentation/driver-api/vfio-mediated-device.rst @@ -93,7 +93,7 @@ interfaces: Registration Interface for a Mediated Bus Driver ------------------------------------------------
-The registration interface for a mediated bus driver provides the following +The registration interface for a mediated device driver provides the following structure to represent a mediated device's driver::
/* @@ -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,35 +120,24 @@ 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. +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.
- -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 - -A driver should use the mdev_parent_ops structure in the function call to -register itself with the mdev core driver:: +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 attached +to the specified driver.
-However, the mdev_parent_ops structure is not required in the function call -that a driver should use to unregister itself with the mdev core driver:: +When the driver needs to remove itself it calls:
extern void mdev_unregister_device(struct device *dev);
+Which will unbind and destroy all the created mdevs and remove the sysfs files.
Mediated Device Management Interface Through sysfs ================================================== diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 85ef300087e091..02089efd15bb92 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -1669,10 +1669,6 @@ static struct mdev_driver intel_vgpu_mdev_driver = { .remove = intel_vgpu_remove, };
-static struct mdev_parent_ops intel_vgpu_ops = { - .device_driver = &intel_vgpu_mdev_driver, -}; - static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops) { struct attribute_group **kvm_vgpu_type_groups; @@ -1680,9 +1676,9 @@ static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops) intel_gvt_ops = ops; if (!intel_gvt_ops->get_gvt_attrs(&kvm_vgpu_type_groups)) return -EFAULT; - intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups; + intel_vgpu_mdev_driver.supported_type_groups = kvm_vgpu_type_groups;
- return mdev_register_device(dev, &intel_vgpu_ops); + return mdev_register_device(dev, &intel_vgpu_mdev_driver); }
static void kvmgt_host_exit(struct device *dev) diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index 0fcf46031d3821..161697529dcc41 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -655,17 +655,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 79872c857dd522..92789257c87639 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1339,12 +1339,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) @@ -1357,7 +1352,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 f95d01b57fb168..7e918241de10cc 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"); @@ -257,7 +255,7 @@ static int mdev_bind_driver(struct mdev_device *mdev) while (1) { device_lock(&mdev->dev); if (mdev->dev.driver == - &mdev->type->parent->ops->device_driver->driver) { + &mdev->type->parent->mdev_driver->driver) { ret = 0; goto out_unlock; } @@ -304,7 +302,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_driver.c b/drivers/vfio/mdev/mdev_driver.c index 0012a9ee7cb0a4..12091e32afa396 100644 --- a/drivers/vfio/mdev/mdev_driver.c +++ b/drivers/vfio/mdev/mdev_driver.c @@ -75,7 +75,7 @@ static int mdev_match(struct device *dev, struct device_driver *drv) { struct mdev_device *mdev = to_mdev_device(dev);
- return drv == &mdev->type->parent->ops->device_driver->driver; + return drv == &mdev->type->parent->mdev_driver->driver; }
struct bus_type mdev_bus_type = { diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index a656cfe0346c33..839567d059a07d 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; + const 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 66eef08833a4ef..5a3873d1a275ae 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 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, 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 fd9fe1dcf0e230..af807c77c1e0f5 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -51,25 +51,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; @@ -94,12 +75,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; };
@@ -118,7 +102,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 e18821a8a6beb8..c76ceec584b41b 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -1418,12 +1418,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 = { @@ -1466,7 +1461,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 82638de333330d..c22b2c808d132d 100644 --- a/samples/vfio-mdev/mdpy.c +++ b/samples/vfio-mdev/mdpy.c @@ -735,12 +735,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 = { @@ -783,7 +778,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 31eec76bc553ce..87f5ba12a230e3 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -1308,12 +1308,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) @@ -1364,7 +1359,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;
On 24.04.21 01:02, Jason Gunthorpe wrote:
Prologue
This is series #3 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 applies on top of Alex's current tree and requires the prior two series.
Do you have a tree somewhere?
On Mon, Apr 26, 2021 at 06:43:14PM +0200, Christian Borntraeger wrote:
On 24.04.21 01:02, Jason Gunthorpe wrote:
Prologue
This is series #3 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 applies on top of Alex's current tree and requires the prior two series.
Do you have a tree somewhere?
[..]
A preview of the future series's is here: https://github.com/jgunthorpe/linux/pull/3/commits
Has everything, you'll want to go to: cover-letter: Remove vfio_mdev.c, mdev_parent_ops and more
As there are additional WIPs in that tree.
Jason
On 26.04.21 19:42, Jason Gunthorpe wrote:
On Mon, Apr 26, 2021 at 06:43:14PM +0200, Christian Borntraeger wrote:
On 24.04.21 01:02, Jason Gunthorpe wrote:
Prologue
This is series #3 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 applies on top of Alex's current tree and requires the prior two series.
Do you have a tree somewhere?
[..]
A preview of the future series's is here: https://github.com/jgunthorpe/linux/pull/3/commits
Has everything, you'll want to go to: cover-letter: Remove vfio_mdev.c, mdev_parent_ops and more
As there are additional WIPs in that tree.
I gave this a quick spin on s390x vfio-ap and it seems to work ok. This is really just a quick test, but no obvious problem.
On Tue, Apr 27, 2021 at 09:33:56AM +0200, Christian Borntraeger wrote:
On 26.04.21 19:42, Jason Gunthorpe wrote:
On Mon, Apr 26, 2021 at 06:43:14PM +0200, Christian Borntraeger wrote:
On 24.04.21 01:02, Jason Gunthorpe wrote:
Prologue
This is series #3 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 applies on top of Alex's current tree and requires the prior two series.
Do you have a tree somewhere?
[..]
A preview of the future series's is here: https://github.com/jgunthorpe/linux/pull/3/commits
Has everything, you'll want to go to: cover-letter: Remove vfio_mdev.c, mdev_parent_ops and more
As there are additional WIPs in that tree.
I gave this a quick spin on s390x vfio-ap and it seems to work ok. This is really just a quick test, but no obvious problem.
Great! Thanks
Jason
dri-devel@lists.freedesktop.org