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

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 6700 bytes --]

Hi all,

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@canb.auug.org.au>
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@canb.auug.org.au>
---
 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);
  }
  

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Greg KH]

On Tue, Dec 06, 2022 at 01:21:53PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> 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@canb.auug.org.au>
> 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@canb.auug.org.au>
> ---
>  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));
>  }
> 

Fix looks good to me, thanks!

greg k-h

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

[Alex Williamson]

On Tue, 6 Dec 2022 13:21:53 +1100
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> 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@canb.auug.org.au>
> 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@canb.auug.org.au>
> ---
>  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));
>  }
> 

Looks good, thank you,

Alex

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 7342 bytes --]

Hi all,

On Tue, 6 Dec 2022 13:21:53 +1100 Stephen Rothwell <sfr@canb.auug.org.au> 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@canb.auug.org.au>
> 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@canb.auug.org.au>
> ---
>  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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]