Skip to content

Commit eed20c7

Browse files
rmurphy-armawilliam
authored andcommitted
vfio/type1: Simplify bus_type determination
Since IOMMU groups are mandatory for drivers to support, it stands to reason that any device which has been successfully added to a group must be on a bus supported by that IOMMU driver, and therefore a domain viable for any device in the group must be viable for all devices in the group. This already has to be the case for the IOMMU API's internal default domain, for instance. Thus even if the group contains devices on different buses, that can only mean that the IOMMU driver actually supports such an odd topology, and so without loss of generality we can expect the bus type of any device in a group to be suitable for IOMMU API calls. Furthermore, scrutiny reveals a lack of protection for the bus being removed while vfio_iommu_type1_attach_group() is using it; the reference that VFIO holds on the iommu_group ensures that data remains valid, but does not prevent the group's membership changing underfoot. We can address both concerns by recycling vfio_bus_type() into some superficially similar logic to indirect the IOMMU API calls themselves. Each call is thus protected from races by the IOMMU group's own locking, and we no longer need to hold group-derived pointers beyond that scope. It also gives us an easy path for the IOMMU API's migration of bus-based interfaces to device-based, of which we can already take the first step with device_iommu_capable(). As with domains, any capability must in practice be consistent for devices in a given group - and after all it's still the same capability which was expected to be consistent across an entire bus! - so there's no need for any complicated validation. Signed-off-by: Robin Murphy <[email protected]> Reviewed-by: Kevin Tian <[email protected]> Reviewed-by: Jason Gunthorpe <[email protected]> Link: https://lore.kernel.org/r/194a12d3434d7b38f84fa96503c7664451c8c395.1656092606.git.robin.murphy@arm.com [aw: add comment to vfio_iommu_device_capable()] Signed-off-by: Alex Williamson <[email protected]>
1 parent d1877e6 commit eed20c7

File tree

1 file changed

+23
-20
lines changed

1 file changed

+23
-20
lines changed

drivers/vfio/vfio_iommu_type1.c

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,18 +1679,6 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
16791679
return ret;
16801680
}
16811681

1682-
static int vfio_bus_type(struct device *dev, void *data)
1683-
{
1684-
struct bus_type **bus = data;
1685-
1686-
if (*bus && *bus != dev->bus)
1687-
return -EINVAL;
1688-
1689-
*bus = dev->bus;
1690-
1691-
return 0;
1692-
}
1693-
16941682
static int vfio_iommu_replay(struct vfio_iommu *iommu,
16951683
struct vfio_domain *domain)
16961684
{
@@ -2153,13 +2141,26 @@ static void vfio_iommu_iova_insert_copy(struct vfio_iommu *iommu,
21532141
list_splice_tail(iova_copy, iova);
21542142
}
21552143

2144+
/* Redundantly walks non-present capabilities to simplify caller */
2145+
static int vfio_iommu_device_capable(struct device *dev, void *data)
2146+
{
2147+
return device_iommu_capable(dev, (enum iommu_cap)data);
2148+
}
2149+
2150+
static int vfio_iommu_domain_alloc(struct device *dev, void *data)
2151+
{
2152+
struct iommu_domain **domain = data;
2153+
2154+
*domain = iommu_domain_alloc(dev->bus);
2155+
return 1; /* Don't iterate */
2156+
}
2157+
21562158
static int vfio_iommu_type1_attach_group(void *iommu_data,
21572159
struct iommu_group *iommu_group, enum vfio_group_type type)
21582160
{
21592161
struct vfio_iommu *iommu = iommu_data;
21602162
struct vfio_iommu_group *group;
21612163
struct vfio_domain *domain, *d;
2162-
struct bus_type *bus = NULL;
21632164
bool resv_msi, msi_remap;
21642165
phys_addr_t resv_msi_base = 0;
21652166
struct iommu_domain_geometry *geo;
@@ -2192,18 +2193,19 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
21922193
goto out_unlock;
21932194
}
21942195

2195-
/* Determine bus_type in order to allocate a domain */
2196-
ret = iommu_group_for_each_dev(iommu_group, &bus, vfio_bus_type);
2197-
if (ret)
2198-
goto out_free_group;
2199-
22002196
ret = -ENOMEM;
22012197
domain = kzalloc(sizeof(*domain), GFP_KERNEL);
22022198
if (!domain)
22032199
goto out_free_group;
22042200

2201+
/*
2202+
* Going via the iommu_group iterator avoids races, and trivially gives
2203+
* us a representative device for the IOMMU API call. We don't actually
2204+
* want to iterate beyond the first device (if any).
2205+
*/
22052206
ret = -EIO;
2206-
domain->domain = iommu_domain_alloc(bus);
2207+
iommu_group_for_each_dev(iommu_group, &domain->domain,
2208+
vfio_iommu_domain_alloc);
22072209
if (!domain->domain)
22082210
goto out_free_domain;
22092211

@@ -2258,7 +2260,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
22582260
list_add(&group->next, &domain->group_list);
22592261

22602262
msi_remap = irq_domain_check_msi_remap() ||
2261-
iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
2263+
iommu_group_for_each_dev(iommu_group, (void *)IOMMU_CAP_INTR_REMAP,
2264+
vfio_iommu_device_capable);
22622265

22632266
if (!allow_unsafe_interrupts && !msi_remap) {
22642267
pr_warn("%s: No interrupt remapping support. Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",

0 commit comments

Comments
 (0)