Hi Joerg,
> -----Original Message-----
> From: Joerg Roedel <j...@8bytes.org>
> Sent: Thursday, May 14, 2020 12:56 PM
> To: Prakhya, Sai Praneeth <sai.praneeth.prak...@intel.com>
> Cc: iommu@lists.linux-foundation.org; Lu Baolu <baolu...@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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
