Re: linux-next: manual merge of the iommufd tree with the driver-core, vfio trees

[Stephen Rothwell]

Hi all,

On Tue, 6 Dec 2022 13:21:53 +1100 Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
>
> Today's linux-next merge of the iommufd tree got a conflict in:
> 
>   drivers/vfio/vfio_main.c
> 
> between commits:
> 
>   ff62b8e6588f ("driver core: make struct class.devnode() take a const *")
>   e2d55709398e ("vfio: Fold vfio_virqfd.ko into vfio.ko")
> 
> from the driver-core, vfio trees and commits:
> 
>   c2849d718d26 ("vfio: Move vfio group specific code into group.c")
>   f96dc03a8368 ("vfio: Wrap vfio group module init/clean code into helpers")
> 
> from the iommufd tree.
> 
> I fixed it up (I think - see below and I had to add the fix up patch as
> well) and can carry the fix as necessary. This is now fixed as far as
> linux-next is concerned, but any non trivial conflicts should be mentioned
> to your upstream maintainer when your tree is submitted for merging.
> You may also want to consider cooperating with the maintainer of the
> conflicting tree to minimise any particularly complex conflicts.
> 
> From: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
> Date: Tue, 6 Dec 2022 13:17:41 +1100
> Subject: [PATCH] vfio: fix up for "driver core: make struct class.devnode() take a const *"
> 
> Signed-off-by: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
> ---
>  drivers/vfio/group.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
> index c5d8bf10495e..bb24b2f0271e 100644
> --- a/drivers/vfio/group.c
> +++ b/drivers/vfio/group.c
> @@ -827,7 +827,7 @@ bool vfio_file_has_dev(struct file *file, struct vfio_device *device)
>  }
>  EXPORT_SYMBOL_GPL(vfio_file_has_dev);
>  
> -static char *vfio_devnode(struct device *dev, umode_t *mode)
> +static char *vfio_devnode(const struct device *dev, umode_t *mode)
>  {
>  	return kasprintf(GFP_KERNEL, "vfio/%s", dev_name(dev));
>  }
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/vfio/vfio_main.c
> index 718215b55038,e21ff965141e..000000000000
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@@ -433,95 -241,28 +239,16 @@@ out_uninit
>   	ida_free(&vfio.device_ida, device->index);
>   	return ret;
>   }
>  -EXPORT_SYMBOL_GPL(vfio_init_device);
>  -
>  -/*
>  - * The helper called by driver @release callback to free the device
>  - * structure. Drivers which don't have private data to clean can
>  - * simply use this helper as its @release.
>  - */
>  -void vfio_free_device(struct vfio_device *device)
>  -{
>  -	kvfree(device);
>  -}
>  -EXPORT_SYMBOL_GPL(vfio_free_device);
>   
> - static struct vfio_group *vfio_noiommu_group_alloc(struct device *dev,
> - 		enum vfio_group_type type)
> - {
> - 	struct iommu_group *iommu_group;
> - 	struct vfio_group *group;
> - 	int ret;
> - 
> - 	iommu_group = iommu_group_alloc();
> - 	if (IS_ERR(iommu_group))
> - 		return ERR_CAST(iommu_group);
> - 
> - 	ret = iommu_group_set_name(iommu_group, "vfio-noiommu");
> - 	if (ret)
> - 		goto out_put_group;
> - 	ret = iommu_group_add_device(iommu_group, dev);
> - 	if (ret)
> - 		goto out_put_group;
> - 
> - 	group = vfio_create_group(iommu_group, type);
> - 	if (IS_ERR(group)) {
> - 		ret = PTR_ERR(group);
> - 		goto out_remove_device;
> - 	}
> - 	iommu_group_put(iommu_group);
> - 	return group;
> - 
> - out_remove_device:
> - 	iommu_group_remove_device(dev);
> - out_put_group:
> - 	iommu_group_put(iommu_group);
> - 	return ERR_PTR(ret);
> - }
> - 
> - static struct vfio_group *vfio_group_find_or_alloc(struct device *dev)
> - {
> - 	struct iommu_group *iommu_group;
> - 	struct vfio_group *group;
> - 
> - 	iommu_group = iommu_group_get(dev);
> - 	if (!iommu_group && vfio_noiommu) {
> - 		/*
> - 		 * With noiommu enabled, create an IOMMU group for devices that
> - 		 * don't already have one, implying no IOMMU hardware/driver
> - 		 * exists.  Taint the kernel because we're about to give a DMA
> - 		 * capable device to a user without IOMMU protection.
> - 		 */
> - 		group = vfio_noiommu_group_alloc(dev, VFIO_NO_IOMMU);
> - 		if (!IS_ERR(group)) {
> - 			add_taint(TAINT_USER, LOCKDEP_STILL_OK);
> - 			dev_warn(dev, "Adding kernel taint for vfio-noiommu group on device\n");
> - 		}
> - 		return group;
> - 	}
> - 
> - 	if (!iommu_group)
> - 		return ERR_PTR(-EINVAL);
> - 
> - 	/*
> - 	 * VFIO always sets IOMMU_CACHE because we offer no way for userspace to
> - 	 * restore cache coherency. It has to be checked here because it is only
> - 	 * valid for cases where we are using iommu groups.
> - 	 */
> - 	if (!device_iommu_capable(dev, IOMMU_CAP_CACHE_COHERENCY)) {
> - 		iommu_group_put(iommu_group);
> - 		return ERR_PTR(-EINVAL);
> - 	}
> - 
> - 	group = vfio_group_get_from_iommu(iommu_group);
> - 	if (!group)
> - 		group = vfio_create_group(iommu_group, VFIO_IOMMU);
> - 
> - 	/* The vfio_group holds a reference to the iommu_group */
> - 	iommu_group_put(iommu_group);
> - 	return group;
> - }
> - 
>   static int __vfio_register_dev(struct vfio_device *device,
> - 		struct vfio_group *group)
> + 			       enum vfio_group_type type)
>   {
> - 	struct vfio_device *existing_device;
>   	int ret;
>   
> - 	/*
> - 	 * In all cases group is the output of one of the group allocation
> - 	 * functions and we have group->drivers incremented for us.
> - 	 */
> - 	if (IS_ERR(group))
> - 		return PTR_ERR(group);
> + 	if (WARN_ON(device->ops->bind_iommufd &&
> + 		    (!device->ops->unbind_iommufd ||
> + 		     !device->ops->attach_ioas)))
> + 		return -EINVAL;
>   
>   	/*
>   	 * If the driver doesn't specify a set then the device is added to a
> @@@ -1848,19 -1260,6 +1278,10 @@@ static int __init vfio_init(void
>   	if (ret)
>   		return ret;
>   
>  +	ret = vfio_virqfd_init();
>  +	if (ret)
>  +		goto err_virqfd;
>  +
> - 	/* /dev/vfio/$GROUP */
> - 	vfio.class = class_create(THIS_MODULE, "vfio");
> - 	if (IS_ERR(vfio.class)) {
> - 		ret = PTR_ERR(vfio.class);
> - 		goto err_group_class;
> - 	}
> - 
> - 	vfio.class->devnode = vfio_devnode;
> - 
>   	/* /sys/class/vfio-dev/vfioX */
>   	vfio.device_class = class_create(THIS_MODULE, "vfio-dev");
>   	if (IS_ERR(vfio.device_class)) {
> @@@ -1871,36 -1270,17 +1292,20 @@@
>   	pr_info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
>   	return 0;
>   
> - err_alloc_chrdev:
> - 	class_destroy(vfio.device_class);
> - 	vfio.device_class = NULL;
>   err_dev_class:
> - 	class_destroy(vfio.class);
> - 	vfio.class = NULL;
> - err_group_class:
>  +	vfio_virqfd_exit();
>  +err_virqfd:
> - 	vfio_container_cleanup();
> + 	vfio_group_cleanup();
>   	return ret;
>   }
>   
>   static void __exit vfio_cleanup(void)
>   {
> - 	WARN_ON(!list_empty(&vfio.group_list));
> - 
>   	ida_destroy(&vfio.device_ida);
> - 	ida_destroy(&vfio.group_ida);
> - 	unregister_chrdev_region(vfio.group_devt, MINORMASK + 1);
>   	class_destroy(vfio.device_class);
>   	vfio.device_class = NULL;
> - 	class_destroy(vfio.class);
>  +	vfio_virqfd_exit();
> - 	vfio_container_cleanup();
> - 	vfio.class = NULL;
> + 	vfio_group_cleanup();
>   	xa_destroy(&vfio_device_set_xa);
>   }
>   

This is now a conflict between the driver-core, vfio trees and Linus' tree.

-- 
Cheers,
Stephen Rothwell

Attachment: pgpMIrssTS6ez.pgp
Description: OpenPGP digital signature