On Mon, Apr 26, 2021 at 04:13:55PM +0200, Christoph Hellwig wrote:

...

diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile index ff9ecd80212503..7c236ba1b90eb1 100644 +++ 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 +++ b/drivers/vfio/mdev/mdev_core.c

I think all these mdev core changes belong into a separate commit with a separate commit log.

Gah, they were split, I must have flubbed up a rebase on Friday :\

commit daeb9dd3a152e21d11960805b55e34967987e8cf

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.

Preserve VFIO's design of allowing mdev drivers to be !GPL by allowing the three functions that replace this module for !GPL usage. This goes along with the other 19 symbols that are already marked !GPL in VFIO.

Signed-off-by: Jason Gunthorpe jgg@nvidia.com

I'll fix it

...

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();

Weird indentation. But I think it would be best to just kill off the mdev_init wrapper anyway.

Oh, right good point

...

diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c index 6e96c023d7823d..0012a9ee7cb0a4 100644 +++ 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);

• return drv == &mdev->type->parent->ops->device_driver->driver;

}

Btw, I think we don't even need ->match with the switch to use device_bind_driver that I suggested.

See my other email for why it is like this..

...

-EXPORT_SYMBOL_GPL(vfio_init_group_dev); +EXPORT_SYMBOL(vfio_init_group_dev);

...

-EXPORT_SYMBOL_GPL(vfio_register_group_dev); +EXPORT_SYMBOL(vfio_register_group_dev);

...

-EXPORT_SYMBOL_GPL(vfio_unregister_group_dev); +EXPORT_SYMBOL(vfio_unregister_group_dev);

Err, no. vfio should remain EXPORT_SYMBOL_GPL, just because the weird mdev "GPL condom" that should never have been merged in that form went away.

VFIO is already !GPL - there are 19 symbols supporting this today. What happened here is that this patch make all of those symbols unusable !GPL by changing how registration works so you can't get the vfio_device argument to use with the API family.

So, either the two registration functions need to be !GPL to make the other 19 symbols make sense, or the entire !GPL needs to be ripped out. The lost commit message above was explaining this.

Since it is predominately !GPL today, I'd prefer a discussion on changing VFIO to be GPL only to be in its own patch proposing removing all 22 !GPL symbols. Those are always fun threads..

Jason