On Thu, 2021-09-09 at 16:38 -0300, Jason Gunthorpe wrote:
This is a more complicated conversion because vfio_ccw is sharing the vfio_device between both the mdev_device, its vfio_device and the css_driver.
The mdev is a singleton, and the reason for this sharing is so the extra css_driver function callbacks to be delivered to the vfio_device implementation.
This keeps things as they are, with the css_driver allocating the singleton, not the mdev_driver. Following patches work to clean this further.
Embed the vfio_device in the vfio_ccw_private and instantiate it as a vfio_device when the mdev probes. The drvdata of both the css_device and the mdev_device point at the private, and container_of is used to get it back from the vfio_device.
Signed-off-by: Jason Gunthorpe jgg@nvidia.com
drivers/s390/cio/vfio_ccw_drv.c | 21 ++++-- drivers/s390/cio/vfio_ccw_ops.c | 107 +++++++++++++++++--------- -- drivers/s390/cio/vfio_ccw_private.h | 5 ++ 3 files changed, 85 insertions(+), 48 deletions(-)
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 1e8d3151e5480e..396e815f81f8a4 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -469,7 +469,7 @@ static int __init vfio_ccw_sch_init(void) vfio_ccw_work_q = create_singlethread_workqueue("vfio-ccw"); if (!vfio_ccw_work_q) { ret = -ENOMEM;
goto out_err;
goto out_regions;}
vfio_ccw_io_region =
kmem_cache_create_usercopy("vfio_ccw_io_region", @@ -478,7 +478,7 @@ static int __init vfio_ccw_sch_init(void) sizeof(struct ccw_io_region), NULL); if (!vfio_ccw_io_region) { ret = -ENOMEM;
goto out_err;
goto out_regions;}
vfio_ccw_cmd_region =
kmem_cache_create_usercopy("vfio_ccw_cmd_region", @@ -487,7 +487,7 @@ static int __init vfio_ccw_sch_init(void) sizeof(struct ccw_cmd_region), NULL); if (!vfio_ccw_cmd_region) { ret = -ENOMEM;
goto out_err;
goto out_regions;}
vfio_ccw_schib_region =
kmem_cache_create_usercopy("vfio_ccw_schib_region", @@ -497,7 +497,7 @@ static int __init vfio_ccw_sch_init(void)
if (!vfio_ccw_schib_region) { ret = -ENOMEM;
goto out_err;
goto out_regions;}
vfio_ccw_crw_region =
kmem_cache_create_usercopy("vfio_ccw_crw_region", @@ -507,19 +507,25 @@ static int __init vfio_ccw_sch_init(void)
if (!vfio_ccw_crw_region) { ret = -ENOMEM;
goto out_err;
goto out_regions;}
ret = mdev_register_driver(&vfio_ccw_mdev_driver);
if (ret)
goto out_regions;isc_register(VFIO_CCW_ISC); ret = css_driver_register(&vfio_ccw_sch_driver); if (ret) { isc_unregister(VFIO_CCW_ISC);
goto out_err;
goto out_driver;}
return ret;
-out_err: +out_driver:
- mdev_unregister_driver(&vfio_ccw_mdev_driver);
+out_regions: vfio_ccw_destroy_regions(); destroy_workqueue(vfio_ccw_work_q); vfio_ccw_debug_exit(); @@ -528,6 +534,7 @@ static int __init vfio_ccw_sch_init(void)
static void __exit vfio_ccw_sch_exit(void) {
- mdev_unregister_driver(&vfio_ccw_mdev_driver);
Wouldn't it be better to mirror the unwind-init case, such that the above goes...
css_driver_unregister(&vfio_ccw_sch_driver); isc_unregister(VFIO_CCW_ISC);
...here?
vfio_ccw_destroy_regions(); diff --git a/drivers/s390/cio/vfio_ccw_ops.c
...snip...
Besides that, looks fine to me.