| Message ID | 20210615133519.754763-8-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] driver core: Pull required checks into driver_probe_device() | expand |
On Tue, Jun 15 2021, Christoph Hellwig <hch@lst.de> wrote: > From: Jason Gunthorpe <jgg@nvidia.com> > > This allows a mdev driver to opt out of using vfio_mdev.c, instead the > driver will provide a 'struct mdev_driver' and register directly with the > driver core. > > Much of mdev_parent_ops becomes unused in this mode: > - create()/remove() are done via the mdev_driver probe()/remove() > - mdev_attr_groups becomes mdev_driver driver.dev_groups > - Wrapper function callbacks are replaced with the same ones from > struct vfio_device_ops > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/vfio/mdev/mdev_core.c | 30 ++++++++++++++++++++++-------- > drivers/vfio/mdev/mdev_driver.c | 10 ++++++++++ > include/linux/mdev.h | 2 ++ > 3 files changed, 34 insertions(+), 8 deletions(-) > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On Tue, Jun 15, 2021 at 03:35:16PM +0200, Christoph Hellwig wrote: > From: Jason Gunthorpe <jgg@nvidia.com> > > This allows a mdev driver to opt out of using vfio_mdev.c, instead the > driver will provide a 'struct mdev_driver' and register directly with the > driver core. > > Much of mdev_parent_ops becomes unused in this mode: > - create()/remove() are done via the mdev_driver probe()/remove() > - mdev_attr_groups becomes mdev_driver driver.dev_groups > - Wrapper function callbacks are replaced with the same ones from > struct vfio_device_ops > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> Messy, but ok... Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Tue, Jun 15, 2021 at 04:11:29PM +0200, Greg Kroah-Hartman wrote: > On Tue, Jun 15, 2021 at 03:35:16PM +0200, Christoph Hellwig wrote: > > From: Jason Gunthorpe <jgg@nvidia.com> > > > > This allows a mdev driver to opt out of using vfio_mdev.c, instead the > > driver will provide a 'struct mdev_driver' and register directly with the > > driver core. > > > > Much of mdev_parent_ops becomes unused in this mode: > > - create()/remove() are done via the mdev_driver probe()/remove() > > - mdev_attr_groups becomes mdev_driver driver.dev_groups > > - Wrapper function callbacks are replaced with the same ones from > > struct vfio_device_ops > > > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Messy, but ok... Is there something you'd like to see changed, eg in later patches? This whole work still has another approx 30 patches to go and much of this ends up being erased once the drivers are all converted. Jason
On Tue, Jun 15, 2021 at 09:00:40PM -0300, Jason Gunthorpe wrote: > On Tue, Jun 15, 2021 at 04:11:29PM +0200, Greg Kroah-Hartman wrote: > > On Tue, Jun 15, 2021 at 03:35:16PM +0200, Christoph Hellwig wrote: > > > From: Jason Gunthorpe <jgg@nvidia.com> > > > > > > This allows a mdev driver to opt out of using vfio_mdev.c, instead the > > > driver will provide a 'struct mdev_driver' and register directly with the > > > driver core. > > > > > > Much of mdev_parent_ops becomes unused in this mode: > > > - create()/remove() are done via the mdev_driver probe()/remove() > > > - mdev_attr_groups becomes mdev_driver driver.dev_groups > > > - Wrapper function callbacks are replaced with the same ones from > > > struct vfio_device_ops > > > > > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > Messy, but ok... > > Is there something you'd like to see changed, eg in later patches? > This whole work still has another approx 30 patches to go and much of > this ends up being erased once the drivers are all converted. If this mostly gets removed in the end, I'm happy. Let's see how it looks after all of that is done. This is going forward in the right way, so I do not object to this at all. thanks, greg k-h
On 6/15/2021 7:05 PM, Christoph Hellwig wrote: > From: Jason Gunthorpe <jgg@nvidia.com> > > This allows a mdev driver to opt out of using vfio_mdev.c, instead the > driver will provide a 'struct mdev_driver' and register directly with the > driver core. > > Much of mdev_parent_ops becomes unused in this mode: > - create()/remove() are done via the mdev_driver probe()/remove() > - mdev_attr_groups becomes mdev_driver driver.dev_groups > - Wrapper function callbacks are replaced with the same ones from > struct vfio_device_ops > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/vfio/mdev/mdev_core.c | 30 ++++++++++++++++++++++-------- > drivers/vfio/mdev/mdev_driver.c | 10 ++++++++++ > include/linux/mdev.h | 2 ++ > 3 files changed, 34 insertions(+), 8 deletions(-) Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index ff8c1a845166..e4581ec093a6 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -94,9 +94,11 @@ static void mdev_device_remove_common(struct mdev_device *mdev) mdev_remove_sysfs_files(mdev); device_del(&mdev->dev); lockdep_assert_held(&parent->unreg_sem); - ret = parent->ops->remove(mdev); - if (ret) - dev_err(&mdev->dev, "Remove failed: err=%d\n", ret); + 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); @@ -127,7 +129,9 @@ 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->create || !ops->remove || !ops->supported_type_groups) + if (!ops || !ops->supported_type_groups) + return -EINVAL; + if (!ops->device_driver && (!ops->create || !ops->remove)) return -EINVAL; dev = get_device(dev); @@ -256,6 +260,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; mutex_lock(&mdev_list_lock); @@ -296,14 +301,22 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) goto out_put_device; } - ret = parent->ops->create(mdev); - if (ret) - goto out_unlock; + if (parent->ops->create) { + ret = parent->ops->create(mdev); + if (ret) + goto out_unlock; + } ret = device_add(&mdev->dev); if (ret) goto out_remove; + if (!drv) + drv = &vfio_mdev_driver; + ret = device_driver_attach(&drv->driver, &mdev->dev); + if (ret) + goto out_del; + ret = mdev_create_sysfs_files(mdev); if (ret) goto out_del; @@ -317,7 +330,8 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) out_del: device_del(&mdev->dev); out_remove: - parent->ops->remove(mdev); + if (parent->ops->remove) + parent->ops->remove(mdev); out_unlock: up_read(&parent->unreg_sem); out_put_device: diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c index 041699571b7e..c368ec824e2b 100644 --- a/drivers/vfio/mdev/mdev_driver.c +++ b/drivers/vfio/mdev/mdev_driver.c @@ -71,10 +71,20 @@ static int mdev_remove(struct device *dev) return 0; } +static int mdev_match(struct device *dev, struct device_driver *drv) +{ + /* + * No drivers automatically match. Drivers are only bound by explicit + * device_driver_attach() + */ + return 0; +} + struct bus_type mdev_bus_type = { .name = "mdev", .probe = mdev_probe, .remove = mdev_remove, + .match = mdev_match, }; EXPORT_SYMBOL_GPL(mdev_bus_type); diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 1fb34ea394ad..3a38598c2605 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -55,6 +55,7 @@ struct device *mtype_get_parent_dev(struct mdev_type *mtype); * register the device to mdev module. * * @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 @@ -103,6 +104,7 @@ 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;