From c36abc6d421299349517e171acd36a5a5e8bdc83 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Mon, 5 Jun 2023 21:59:39 -0300 Subject: [PATCH] BACKPORT: iommu: Have __iommu_probe_device() check for already probed devices This is a step toward making __iommu_probe_device() self contained. It should, under proper locking, check if the device is already associated with an iommu driver and resolve parallel probes. All but one of the callers open code this test using two different means, but they all rely on dev->iommu_group. Currently the bus_iommu_probe()/probe_iommu_group() and probe_acpi_namespace_devices() rejects already probed devices with an unlocked read of dev->iommu_group. The OF and ACPI "replay" functions use device_iommu_mapped() which is the same read without the pointless refcount. Move this test into __iommu_probe_device() and put it under the iommu_probe_device_lock. The store to dev->iommu_group is in iommu_group_add_device() which is also called under this lock for iommu driver devices, making it properly locked. The only path that didn't have this check is the hotplug path triggered by BUS_NOTIFY_ADD_DEVICE. The only way to get dev->iommu_group assigned outside the probe path is via iommu_group_add_device(). Today the only caller is VFIO no-iommu which never associates with an iommu driver. Thus adding this additional check is safe. Bug: 337990354 Reviewed-by: Kevin Tian Reviewed-by: Lu Baolu Acked-by: Rafael J. Wysocki Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/1-v3-328044aa278c+45e49-iommu_probe_jgg@nvidia.com Signed-off-by: Joerg Roedel (cherry picked from commit 6eb4da8cf54537992fc9843be8b2af4f83f717e0) Change-Id: I079ee5467e96367a5e1aa2ae5f0d5f6837df597c [quic_nprakash: Resolved conflicts in drivers/iommu/iommu.c] Signed-off-by: Nikhil V --- drivers/acpi/scan.c | 2 +- drivers/iommu/intel/iommu.c | 7 ------- drivers/iommu/iommu.c | 19 +++++++++---------- drivers/iommu/of_iommu.c | 2 +- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 94154a849a3e..a3f2af72b9b8 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1584,7 +1584,7 @@ static const struct iommu_ops *acpi_iommu_configure_id(struct device *dev, * If we have reason to believe the IOMMU driver missed the initial * iommu_probe_device() call for dev, replay it to get things in order. */ - if (!err && dev->bus && !device_iommu_mapped(dev)) + if (!err && dev->bus) err = iommu_probe_device(dev); /* Ignore all other errors apart from EPROBE_DEFER */ diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 820ad242a23c..442ed4246f2e 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -3893,7 +3893,6 @@ static int __init probe_acpi_namespace_devices(void) for_each_active_dev_scope(drhd->devices, drhd->devices_cnt, i, dev) { struct acpi_device_physical_node *pn; - struct iommu_group *group; struct acpi_device *adev; if (dev->bus != &acpi_bus_type) @@ -3903,12 +3902,6 @@ static int __init probe_acpi_namespace_devices(void) mutex_lock(&adev->physical_node_lock); list_for_each_entry(pn, &adev->physical_node_list, node) { - group = iommu_group_get(pn->dev); - if (group) { - iommu_group_put(group); - continue; - } - ret = iommu_probe_device(pn->dev); if (ret) break; diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 5ce7ec724a40..343482649b24 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -327,9 +327,16 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list * but for now enforcing a simple global ordering is fine. */ lockdep_assert_held(&iommu_probe_device_lock); + + /* Device is probed already if in a group */ + if (dev->iommu_group) { + ret = 0; + goto out_unlock; + } + if (!dev_iommu_get(dev)) { ret = -ENOMEM; - goto err_out; + goto out_unlock; } if (!try_module_get(ops->owner)) { @@ -372,7 +379,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list err_free: dev_iommu_free(dev); -err_out: +out_unlock: return ret; } @@ -1706,16 +1713,8 @@ struct iommu_domain *iommu_group_default_domain(struct iommu_group *group) static int probe_iommu_group(struct device *dev, void *data) { struct list_head *group_list = data; - struct iommu_group *group; int ret; - /* Device is probed already if in a group */ - group = iommu_group_get(dev); - if (group) { - iommu_group_put(group); - return 0; - } - mutex_lock(&iommu_probe_device_lock); ret = __iommu_probe_device(dev, group_list); mutex_unlock(&iommu_probe_device_lock); diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 47bf96fc0c45..e5057c464f9c 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -166,7 +166,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, * If we have reason to believe the IOMMU driver missed the initial * probe for dev, replay it to get things in order. */ - if (!err && dev->bus && !device_iommu_mapped(dev)) + if (!err && dev->bus) err = iommu_probe_device(dev); /* Ignore all other errors apart from EPROBE_DEFER */