[PATCH v4 09/17] iommu: Fix iommu_probe_device() to attach the right domain

Jason Gunthorpe
2023-04-12T13:52:00Z

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>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
drivers/iommu/iommu.c | 44 +++++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 72c3a9c8e4751d..9076fbbb7d50cd 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -416,27 +416,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);

@@ -449,6 +453,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);

@@ -1689,9 +1696,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);
--
2.40.0