[PATCH] iommu: Remove functions that support private domain

[Prakhya, Sai Praneeth]

Hi Joerg,

> -----Original Message-----
> From: Joerg Roedel <joro at 8bytes.org>
> Sent: Thursday, May 14, 2020 12:56 PM
> To: Prakhya, Sai Praneeth <sai.praneeth.prakhya at intel.com>
> Cc: iommu at lists.linux-foundation.org; Lu Baolu <baolu.lu at linux.intel.com>
> Subject: Re: [PATCH] iommu: Remove functions that support private domain
> 
> On Thu, May 14, 2020 at 06:44:16PM +0000, Prakhya, Sai Praneeth wrote:
> > Could you please explain why we shouldn't change default-domain for an
> > iommu group that has multiple devices?
> 
> Because you can't be sure that a device is bound to a driver while the default
> domain of the group is changed. As long as this race condition exists we can't
> change the default domains of groups with multiple devices at runtime.
> 
> > I am asking this particularly because the patch set I am working on
> > allows to change default-domain for an iommu group that has multiple
> > devices. The pre-requisite being that all the devices in the group
> > should already be unbounded from the device driver and the
> > default-domain preferences of all the devices in the group shouldn't
> > have conflicting types i.e. some devices cannot say they *only* need
> > identity domain while other devices in the same group say that they
> > *only* need to be in DMA domain. In this case, we will not be able to
> > decide upon a default-domain for the iommu group.
> 
> Yeah, but as I wrote above, this is racy and there is currently no way to fix that.
> So we can't support it.

Ok.. below is a previous *similar* comment that you had for one of my patches that change default-domain dynamically.. could you please elaborate it more so that I could have better understanding?
Specifically this one.. "locking all devices in the group and then do the re-attach will introduce a lock-inversion with group->mutex". I didn't get which function call chain would lead us to lock inversion. Could you please shed some light here?

+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;
+}

[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;
+	}

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?

Regards,
Sai