On Tue, 16 Mar 2021 08:09:19 +0000
"Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > Sent: Saturday, March 13, 2021 8:56 AM
> >
> > mdev gets little benefit because it doesn't actually do anything, however
> > it is the last user, so move the code here for now.
>
> and indicate that vfio_add/del_group_dev is removed in this patch.
>
> >
> > Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> > Reviewed-by: Liu Yi L <yi.l.liu@xxxxxxxxx>
> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > ---
> > drivers/vfio/mdev/vfio_mdev.c | 24 +++++++++++++++++++--
> > drivers/vfio/vfio.c | 39 ++---------------------------------
> > include/linux/vfio.h | 5 -----
> > 3 files changed, 24 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
> > index b52eea128549ee..4469aaf31b56cb 100644
> > --- a/drivers/vfio/mdev/vfio_mdev.c
> > +++ b/drivers/vfio/mdev/vfio_mdev.c
> > @@ -21,6 +21,10 @@
> > #define DRIVER_AUTHOR "NVIDIA Corporation"
> > #define DRIVER_DESC "VFIO based driver for Mediated device"
> >
> > +struct mdev_vfio_device {
> > + struct vfio_device vdev;
> > +};
>
> following other vfio_XXX_device convention, what about calling it
> vfio_mdev_device? otherwise,
Or, why actually create this structure at all? _probe and _remove
could just use a vfio_device. Thanks,
Alex
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
>
> > +
> > static int vfio_mdev_open(void *device_data)
> > {
> > struct mdev_device *mdev = device_data;
> > @@ -124,13 +128,29 @@ static const struct vfio_device_ops
> > vfio_mdev_dev_ops = {
> > static int vfio_mdev_probe(struct device *dev)
> > {
> > struct mdev_device *mdev = to_mdev_device(dev);
> > + struct mdev_vfio_device *mvdev;
> > + int ret;
> >
> > - return vfio_add_group_dev(dev, &vfio_mdev_dev_ops, mdev);
> > + mvdev = kzalloc(sizeof(*mvdev), GFP_KERNEL);
> > + if (!mvdev)
> > + return -ENOMEM;
> > +
> > + vfio_init_group_dev(&mvdev->vdev, &mdev->dev,
> > &vfio_mdev_dev_ops, mdev);
> > + ret = vfio_register_group_dev(&mvdev->vdev);
> > + if (ret) {
> > + kfree(mvdev);
> > + return ret;
> > + }
> > + dev_set_drvdata(&mdev->dev, mvdev);
> > + return 0;
> > }
> >
> > static void vfio_mdev_remove(struct device *dev)
> > {
> > - vfio_del_group_dev(dev);
> > + struct mdev_vfio_device *mvdev = dev_get_drvdata(dev);
> > +
> > + vfio_unregister_group_dev(&mvdev->vdev);
> > + kfree(mvdev);
> > }
> >
> > static struct mdev_driver vfio_mdev_driver = {
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index cfa06ae3b9018b..2d6d7cc1d1ebf9 100644
> > --- a/drivers/vfio/vfio.c
> > +++ b/drivers/vfio/vfio.c
> > @@ -99,8 +99,8 @@
> > MODULE_PARM_DESC(enable_unsafe_noiommu_mode, "Enable UNSAFE,
> > no-IOMMU mode. Thi
> > /*
> > * vfio_iommu_group_{get,put} are only intended for VFIO bus driver probe
> > * and remove functions, any use cases other than acquiring the first
> > - * reference for the purpose of calling vfio_add_group_dev() or removing
> > - * that symmetric reference after vfio_del_group_dev() should use the raw
> > + * reference for the purpose of calling vfio_register_group_dev() or
> > removing
> > + * that symmetric reference after vfio_unregister_group_dev() should use
> > the raw
> > * iommu_group_{get,put} functions. In particular, vfio_iommu_group_put()
> > * removes the device from the dummy group and cannot be nested.
> > */
> > @@ -799,29 +799,6 @@ int vfio_register_group_dev(struct vfio_device
> > *device)
> > }
> > EXPORT_SYMBOL_GPL(vfio_register_group_dev);
> >
> > -int vfio_add_group_dev(struct device *dev, const struct vfio_device_ops
> > *ops,
> > - void *device_data)
> > -{
> > - struct vfio_device *device;
> > - int ret;
> > -
> > - device = kzalloc(sizeof(*device), GFP_KERNEL);
> > - if (!device)
> > - return -ENOMEM;
> > -
> > - vfio_init_group_dev(device, dev, ops, device_data);
> > - ret = vfio_register_group_dev(device);
> > - if (ret)
> > - goto err_kfree;
> > - dev_set_drvdata(dev, device);
> > - return 0;
> > -
> > -err_kfree:
> > - kfree(device);
> > - return ret;
> > -}
> > -EXPORT_SYMBOL_GPL(vfio_add_group_dev);
> > -
> > /**
> > * Get a reference to the vfio_device for a device. Even if the
> > * caller thinks they own the device, they could be racing with a
> > @@ -962,18 +939,6 @@ void vfio_unregister_group_dev(struct vfio_device
> > *device)
> > }
> > EXPORT_SYMBOL_GPL(vfio_unregister_group_dev);
> >
> > -void *vfio_del_group_dev(struct device *dev)
> > -{
> > - struct vfio_device *device = dev_get_drvdata(dev);
> > - void *device_data = device->device_data;
> > -
> > - vfio_unregister_group_dev(device);
> > - dev_set_drvdata(dev, NULL);
> > - kfree(device);
> > - return device_data;
> > -}
> > -EXPORT_SYMBOL_GPL(vfio_del_group_dev);
> > -
> > /**
> > * VFIO base fd, /dev/vfio/vfio
> > */
> > diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> > index ad8b579d67d34a..4995faf51efeae 100644
> > --- a/include/linux/vfio.h
> > +++ b/include/linux/vfio.h
> > @@ -63,11 +63,6 @@ extern void vfio_iommu_group_put(struct
> > iommu_group *group, struct device *dev);
> > void vfio_init_group_dev(struct vfio_device *device, struct device *dev,
> > const struct vfio_device_ops *ops, void
> > *device_data);
> > int vfio_register_group_dev(struct vfio_device *device);
> > -extern int vfio_add_group_dev(struct device *dev,
> > - const struct vfio_device_ops *ops,
> > - void *device_data);
> > -
> > -extern void *vfio_del_group_dev(struct device *dev);
> > void vfio_unregister_group_dev(struct vfio_device *device);
> > extern struct vfio_device *vfio_device_get_from_dev(struct device *dev);
> > extern void vfio_device_put(struct vfio_device *device);
> > --
> > 2.30.2
>
