Re: [PATCH] iommu: Remove functions that support private domain

[Joerg Roedel <joro@8bytes.org>]

On Thu, May 14, 2020 at 11:12:52PM +0000, Prakhya, Sai Praneeth wrote:
> +static int is_driver_bound(struct device *dev, void *not_used)
> +{
> +	int ret = 0;
> +
> +	device_lock(dev);
> +	if (device_is_bound(dev))
> +		ret = 1;
> +	device_unlock(dev);
> +	return ret;
> +}

This locks only one device, so without lock-conversion there could be a
driver probe after the device_unlock(), while we are probing the other
devices of the group.

> [SNIP]
> 
> +	/*
> +	 * Check if any device in the group still has a driver binded to it.
> +	 * This might race with device driver probing code and unfortunately
> +	 * there is no clean way out of that either, locking all devices in the
> +	 * group and then do the re-attach will introduce a lock-inversion with
> +	 * group->mutex - Joerg.
> +	 */
> +	if (iommu_group_for_each_dev(group, NULL, is_driver_bound)) {
> +		pr_err("Active drivers exist for devices in the group\n");
> +		return -EBUSY;
> +	}

The lock inversion comes into the picture when this code is called from
device(-driver) core through the bus-notifiers. The notifiers are called
with the device already locked.

> Another question I have is.. if it's racy then it should be racy even
> for one device iommu groups.. right? Why would it be racy only with
> multiple devices iommu group?

Valid point. So the device needs to be locked _while_ the default domain
change happens. If triggered by sysfs there should be no locking
problems, I guess. But you better try it out.

Regards,

	Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu