From e7f85dfbbc9cf8660174c45c213571aaa518df85 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:07 -0300 Subject: [PATCH] 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 Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/9-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- 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 ea61a81c0006..29ab5d990ef6 100644 --- 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);