iommu: Fix iommu_probe_device() to attach the right domain

The general invariant is that all devices in an iommu_group are attached
to group->domain. We missed some cases here where an owned group would not
get the device attached.

Rework this logic so it follows the default domain flow of the
bus_iommu_probe() - call iommu_alloc_default_domain(), then use
__iommu_group_set_domain_internal() to set up all the devices.

Finally always attach the device to the current domain if it is already
set.

This is an unlikely functional issue as iommufd uses iommu_attach_group().
It is possible to hot plug in a new group member, add a vfio driver to it
and then hot add it to an existing iommufd. In this case it is required
that the core code set the iommu_domain properly since iommufd won't call
iommu_attach_group() again.

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/9-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com
Signed-off-by: Joerg Roedel <jroedel@suse.de>

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ea61a81..29ab5d9 100644 (file)
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -421,27 +421,31 @@ int iommu_probe_device(struct device *dev)
                goto err_release;
        }
 
-       /*
-        * Try to allocate a default domain - needs support from the
-        * IOMMU driver. There are still some drivers which don't
-        * support default domains, so the return value is not yet
-        * checked.
-        */
        mutex_lock(&group->mutex);
-       iommu_alloc_default_domain(group, dev);
 
-       /*
-        * If device joined an existing group which has been claimed, don't
-        * attach the default domain.
-        */
-       if (group->default_domain && !group->owner) {
+       if (group->domain) {
                ret = __iommu_device_set_domain(group, dev, group->domain, 0);
-               if (ret) {
-                       mutex_unlock(&group->mutex);
-                       iommu_group_put(group);
-                       goto err_release;
-               }
+       } else if (!group->default_domain) {
+               /*
+                * Try to allocate a default domain - needs support from the
+                * IOMMU driver. There are still some drivers which don't
+                * support default domains, so the return value is not yet
+                * checked.
+                */
+               iommu_alloc_default_domain(group, dev);
+               group->domain = NULL;
+               if (group->default_domain)
+                       ret = __iommu_group_set_domain(group,
+                                                      group->default_domain);
+
+               /*
+                * We assume that the iommu driver starts up the device in
+                * 'set_platform_dma_ops' mode if it does not support default
+                * domains.
+                */
        }
+       if (ret)
+               goto err_unlock;
 
        iommu_create_device_direct_mappings(group, dev);
 
@@ -454,6 +458,9 @@ int iommu_probe_device(struct device *dev)
 
        return 0;
 
+err_unlock:
+       mutex_unlock(&group->mutex);
+       iommu_group_put(group);
 err_release:
        iommu_release_device(dev);
 
@@ -1665,9 +1672,6 @@ static int iommu_alloc_default_domain(struct iommu_group *group,
 {
        unsigned int type;
 
-       if (group->default_domain)
-               return 0;
-
        type = iommu_get_def_domain_type(dev) ? : iommu_def_domain_type;
 
        return iommu_group_alloc_default_domain(dev->bus, group, type);