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

[Lu Baolu <baolu.lu@linux.intel.com>]

Hi,

On 2020/5/15 17:59, Joerg Roedel wrote:
> 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.

It seems that we can do like this:

[1] mutex_lock(&group->lock)
[2] for_each_group_device(device_lock())
[3] if (for_each_group_device(!device_is_bound()))
	change_default_domain()
[4] for_each_group_device_reverse(device_unlock())
[5] mutex_unlock(&group->lock)

A possible problem exists at step 2 when another thread is trying to
lock devices in the reverse order at the same time.

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu