Hi,
here is the second version of this patch-set. The first version with
some more introductory text can be found here:
https://lore.kernel.org/lkml/[email protected]/
Changes v1->v2:
* Rebased to v5.7-rc1
* Re-wrote the arm-smmu changes as suggested by Robin Murphy
* Re-worked the Exynos patches to hopefully not break the
driver anymore
* Fixed a missing mutex_unlock() reported by Marek Szyprowski,
thanks for that.
There is also a git-branch available with these patches applied:
https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v2
Please review.
Thanks,
Joerg
Joerg Roedel (32):
iommu: Move default domain allocation to separate function
iommu/amd: Implement iommu_ops->def_domain_type call-back
iommu/vt-d: Wire up iommu_ops->def_domain_type
iommu/amd: Remove dma_mask check from check_device()
iommu/amd: Return -ENODEV in add_device when device is not handled by
IOMMU
iommu: Add probe_device() and remove_device() call-backs
iommu: Move default domain allocation to iommu_probe_device()
iommu: Keep a list of allocated groups in __iommu_probe_device()
iommu: Move new probe_device path to separate function
iommu: Split off default domain allocation from group assignment
iommu: Move iommu_group_create_direct_mappings() out of
iommu_group_add_device()
iommu: Export bus_iommu_probe() and make is safe for re-probing
iommu/amd: Remove dev_data->passthrough
iommu/amd: Convert to probe/release_device() call-backs
iommu/vt-d: Convert to probe/release_device() call-backs
iommu/arm-smmu: Convert to probe/release_device() call-backs
iommu/pamu: Convert to probe/release_device() call-backs
iommu/s390: Convert to probe/release_device() call-backs
iommu/virtio: Convert to probe/release_device() call-backs
iommu/msm: Convert to probe/release_device() call-backs
iommu/mediatek: Convert to probe/release_device() call-backs
iommu/mediatek-v1 Convert to probe/release_device() call-backs
iommu/qcom: Convert to probe/release_device() call-backs
iommu/rockchip: Convert to probe/release_device() call-backs
iommu/tegra: Convert to probe/release_device() call-backs
iommu/renesas: Convert to probe/release_device() call-backs
iommu/omap: Remove orphan_dev tracking
iommu/omap: Convert to probe/release_device() call-backs
iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
iommu/exynos: Convert to probe/release_device() call-backs
iommu: Remove add_device()/remove_device() code-paths
iommu: Unexport iommu_group_get_for_dev()
Sai Praneeth Prakhya (1):
iommu: Add def_domain_type() callback in iommu_ops
drivers/iommu/amd_iommu.c | 97 ++++----
drivers/iommu/amd_iommu_types.h | 1 -
drivers/iommu/arm-smmu-v3.c | 38 +--
drivers/iommu/arm-smmu.c | 39 ++--
drivers/iommu/exynos-iommu.c | 24 +-
drivers/iommu/fsl_pamu_domain.c | 22 +-
drivers/iommu/intel-iommu.c | 68 +-----
drivers/iommu/iommu.c | 393 +++++++++++++++++++++++++-------
drivers/iommu/ipmmu-vmsa.c | 60 ++---
drivers/iommu/msm_iommu.c | 34 +--
drivers/iommu/mtk_iommu.c | 24 +-
drivers/iommu/mtk_iommu_v1.c | 50 ++--
drivers/iommu/omap-iommu.c | 99 ++------
drivers/iommu/qcom_iommu.c | 24 +-
drivers/iommu/rockchip-iommu.c | 26 +--
drivers/iommu/s390-iommu.c | 22 +-
drivers/iommu/tegra-gart.c | 24 +-
drivers/iommu/tegra-smmu.c | 31 +--
drivers/iommu/virtio-iommu.c | 41 +---
include/linux/iommu.h | 21 +-
20 files changed, 533 insertions(+), 605 deletions(-)
--
2.17.1
From: Joerg Roedel <[email protected]>
Add a check to the bus_iommu_probe() call-path to make sure it ignores
devices which have already been successfully probed. Then export the
bus_iommu_probe() function so it can be used by IOMMU drivers.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/iommu.c | 6 +++++-
include/linux/iommu.h | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 834a45da0ed0..a2ff95424044 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1615,6 +1615,10 @@ static int probe_iommu_group(struct device *dev, void *data)
if (!dev_iommu_get(dev))
return -ENOMEM;
+ /* Device is probed already if in a group */
+ if (iommu_group_get(dev) != NULL)
+ return 0;
+
if (!try_module_get(ops->owner)) {
ret = -EINVAL;
goto err_free_dev_iommu;
@@ -1783,7 +1787,7 @@ static int iommu_group_create_direct_mappings(struct iommu_group *group)
iommu_do_create_direct_mappings);
}
-static int bus_iommu_probe(struct bus_type *bus)
+int bus_iommu_probe(struct bus_type *bus)
{
const struct iommu_ops *ops = bus->iommu_ops;
int ret;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 30170d191e5e..fea1622408ad 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -445,6 +445,7 @@ static inline void iommu_iotlb_gather_init(struct iommu_iotlb_gather *gather)
#define IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER 6 /* Post Driver unbind */
extern int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops);
+extern int bus_iommu_probe(struct bus_type *bus);
extern bool iommu_present(struct bus_type *bus);
extern bool iommu_capable(struct bus_type *bus, enum iommu_cap cap);
extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
--
2.17.1
From: Joerg Roedel <[email protected]>
Convert the Exynos IOMMU driver to use the probe_device() and
release_device() call-backs of iommu_ops, so that the iommu core code
does the group and sysfs setup.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/exynos-iommu.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 09cdd163560a..60c8a56e4a3f 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1235,19 +1235,13 @@ static phys_addr_t exynos_iommu_iova_to_phys(struct iommu_domain *iommu_domain,
return phys;
}
-static int exynos_iommu_add_device(struct device *dev)
+static struct iommu_device *exynos_iommu_probe_device(struct device *dev)
{
struct exynos_iommu_owner *owner = dev->archdata.iommu;
struct sysmmu_drvdata *data;
- struct iommu_group *group;
if (!has_sysmmu(dev))
- return -ENODEV;
-
- group = iommu_group_get_for_dev(dev);
-
- if (IS_ERR(group))
- return PTR_ERR(group);
+ return ERR_PTR(-ENODEV);
list_for_each_entry(data, &owner->controllers, owner_node) {
/*
@@ -1259,17 +1253,15 @@ static int exynos_iommu_add_device(struct device *dev)
DL_FLAG_STATELESS |
DL_FLAG_PM_RUNTIME);
}
- iommu_group_put(group);
/* There is always at least one entry, see exynos_iommu_of_xlate() */
data = list_first_entry(&owner->controllers,
struct sysmmu_drvdata, owner_node);
- iommu_device_link(&data->iommu, dev);
- return 0;
+ return &data->iommu;
}
-static void exynos_iommu_remove_device(struct device *dev)
+static void exynos_iommu_release_device(struct device *dev)
{
struct exynos_iommu_owner *owner = dev->archdata.iommu;
struct sysmmu_drvdata *data;
@@ -1287,15 +1279,9 @@ static void exynos_iommu_remove_device(struct device *dev)
iommu_group_put(group);
}
}
- iommu_group_remove_device(dev);
list_for_each_entry(data, &owner->controllers, owner_node)
device_link_del(data->link);
-
- /* There is always at least one entry, see exynos_iommu_of_xlate() */
- data = list_first_entry(&owner->controllers,
- struct sysmmu_drvdata, owner_node);
- iommu_device_unlink(&data->iommu, dev);
}
static int exynos_iommu_of_xlate(struct device *dev,
@@ -1341,8 +1327,8 @@ static const struct iommu_ops exynos_iommu_ops = {
.unmap = exynos_iommu_unmap,
.iova_to_phys = exynos_iommu_iova_to_phys,
.device_group = generic_device_group,
- .add_device = exynos_iommu_add_device,
- .remove_device = exynos_iommu_remove_device,
+ .probe_device = exynos_iommu_probe_device,
+ .release_device = exynos_iommu_release_device,
.pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
.of_xlate = exynos_iommu_of_xlate,
};
--
2.17.1
From: Joerg Roedel <[email protected]>
Move the code out of iommu_group_get_for_dev() into a separate
function.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/iommu.c | 74 ++++++++++++++++++++++++++-----------------
1 file changed, 45 insertions(+), 29 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2b471419e26c..bfe011760ed1 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1361,6 +1361,41 @@ struct iommu_group *fsl_mc_device_group(struct device *dev)
}
EXPORT_SYMBOL_GPL(fsl_mc_device_group);
+static int iommu_alloc_default_domain(struct device *dev,
+ struct iommu_group *group)
+{
+ struct iommu_domain *dom;
+
+ if (group->default_domain)
+ return 0;
+
+ dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
+ if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
+ dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
+ if (dom) {
+ dev_warn(dev,
+ "failed to allocate default IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
+ iommu_def_domain_type);
+ }
+ }
+
+ if (!dom)
+ return -ENOMEM;
+
+ group->default_domain = dom;
+ if (!group->domain)
+ group->domain = dom;
+
+ if (!iommu_dma_strict) {
+ int attr = 1;
+ iommu_domain_set_attr(dom,
+ DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
+ &attr);
+ }
+
+ return 0;
+}
+
/**
* iommu_group_get_for_dev - Find or create the IOMMU group for a device
* @dev: target device
@@ -1393,40 +1428,21 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
/*
* Try to allocate a default domain - needs support from the
- * IOMMU driver.
+ * IOMMU driver. There are still some drivers which don't support
+ * default domains, so the return value is not yet checked.
*/
- if (!group->default_domain) {
- struct iommu_domain *dom;
-
- dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
- if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
- dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
- if (dom) {
- dev_warn(dev,
- "failed to allocate default IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
- iommu_def_domain_type);
- }
- }
-
- group->default_domain = dom;
- if (!group->domain)
- group->domain = dom;
-
- if (dom && !iommu_dma_strict) {
- int attr = 1;
- iommu_domain_set_attr(dom,
- DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
- &attr);
- }
- }
+ iommu_alloc_default_domain(dev, group);
ret = iommu_group_add_device(group, dev);
- if (ret) {
- iommu_group_put(group);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto out_put_group;
return group;
+
+out_put_group:
+ iommu_group_put(group);
+
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL(iommu_group_get_for_dev);
--
2.17.1
From: Joerg Roedel <[email protected]>
Convert the Mediatek-v1 IOMMU driver to use the probe_device() and
release_device() call-backs of iommu_ops, so that the iommu core code
does the group and sysfs setup.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/mtk_iommu_v1.c | 50 +++++++++++++++---------------------
1 file changed, 20 insertions(+), 30 deletions(-)
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index a31be05601c9..7bdd74c7cb9f 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -416,14 +416,12 @@ static int mtk_iommu_create_mapping(struct device *dev,
return 0;
}
-static int mtk_iommu_add_device(struct device *dev)
+static struct iommu_device *mtk_iommu_probe_device(struct device *dev)
{
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
- struct dma_iommu_mapping *mtk_mapping;
struct of_phandle_args iommu_spec;
struct of_phandle_iterator it;
struct mtk_iommu_data *data;
- struct iommu_group *group;
int err;
of_for_each_phandle(&it, err, dev->of_node, "iommus",
@@ -442,35 +440,28 @@ static int mtk_iommu_add_device(struct device *dev)
}
if (!fwspec || fwspec->ops != &mtk_iommu_ops)
- return -ENODEV; /* Not a iommu client device */
+ return ERR_PTR(-ENODEV); /* Not a iommu client device */
- /*
- * This is a short-term bodge because the ARM DMA code doesn't
- * understand multi-device groups, but we have to call into it
- * successfully (and not just rely on a normal IOMMU API attach
- * here) in order to set the correct DMA API ops on @dev.
- */
- group = iommu_group_alloc();
- if (IS_ERR(group))
- return PTR_ERR(group);
+ data = dev_iommu_priv_get(dev);
- err = iommu_group_add_device(group, dev);
- iommu_group_put(group);
- if (err)
- return err;
+ return &data->iommu;
+}
- data = dev_iommu_priv_get(dev);
+static void mtk_iommu_probe_finalize(struct device *dev)
+{
+ struct dma_iommu_mapping *mtk_mapping;
+ struct mtk_iommu_data *data;
+ int err;
+
+ data = dev_iommu_priv_get(dev);
mtk_mapping = data->dev->archdata.iommu;
- err = arm_iommu_attach_device(dev, mtk_mapping);
- if (err) {
- iommu_group_remove_device(dev);
- return err;
- }
- return iommu_device_link(&data->iommu, dev);
+ err = arm_iommu_attach_device(dev, mtk_mapping);
+ if (err)
+ dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
}
-static void mtk_iommu_remove_device(struct device *dev)
+static void mtk_iommu_release_device(struct device *dev)
{
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct mtk_iommu_data *data;
@@ -479,9 +470,6 @@ static void mtk_iommu_remove_device(struct device *dev)
return;
data = dev_iommu_priv_get(dev);
- iommu_device_unlink(&data->iommu, dev);
-
- iommu_group_remove_device(dev);
iommu_fwspec_free(dev);
}
@@ -534,8 +522,10 @@ static const struct iommu_ops mtk_iommu_ops = {
.map = mtk_iommu_map,
.unmap = mtk_iommu_unmap,
.iova_to_phys = mtk_iommu_iova_to_phys,
- .add_device = mtk_iommu_add_device,
- .remove_device = mtk_iommu_remove_device,
+ .probe_device = mtk_iommu_probe_device,
+ .probe_finalize = mtk_iommu_probe_finalize,
+ .release_device = mtk_iommu_release_device,
+ .device_group = generic_device_group,
.pgsize_bitmap = ~0UL << MT2701_IOMMU_PAGE_SHIFT,
};
--
2.17.1
From: Joerg Roedel <[email protected]>
Remove the tracking of device which could not be probed because
their IOMMU is not probed yet. Replace it with a call to
bus_iommu_probe() when a new IOMMU is probed.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/omap-iommu.c | 54 +++-----------------------------------
1 file changed, 4 insertions(+), 50 deletions(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 887fefcb03b4..ecc9d0829a91 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -35,15 +35,6 @@
static const struct iommu_ops omap_iommu_ops;
-struct orphan_dev {
- struct device *dev;
- struct list_head node;
-};
-
-static LIST_HEAD(orphan_dev_list);
-
-static DEFINE_SPINLOCK(orphan_lock);
-
#define to_iommu(dev) ((struct omap_iommu *)dev_get_drvdata(dev))
/* bitmap of the page sizes currently supported */
@@ -62,8 +53,6 @@ static DEFINE_SPINLOCK(orphan_lock);
static struct platform_driver omap_iommu_driver;
static struct kmem_cache *iopte_cachep;
-static int _omap_iommu_add_device(struct device *dev);
-
/**
* to_omap_domain - Get struct omap_iommu_domain from generic iommu_domain
* @dom: generic iommu domain handle
@@ -1177,7 +1166,6 @@ static int omap_iommu_probe(struct platform_device *pdev)
struct omap_iommu *obj;
struct resource *res;
struct device_node *of = pdev->dev.of_node;
- struct orphan_dev *orphan_dev, *tmp;
if (!of) {
pr_err("%s: only DT-based devices are supported\n", __func__);
@@ -1260,13 +1248,8 @@ static int omap_iommu_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "%s registered\n", obj->name);
- list_for_each_entry_safe(orphan_dev, tmp, &orphan_dev_list, node) {
- err = _omap_iommu_add_device(orphan_dev->dev);
- if (!err) {
- list_del(&orphan_dev->node);
- kfree(orphan_dev);
- }
- }
+ /* Re-probe bus to probe device attached to this IOMMU */
+ bus_iommu_probe(&platform_bus_type);
return 0;
@@ -1657,7 +1640,7 @@ static phys_addr_t omap_iommu_iova_to_phys(struct iommu_domain *domain,
return ret;
}
-static int _omap_iommu_add_device(struct device *dev)
+static int omap_iommu_add_device(struct device *dev)
{
struct omap_iommu_arch_data *arch_data, *tmp;
struct omap_iommu *oiommu;
@@ -1666,8 +1649,6 @@ static int _omap_iommu_add_device(struct device *dev)
struct platform_device *pdev;
int num_iommus, i;
int ret;
- struct orphan_dev *orphan_dev;
- unsigned long flags;
/*
* Allocate the archdata iommu structure for DT-based devices.
@@ -1702,23 +1683,7 @@ static int _omap_iommu_add_device(struct device *dev)
if (!pdev) {
of_node_put(np);
kfree(arch_data);
- spin_lock_irqsave(&orphan_lock, flags);
- list_for_each_entry(orphan_dev, &orphan_dev_list,
- node) {
- if (orphan_dev->dev == dev)
- break;
- }
- spin_unlock_irqrestore(&orphan_lock, flags);
-
- if (orphan_dev && orphan_dev->dev == dev)
- return -EPROBE_DEFER;
-
- orphan_dev = kzalloc(sizeof(*orphan_dev), GFP_KERNEL);
- orphan_dev->dev = dev;
- spin_lock_irqsave(&orphan_lock, flags);
- list_add(&orphan_dev->node, &orphan_dev_list);
- spin_unlock_irqrestore(&orphan_lock, flags);
- return -EPROBE_DEFER;
+ return -ENODEV;
}
oiommu = platform_get_drvdata(pdev);
@@ -1764,17 +1729,6 @@ static int _omap_iommu_add_device(struct device *dev)
return 0;
}
-static int omap_iommu_add_device(struct device *dev)
-{
- int ret;
-
- ret = _omap_iommu_add_device(dev);
- if (ret == -EPROBE_DEFER)
- return 0;
-
- return ret;
-}
-
static void omap_iommu_remove_device(struct device *dev)
{
struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
--
2.17.1
From: Joerg Roedel <[email protected]>
Convert the Rockchip IOMMU driver to use the probe_device() and
release_device() call-backs of iommu_ops, so that the iommu core code
does the group and sysfs setup.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index b33cdd5aad81..d25c2486ca07 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1054,40 +1054,28 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
kfree(rk_domain);
}
-static int rk_iommu_add_device(struct device *dev)
+static struct iommu_device *rk_iommu_probe_device(struct device *dev)
{
- struct iommu_group *group;
- struct rk_iommu *iommu;
struct rk_iommudata *data;
+ struct rk_iommu *iommu;
data = dev->archdata.iommu;
if (!data)
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
iommu = rk_iommu_from_dev(dev);
- group = iommu_group_get_for_dev(dev);
- if (IS_ERR(group))
- return PTR_ERR(group);
- iommu_group_put(group);
-
- iommu_device_link(&iommu->iommu, dev);
data->link = device_link_add(dev, iommu->dev,
DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
- return 0;
+ return &iommu->iommu;
}
-static void rk_iommu_remove_device(struct device *dev)
+static void rk_iommu_release_device(struct device *dev)
{
- struct rk_iommu *iommu;
struct rk_iommudata *data = dev->archdata.iommu;
- iommu = rk_iommu_from_dev(dev);
-
device_link_del(data->link);
- iommu_device_unlink(&iommu->iommu, dev);
- iommu_group_remove_device(dev);
}
static struct iommu_group *rk_iommu_device_group(struct device *dev)
@@ -1126,8 +1114,8 @@ static const struct iommu_ops rk_iommu_ops = {
.detach_dev = rk_iommu_detach_device,
.map = rk_iommu_map,
.unmap = rk_iommu_unmap,
- .add_device = rk_iommu_add_device,
- .remove_device = rk_iommu_remove_device,
+ .probe_device = rk_iommu_probe_device,
+ .release_device = rk_iommu_release_device,
.iova_to_phys = rk_iommu_iova_to_phys,
.device_group = rk_iommu_device_group,
.pgsize_bitmap = RK_IOMMU_PGSIZE_BITMAP,
--
2.17.1
From: Joerg Roedel <[email protected]>
On Exynos platforms there can be more than one SYSMMU (IOMMU) for one
DMA master device. Since the IOMMU core code expects only one hardware
IOMMU, use the first SYSMMU in the list.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/exynos-iommu.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 186ff5cc975c..09cdd163560a 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1261,6 +1261,11 @@ static int exynos_iommu_add_device(struct device *dev)
}
iommu_group_put(group);
+ /* There is always at least one entry, see exynos_iommu_of_xlate() */
+ data = list_first_entry(&owner->controllers,
+ struct sysmmu_drvdata, owner_node);
+ iommu_device_link(&data->iommu, dev);
+
return 0;
}
@@ -1286,6 +1291,11 @@ static void exynos_iommu_remove_device(struct device *dev)
list_for_each_entry(data, &owner->controllers, owner_node)
device_link_del(data->link);
+
+ /* There is always at least one entry, see exynos_iommu_of_xlate() */
+ data = list_first_entry(&owner->controllers,
+ struct sysmmu_drvdata, owner_node);
+ iommu_device_unlink(&data->iommu, dev);
}
static int exynos_iommu_of_xlate(struct device *dev,
--
2.17.1
From: Joerg Roedel <[email protected]>
Convert the VirtIO IOMMU driver to use the probe_device() and
release_device() call-backs of iommu_ops, so that the iommu core code
does the group and sysfs setup.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/virtio-iommu.c | 41 +++++++++---------------------------
1 file changed, 10 insertions(+), 31 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index d5cac4f46ca5..bda300c2a438 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -865,24 +865,23 @@ static struct viommu_dev *viommu_get_by_fwnode(struct fwnode_handle *fwnode)
return dev ? dev_to_virtio(dev)->priv : NULL;
}
-static int viommu_add_device(struct device *dev)
+static struct iommu_device *viommu_probe_device(struct device *dev)
{
int ret;
- struct iommu_group *group;
struct viommu_endpoint *vdev;
struct viommu_dev *viommu = NULL;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
if (!fwspec || fwspec->ops != &viommu_ops)
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
viommu = viommu_get_by_fwnode(fwspec->iommu_fwnode);
if (!viommu)
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
if (!vdev)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
vdev->dev = dev;
vdev->viommu = viommu;
@@ -896,45 +895,25 @@ static int viommu_add_device(struct device *dev)
goto err_free_dev;
}
- ret = iommu_device_link(&viommu->iommu, dev);
- if (ret)
- goto err_free_dev;
+ return &viommu->iommu;
- /*
- * Last step creates a default domain and attaches to it. Everything
- * must be ready.
- */
- group = iommu_group_get_for_dev(dev);
- if (IS_ERR(group)) {
- ret = PTR_ERR(group);
- goto err_unlink_dev;
- }
-
- iommu_group_put(group);
-
- return PTR_ERR_OR_ZERO(group);
-
-err_unlink_dev:
- iommu_device_unlink(&viommu->iommu, dev);
err_free_dev:
generic_iommu_put_resv_regions(dev, &vdev->resv_regions);
kfree(vdev);
- return ret;
+ return ERR_PTR(ret);
}
-static void viommu_remove_device(struct device *dev)
+static void viommu_release_device(struct device *dev)
{
- struct viommu_endpoint *vdev;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ struct viommu_endpoint *vdev;
if (!fwspec || fwspec->ops != &viommu_ops)
return;
vdev = dev_iommu_priv_get(dev);
- iommu_group_remove_device(dev);
- iommu_device_unlink(&vdev->viommu->iommu, dev);
generic_iommu_put_resv_regions(dev, &vdev->resv_regions);
kfree(vdev);
}
@@ -960,8 +939,8 @@ static struct iommu_ops viommu_ops = {
.unmap = viommu_unmap,
.iova_to_phys = viommu_iova_to_phys,
.iotlb_sync = viommu_iotlb_sync,
- .add_device = viommu_add_device,
- .remove_device = viommu_remove_device,
+ .probe_device = viommu_probe_device,
+ .release_device = viommu_release_device,
.device_group = viommu_device_group,
.get_resv_regions = viommu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
--
2.17.1
From: Joerg Roedel <[email protected]>
Convert the S390 IOMMU driver to use the probe_device() and
release_device() call-backs of iommu_ops, so that the iommu core code
does the group and sysfs setup.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/s390-iommu.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index 1137f3ddcb85..610f0828f22d 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -166,21 +166,14 @@ static void s390_iommu_detach_device(struct iommu_domain *domain,
}
}
-static int s390_iommu_add_device(struct device *dev)
+static struct iommu_device *s390_iommu_probe_device(struct device *dev)
{
- struct iommu_group *group = iommu_group_get_for_dev(dev);
struct zpci_dev *zdev = to_pci_dev(dev)->sysdata;
- if (IS_ERR(group))
- return PTR_ERR(group);
-
- iommu_group_put(group);
- iommu_device_link(&zdev->iommu_dev, dev);
-
- return 0;
+ return &zdev->iommu_dev;
}
-static void s390_iommu_remove_device(struct device *dev)
+static void s390_iommu_release_device(struct device *dev)
{
struct zpci_dev *zdev = to_pci_dev(dev)->sysdata;
struct iommu_domain *domain;
@@ -191,7 +184,7 @@ static void s390_iommu_remove_device(struct device *dev)
* to vfio-pci and completing the VFIO_SET_IOMMU ioctl (which triggers
* the attach_dev), removing the device via
* "echo 1 > /sys/bus/pci/devices/.../remove" won't trigger detach_dev,
- * only remove_device will be called via the BUS_NOTIFY_REMOVED_DEVICE
+ * only release_device will be called via the BUS_NOTIFY_REMOVED_DEVICE
* notifier.
*
* So let's call detach_dev from here if it hasn't been called before.
@@ -201,9 +194,6 @@ static void s390_iommu_remove_device(struct device *dev)
if (domain)
s390_iommu_detach_device(domain, dev);
}
-
- iommu_device_unlink(&zdev->iommu_dev, dev);
- iommu_group_remove_device(dev);
}
static int s390_iommu_update_trans(struct s390_domain *s390_domain,
@@ -373,8 +363,8 @@ static const struct iommu_ops s390_iommu_ops = {
.map = s390_iommu_map,
.unmap = s390_iommu_unmap,
.iova_to_phys = s390_iommu_iova_to_phys,
- .add_device = s390_iommu_add_device,
- .remove_device = s390_iommu_remove_device,
+ .probe_device = s390_iommu_probe_device,
+ .release_device = s390_iommu_release_device,
.device_group = generic_device_group,
.pgsize_bitmap = S390_IOMMU_PGSIZES,
};
--
2.17.1
From: Joerg Roedel <[email protected]>
This makes it easier to remove to old code-path when all drivers are
converted. As a side effect that it also fixes the error cleanup
path.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/iommu.c | 69 ++++++++++++++++++++++++++++---------------
1 file changed, 46 insertions(+), 23 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 18eb3623bd00..8be047a4808f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -218,12 +218,55 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
return ret;
}
+static int __iommu_probe_device_helper(struct device *dev)
+{
+ const struct iommu_ops *ops = dev->bus->iommu_ops;
+ struct iommu_group *group;
+ int ret;
+
+ ret = __iommu_probe_device(dev, NULL);
+ if (ret)
+ goto err_out;
+
+ /*
+ * 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(dev);
+
+ group = iommu_group_get(dev);
+ if (!group)
+ goto err_release;
+
+ if (group->default_domain)
+ ret = __iommu_attach_device(group->default_domain, dev);
+
+ iommu_group_put(group);
+
+ if (ret)
+ goto err_release;
+
+ if (ops->probe_finalize)
+ ops->probe_finalize(dev);
+
+ return 0;
+
+err_release:
+ iommu_release_device(dev);
+err_out:
+ return ret;
+
+}
+
int iommu_probe_device(struct device *dev)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
int ret;
WARN_ON(dev->iommu_group);
+
if (!ops)
return -EINVAL;
@@ -235,30 +278,10 @@ int iommu_probe_device(struct device *dev)
goto err_free_dev_param;
}
- if (ops->probe_device) {
- struct iommu_group *group;
-
- ret = __iommu_probe_device(dev, NULL);
-
- /*
- * 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.
- */
- if (!ret)
- iommu_alloc_default_domain(dev);
-
- group = iommu_group_get(dev);
- if (group && group->default_domain) {
- ret = __iommu_attach_device(group->default_domain, dev);
- iommu_group_put(group);
- }
-
- } else {
- ret = ops->add_device(dev);
- }
+ if (ops->probe_device)
+ return __iommu_probe_device_helper(dev);
+ ret = ops->add_device(dev);
if (ret)
goto err_module_put;
--
2.17.1
From: Joerg Roedel <[email protected]>
The Intel VT-d driver already has a matching function to determine the
default domain type for a device. Wire it up in intel_iommu_ops.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/intel-iommu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index ef0a5246700e..b9f905a55dda 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -6209,6 +6209,7 @@ const struct iommu_ops intel_iommu_ops = {
.dev_enable_feat = intel_iommu_dev_enable_feat,
.dev_disable_feat = intel_iommu_dev_disable_feat,
.is_attach_deferred = intel_iommu_is_attach_deferred,
+ .def_domain_type = device_def_domain_type,
.pgsize_bitmap = INTEL_IOMMU_PGSIZES,
};
--
2.17.1
From: Joerg Roedel <[email protected]>
When check_device() fails on the device, it is not handled by the
IOMMU and amd_iommu_add_device() needs to return -ENODEV.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/amd_iommu.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 504f2db75eda..3e0d27f7622e 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2157,9 +2157,12 @@ static int amd_iommu_add_device(struct device *dev)
struct amd_iommu *iommu;
int ret, devid;
- if (!check_device(dev) || get_dev_data(dev))
+ if (get_dev_data(dev))
return 0;
+ if (!check_device(dev))
+ return -ENODEV;
+
devid = get_device_id(dev);
if (devid < 0)
return devid;
--
2.17.1
From: Joerg Roedel <[email protected]>
Implement the new def_domain_type call-back for the AMD IOMMU driver.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/amd_iommu.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 20cce366e951..73b4f84cf449 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2661,6 +2661,20 @@ static void amd_iommu_iotlb_sync(struct iommu_domain *domain,
amd_iommu_flush_iotlb_all(domain);
}
+static int amd_iommu_def_domain_type(struct device *dev)
+{
+ struct iommu_dev_data *dev_data;
+
+ dev_data = get_dev_data(dev);
+ if (!dev_data)
+ return 0;
+
+ if (dev_data->iommu_v2)
+ return IOMMU_DOMAIN_IDENTITY;
+
+ return 0;
+}
+
const struct iommu_ops amd_iommu_ops = {
.capable = amd_iommu_capable,
.domain_alloc = amd_iommu_domain_alloc,
@@ -2680,6 +2694,7 @@ const struct iommu_ops amd_iommu_ops = {
.pgsize_bitmap = AMD_IOMMU_PGSIZES,
.flush_iotlb_all = amd_iommu_flush_iotlb_all,
.iotlb_sync = amd_iommu_iotlb_sync,
+ .def_domain_type = amd_iommu_def_domain_type,
};
/*****************************************************************************
--
2.17.1
On 2020/4/14 21:15, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Add a check to the bus_iommu_probe() call-path to make sure it ignores
> devices which have already been successfully probed. Then export the
> bus_iommu_probe() function so it can be used by IOMMU drivers.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/iommu.c | 6 +++++-
> include/linux/iommu.h | 1 +
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 834a45da0ed0..a2ff95424044 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1615,6 +1615,10 @@ static int probe_iommu_group(struct device *dev, void *data)
> if (!dev_iommu_get(dev))
> return -ENOMEM;
>
> + /* Device is probed already if in a group */
> + if (iommu_group_get(dev) != NULL)
Same as
if (iommu_group_get(dev))
?
By the way, do we need to put the group if device has already been
probed?
Best regards,
baolu
Hi Baolu,
On Wed, Apr 15, 2020 at 02:10:03PM +0800, Lu Baolu wrote:
> On 2020/4/14 21:15, Joerg Roedel wrote:
> > > + /* Device is probed already if in a group */
> > + if (iommu_group_get(dev) != NULL)
>
> Same as
> if (iommu_group_get(dev))
> ?
>
> By the way, do we need to put the group if device has already been
> probed?
Right, fixed both, thank you.
Regards,
Joerg
Hi Joerg,
On 14.04.2020 15:15, Joerg Roedel wrote:
> here is the second version of this patch-set. The first version with
> some more introductory text can be found here:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> Changes v1->v2:
>
> * Rebased to v5.7-rc1
>
> * Re-wrote the arm-smmu changes as suggested by Robin Murphy
>
> * Re-worked the Exynos patches to hopefully not break the
> driver anymore
Thanks for this rework. This version is much better. Works fine on
various Exynos-based boards (ARM and ARM64).
Tested-by: Marek Szyprowski <[email protected]>
Acked-by: Marek Szyprowski <[email protected]> (for Exynos and
core changes)
> * Fixed a missing mutex_unlock() reported by Marek Szyprowski,
> thanks for that.
>
> There is also a git-branch available with these patches applied:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v2
>
> Please review.
>
> Thanks,
>
> Joerg
>
> Joerg Roedel (32):
> iommu: Move default domain allocation to separate function
> iommu/amd: Implement iommu_ops->def_domain_type call-back
> iommu/vt-d: Wire up iommu_ops->def_domain_type
> iommu/amd: Remove dma_mask check from check_device()
> iommu/amd: Return -ENODEV in add_device when device is not handled by
> IOMMU
> iommu: Add probe_device() and remove_device() call-backs
> iommu: Move default domain allocation to iommu_probe_device()
> iommu: Keep a list of allocated groups in __iommu_probe_device()
> iommu: Move new probe_device path to separate function
> iommu: Split off default domain allocation from group assignment
> iommu: Move iommu_group_create_direct_mappings() out of
> iommu_group_add_device()
> iommu: Export bus_iommu_probe() and make is safe for re-probing
> iommu/amd: Remove dev_data->passthrough
> iommu/amd: Convert to probe/release_device() call-backs
> iommu/vt-d: Convert to probe/release_device() call-backs
> iommu/arm-smmu: Convert to probe/release_device() call-backs
> iommu/pamu: Convert to probe/release_device() call-backs
> iommu/s390: Convert to probe/release_device() call-backs
> iommu/virtio: Convert to probe/release_device() call-backs
> iommu/msm: Convert to probe/release_device() call-backs
> iommu/mediatek: Convert to probe/release_device() call-backs
> iommu/mediatek-v1 Convert to probe/release_device() call-backs
> iommu/qcom: Convert to probe/release_device() call-backs
> iommu/rockchip: Convert to probe/release_device() call-backs
> iommu/tegra: Convert to probe/release_device() call-backs
> iommu/renesas: Convert to probe/release_device() call-backs
> iommu/omap: Remove orphan_dev tracking
> iommu/omap: Convert to probe/release_device() call-backs
> iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
> iommu/exynos: Convert to probe/release_device() call-backs
> iommu: Remove add_device()/remove_device() code-paths
> iommu: Unexport iommu_group_get_for_dev()
>
> Sai Praneeth Prakhya (1):
> iommu: Add def_domain_type() callback in iommu_ops
>
> drivers/iommu/amd_iommu.c | 97 ++++----
> drivers/iommu/amd_iommu_types.h | 1 -
> drivers/iommu/arm-smmu-v3.c | 38 +--
> drivers/iommu/arm-smmu.c | 39 ++--
> drivers/iommu/exynos-iommu.c | 24 +-
> drivers/iommu/fsl_pamu_domain.c | 22 +-
> drivers/iommu/intel-iommu.c | 68 +-----
> drivers/iommu/iommu.c | 393 +++++++++++++++++++++++++-------
> drivers/iommu/ipmmu-vmsa.c | 60 ++---
> drivers/iommu/msm_iommu.c | 34 +--
> drivers/iommu/mtk_iommu.c | 24 +-
> drivers/iommu/mtk_iommu_v1.c | 50 ++--
> drivers/iommu/omap-iommu.c | 99 ++------
> drivers/iommu/qcom_iommu.c | 24 +-
> drivers/iommu/rockchip-iommu.c | 26 +--
> drivers/iommu/s390-iommu.c | 22 +-
> drivers/iommu/tegra-gart.c | 24 +-
> drivers/iommu/tegra-smmu.c | 31 +--
> drivers/iommu/virtio-iommu.c | 41 +---
> include/linux/iommu.h | 21 +-
> 20 files changed, 533 insertions(+), 605 deletions(-)
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Joerg,
> Hi,
>
> here is the second version of this patch-set. The first version with
> some more introductory text can be found here:
>
> https://lore.kernel.org/lkml/[email protected]/
Thanks for the continued improvements in this area!
I may have spotted a problem with setups like VMD.
The core PCI bus is set up during early boot.
Then, for the PCI bus, we reach iommu_bus_init() -> bus_iommu_probe().
In there, we call probe_iommu_group() -> dev_iommu_get() for each PCI
device, which allocates dev->iommu in each case. So far so good.
The problem is that this is the last time that we'll call dev_iommu_get().
If any PCI bus devices get added after this point, they do not get passed
to dev_iommu_get().
So when the vmd module gets loaded later, and creates more PCI devices,
we end up in iommu_bus_notifier() -> iommu_probe_device()
-> __iommu_probe_device() which does:
dev->iommu->iommu_dev = iommu_dev;
dev->iommu-> is a NULL dereference because dev_iommu_get() was never
called for this new device.
Daniel
Hi Daniel,
On Fri, 2020-04-17 at 09:03 +0800, Daniel Drake wrote:
> Hi Joerg,
>
> > Hi,
> >
> > here is the second version of this patch-set. The first version with
> > some more introductory text can be found here:
> >
> > https://lore.kernel.org/lkml/[email protected]/
>
> Thanks for the continued improvements in this area!
>
> I may have spotted a problem with setups like VMD.
>
> The core PCI bus is set up during early boot.
> Then, for the PCI bus, we reach iommu_bus_init() -> bus_iommu_probe().
> In there, we call probe_iommu_group() -> dev_iommu_get() for each PCI
> device, which allocates dev->iommu in each case. So far so good.
>
> The problem is that this is the last time that we'll call dev_iommu_get().
> If any PCI bus devices get added after this point, they do not get passed
> to dev_iommu_get().
>
> So when the vmd module gets loaded later, and creates more PCI devices,
> we end up in iommu_bus_notifier() -> iommu_probe_device()
> -> __iommu_probe_device() which does:
>
> dev->iommu->iommu_dev = iommu_dev;
>
> dev->iommu-> is a NULL dereference because dev_iommu_get() was never
> called for this new device.
>
> Daniel
>
I should have CCed you on this, but it should temporarily resolve that
issue:
https://lists.linuxfoundation.org/pipermail/iommu/2020-April/043253.html
Hi Jonathan, Hi Daniel,
On Fri, Apr 17, 2020 at 01:14:30AM +0000, Derrick, Jonathan wrote:
> Hi Daniel> I should have CCed you on this, but it should temporarily resolve that
> issue:
> https://lists.linuxfoundation.org/pipermail/iommu/2020-April/043253.html
Yes, this is an issue in the hotplug handling path which I already fixed
in my branch. With next post of this series it should work.
Regards,
Joerg
On Tue, 2020-04-14 at 15:15 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Convert the Mediatek-v1 IOMMU driver to use the probe_device() and
> release_device() call-backs of iommu_ops, so that the iommu core code
> does the group and sysfs setup.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/mtk_iommu_v1.c | 50 +++++++++++++++---------------------
> 1 file changed, 20 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index a31be05601c9..7bdd74c7cb9f 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c
> @@ -416,14 +416,12 @@ static int mtk_iommu_create_mapping(struct device *dev,
> return 0;
> }
>
> -static int mtk_iommu_add_device(struct device *dev)
> +static struct iommu_device *mtk_iommu_probe_device(struct device *dev)
> {
> struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> - struct dma_iommu_mapping *mtk_mapping;
> struct of_phandle_args iommu_spec;
> struct of_phandle_iterator it;
> struct mtk_iommu_data *data;
> - struct iommu_group *group;
> int err;
>
> of_for_each_phandle(&it, err, dev->of_node, "iommus",
> @@ -442,35 +440,28 @@ static int mtk_iommu_add_device(struct device *dev)
> }
>
> if (!fwspec || fwspec->ops != &mtk_iommu_ops)
> - return -ENODEV; /* Not a iommu client device */
> + return ERR_PTR(-ENODEV); /* Not a iommu client device */
>
> - /*
> - * This is a short-term bodge because the ARM DMA code doesn't
> - * understand multi-device groups, but we have to call into it
> - * successfully (and not just rely on a normal IOMMU API attach
> - * here) in order to set the correct DMA API ops on @dev.
> - */
> - group = iommu_group_alloc();
> - if (IS_ERR(group))
> - return PTR_ERR(group);
> + data = dev_iommu_priv_get(dev);
>
> - err = iommu_group_add_device(group, dev);
> - iommu_group_put(group);
> - if (err)
> - return err;
> + return &data->iommu;
> +}
>
> - data = dev_iommu_priv_get(dev);
> +static void mtk_iommu_probe_finalize(struct device *dev)
> +{
> + struct dma_iommu_mapping *mtk_mapping;
> + struct mtk_iommu_data *data;
> + int err;
> +
> + data = dev_iommu_priv_get(dev);
> mtk_mapping = data->dev->archdata.iommu;
> - err = arm_iommu_attach_device(dev, mtk_mapping);
> - if (err) {
> - iommu_group_remove_device(dev);
> - return err;
> - }
>
> - return iommu_device_link(&data->iommu, dev);
> + err = arm_iommu_attach_device(dev, mtk_mapping);
> + if (err)
> + dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
Hi Joerg,
Thanks very much for this patch.
This arm_iommu_attach_device is called just as we expected.
But it will fail in this callstack as the group->mutex was tried to
be re-locked...
[<c0938e8c>] (iommu_attach_device) from [<c0317590>]
(__arm_iommu_attach_device+0x34/0x90)
[<c0317590>] (__arm_iommu_attach_device) from [<c03175f8>]
(arm_iommu_attach_device+0xc/0x20)
[<c03175f8>] (arm_iommu_attach_device) from [<c09432cc>]
(mtk_iommu_probe_finalize+0x34/0x50)
[<c09432cc>] (mtk_iommu_probe_finalize) from [<c093a8ac>]
(bus_iommu_probe+0x2a8/0x2c4)
[<c093a8ac>] (bus_iommu_probe) from [<c093a950>] (bus_set_iommu
+0x88/0xd4)
[<c093a950>] (bus_set_iommu) from [<c0943c74>] (mtk_iommu_probe
+0x2f8/0x364)
> }
>
> -static void mtk_iommu_remove_device(struct device *dev)
> +static void mtk_iommu_release_device(struct device *dev)
> {
> struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> struct mtk_iommu_data *data;
> @@ -479,9 +470,6 @@ static void mtk_iommu_remove_device(struct device *dev)
> return;
>
> data = dev_iommu_priv_get(dev);
> - iommu_device_unlink(&data->iommu, dev);
> -
> - iommu_group_remove_device(dev);
> iommu_fwspec_free(dev);
> }
>
> @@ -534,8 +522,10 @@ static const struct iommu_ops mtk_iommu_ops = {
> .map = mtk_iommu_map,
> .unmap = mtk_iommu_unmap,
> .iova_to_phys = mtk_iommu_iova_to_phys,
> - .add_device = mtk_iommu_add_device,
> - .remove_device = mtk_iommu_remove_device,
> + .probe_device = mtk_iommu_probe_device,
> + .probe_finalize = mtk_iommu_probe_finalize,
> + .release_device = mtk_iommu_release_device,
> + .device_group = generic_device_group,
> .pgsize_bitmap = ~0UL << MT2701_IOMMU_PAGE_SHIFT,
> };
>
Hi,
On Fri, May 15, 2020 at 03:44:59PM +0800, Yong Wu wrote:
> On Tue, 2020-04-14 at 15:15 +0200, Joerg Roedel wrote:
> > - return iommu_device_link(&data->iommu, dev);
> > + err = arm_iommu_attach_device(dev, mtk_mapping);
> > + if (err)
> > + dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
>
>
> Hi Joerg,
>
> Thanks very much for this patch.
>
> This arm_iommu_attach_device is called just as we expected.
>
> But it will fail in this callstack as the group->mutex was tried to
> be re-locked...
>
> [<c0938e8c>] (iommu_attach_device) from [<c0317590>]
> (__arm_iommu_attach_device+0x34/0x90)
> [<c0317590>] (__arm_iommu_attach_device) from [<c03175f8>]
> (arm_iommu_attach_device+0xc/0x20)
> [<c03175f8>] (arm_iommu_attach_device) from [<c09432cc>]
> (mtk_iommu_probe_finalize+0x34/0x50)
> [<c09432cc>] (mtk_iommu_probe_finalize) from [<c093a8ac>]
> (bus_iommu_probe+0x2a8/0x2c4)
> [<c093a8ac>] (bus_iommu_probe) from [<c093a950>] (bus_set_iommu
> +0x88/0xd4)
> [<c093a950>] (bus_set_iommu) from [<c0943c74>] (mtk_iommu_probe
> +0x2f8/0x364)
Thanks for the report, is
https://lore.kernel.org/lkml/[email protected]/
The fix for this issue or is something else required?
Thanks,
Joerg
On Fri, 2020-05-15 at 12:07 +0200, Joerg Roedel wrote:
> Hi,
>
> On Fri, May 15, 2020 at 03:44:59PM +0800, Yong Wu wrote:
> > On Tue, 2020-04-14 at 15:15 +0200, Joerg Roedel wrote:
> > > - return iommu_device_link(&data->iommu, dev);
> > > + err = arm_iommu_attach_device(dev, mtk_mapping);
> > > + if (err)
> > > + dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
> >
> >
> > Hi Joerg,
> >
> > Thanks very much for this patch.
> >
> > This arm_iommu_attach_device is called just as we expected.
> >
> > But it will fail in this callstack as the group->mutex was tried to
> > be re-locked...
> >
> > [<c0938e8c>] (iommu_attach_device) from [<c0317590>]
> > (__arm_iommu_attach_device+0x34/0x90)
> > [<c0317590>] (__arm_iommu_attach_device) from [<c03175f8>]
> > (arm_iommu_attach_device+0xc/0x20)
> > [<c03175f8>] (arm_iommu_attach_device) from [<c09432cc>]
> > (mtk_iommu_probe_finalize+0x34/0x50)
> > [<c09432cc>] (mtk_iommu_probe_finalize) from [<c093a8ac>]
> > (bus_iommu_probe+0x2a8/0x2c4)
> > [<c093a8ac>] (bus_iommu_probe) from [<c093a950>] (bus_set_iommu
> > +0x88/0xd4)
> > [<c093a950>] (bus_set_iommu) from [<c0943c74>] (mtk_iommu_probe
> > +0x2f8/0x364)
>
> Thanks for the report, is
>
> https://lore.kernel.org/lkml/[email protected]/
>
> The fix for this issue or is something else required?
No. That patch only adjust the internal flow to satisfy the latest
framework, it's not for fixing this mutex issue.
Here I only reported this issue.
below is my local patch. split "dma_attach" to attach_device and
probe_finalize. About attach_device, Use the existed
__iommu_attach_group instead. Then rename from the "dma_attach" to
"probe_finalize" to do the probe_finalize job. And move it outside of
the mutex_unlock.
I'm not sure if it is right. and of course I will test if you have any
other solution. Thanks.
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1665,26 +1665,20 @@ static void probe_alloc_default_domain(struct
bus_type *bus,
}
-static int iommu_group_do_dma_attach(struct device *dev, void *data)
+static int iommu_group_do_probe_finalize(struct device *dev, void
*data)
{
struct iommu_domain *domain = data;
- const struct iommu_ops *ops;
- int ret;
-
- ret = __iommu_attach_device(domain, dev);
-
- ops = domain->ops;
+ const struct iommu_ops *ops = domain->ops;
- if (ret == 0 && ops->probe_finalize)
+ if (ops->probe_finalize)
ops->probe_finalize(dev);
-
- return ret;
+ return 0;
}
-static int __iommu_group_dma_attach(struct iommu_group *group)
+static int iommu_group_probe_finalize(struct iommu_group *group)
{
return __iommu_group_for_each_dev(group, group->default_domain,
- iommu_group_do_dma_attach);
+ iommu_group_do_probe_finalize);
}
static int iommu_do_create_direct_mappings(struct device *dev, void
*data)
@@ -1731,12 +1725,14 @@ int bus_iommu_probe(struct bus_type *bus)
iommu_group_create_direct_mappings(group);
- ret = __iommu_group_dma_attach(group);
+ ret = __iommu_attach_group(group->default_domain, group);
mutex_unlock(&group->mutex);
if (ret)
break;
+
+ iommu_group_probe_finalize(group);
}
return ret;
--
>
>
> Thanks,
>
> Joerg
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi,
On Mon, May 18, 2020 at 02:51:20PM +0800, Yong Wu wrote:
> below is my local patch. split "dma_attach" to attach_device and
> probe_finalize. About attach_device, Use the existed
> __iommu_attach_group instead. Then rename from the "dma_attach" to
> "probe_finalize" to do the probe_finalize job. And move it outside of
> the mutex_unlock.
>
> I'm not sure if it is right. and of course I will test if you have any
> other solution. Thanks.
>
>
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1665,26 +1665,20 @@ static void probe_alloc_default_domain(struct
> bus_type *bus,
>
> }
>
> -static int iommu_group_do_dma_attach(struct device *dev, void *data)
> +static int iommu_group_do_probe_finalize(struct device *dev, void
> *data)
> {
> struct iommu_domain *domain = data;
> - const struct iommu_ops *ops;
> - int ret;
> -
> - ret = __iommu_attach_device(domain, dev);
> -
> - ops = domain->ops;
> + const struct iommu_ops *ops = domain->ops;
>
> - if (ret == 0 && ops->probe_finalize)
> + if (ops->probe_finalize)
> ops->probe_finalize(dev);
> -
> - return ret;
> + return 0;
> }
>
> -static int __iommu_group_dma_attach(struct iommu_group *group)
> +static int iommu_group_probe_finalize(struct iommu_group *group)
> {
> return __iommu_group_for_each_dev(group, group->default_domain,
> - iommu_group_do_dma_attach);
> + iommu_group_do_probe_finalize);
> }
>
> static int iommu_do_create_direct_mappings(struct device *dev, void
> *data)
> @@ -1731,12 +1725,14 @@ int bus_iommu_probe(struct bus_type *bus)
>
> iommu_group_create_direct_mappings(group);
>
> - ret = __iommu_group_dma_attach(group);
> + ret = __iommu_attach_group(group->default_domain, group);
>
> mutex_unlock(&group->mutex);
>
> if (ret)
> break;
> +
> + iommu_group_probe_finalize(group);
> }
>
> return ret;
> --
Yes, I think moving the probe_finalize call out of the group->mutex
section is the right fix for this issue.
Thanks for reporting it and working on a fix.
Regards,
Joerg
On Tue Apr 14 20, Joerg Roedel wrote:
>Hi,
>
>here is the second version of this patch-set. The first version with
>some more introductory text can be found here:
>
> https://lore.kernel.org/lkml/[email protected]/
>
>Changes v1->v2:
>
> * Rebased to v5.7-rc1
>
> * Re-wrote the arm-smmu changes as suggested by Robin Murphy
>
> * Re-worked the Exynos patches to hopefully not break the
> driver anymore
>
> * Fixed a missing mutex_unlock() reported by Marek Szyprowski,
> thanks for that.
>
>There is also a git-branch available with these patches applied:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v2
>
>Please review.
>
>Thanks,
>
> Joerg
>
>Joerg Roedel (32):
> iommu: Move default domain allocation to separate function
> iommu/amd: Implement iommu_ops->def_domain_type call-back
> iommu/vt-d: Wire up iommu_ops->def_domain_type
> iommu/amd: Remove dma_mask check from check_device()
> iommu/amd: Return -ENODEV in add_device when device is not handled by
> IOMMU
> iommu: Add probe_device() and remove_device() call-backs
> iommu: Move default domain allocation to iommu_probe_device()
> iommu: Keep a list of allocated groups in __iommu_probe_device()
> iommu: Move new probe_device path to separate function
> iommu: Split off default domain allocation from group assignment
> iommu: Move iommu_group_create_direct_mappings() out of
> iommu_group_add_device()
> iommu: Export bus_iommu_probe() and make is safe for re-probing
> iommu/amd: Remove dev_data->passthrough
> iommu/amd: Convert to probe/release_device() call-backs
> iommu/vt-d: Convert to probe/release_device() call-backs
> iommu/arm-smmu: Convert to probe/release_device() call-backs
> iommu/pamu: Convert to probe/release_device() call-backs
> iommu/s390: Convert to probe/release_device() call-backs
> iommu/virtio: Convert to probe/release_device() call-backs
> iommu/msm: Convert to probe/release_device() call-backs
> iommu/mediatek: Convert to probe/release_device() call-backs
> iommu/mediatek-v1 Convert to probe/release_device() call-backs
> iommu/qcom: Convert to probe/release_device() call-backs
> iommu/rockchip: Convert to probe/release_device() call-backs
> iommu/tegra: Convert to probe/release_device() call-backs
> iommu/renesas: Convert to probe/release_device() call-backs
> iommu/omap: Remove orphan_dev tracking
> iommu/omap: Convert to probe/release_device() call-backs
> iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
> iommu/exynos: Convert to probe/release_device() call-backs
> iommu: Remove add_device()/remove_device() code-paths
> iommu: Unexport iommu_group_get_for_dev()
>
>Sai Praneeth Prakhya (1):
> iommu: Add def_domain_type() callback in iommu_ops
>
> drivers/iommu/amd_iommu.c | 97 ++++----
> drivers/iommu/amd_iommu_types.h | 1 -
> drivers/iommu/arm-smmu-v3.c | 38 +--
> drivers/iommu/arm-smmu.c | 39 ++--
> drivers/iommu/exynos-iommu.c | 24 +-
> drivers/iommu/fsl_pamu_domain.c | 22 +-
> drivers/iommu/intel-iommu.c | 68 +-----
> drivers/iommu/iommu.c | 393 +++++++++++++++++++++++++-------
> drivers/iommu/ipmmu-vmsa.c | 60 ++---
> drivers/iommu/msm_iommu.c | 34 +--
> drivers/iommu/mtk_iommu.c | 24 +-
> drivers/iommu/mtk_iommu_v1.c | 50 ++--
> drivers/iommu/omap-iommu.c | 99 ++------
> drivers/iommu/qcom_iommu.c | 24 +-
> drivers/iommu/rockchip-iommu.c | 26 +--
> drivers/iommu/s390-iommu.c | 22 +-
> drivers/iommu/tegra-gart.c | 24 +-
> drivers/iommu/tegra-smmu.c | 31 +--
> drivers/iommu/virtio-iommu.c | 41 +---
> include/linux/iommu.h | 21 +-
> 20 files changed, 533 insertions(+), 605 deletions(-)
>
>--
>2.17.1
>
>_______________________________________________
>iommu mailing list
>[email protected]
>https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
Hi Joerg,
With this patchset, I have an epyc system where if I boot with
iommu=nopt and force a dump I will see some io page faults for a nic
on the system. The vmcore is harvested and the system reboots. I
haven't reproduced it on other systems yet, but without the patchset I
don't see the io page faults during the kdump.
Regards,
Jerry
On Fri May 29 20, Jerry Snitselaar wrote:
>On Tue Apr 14 20, Joerg Roedel wrote:
>>Hi,
>>
>>here is the second version of this patch-set. The first version with
>>some more introductory text can be found here:
>>
>> https://lore.kernel.org/lkml/[email protected]/
>>
>>Changes v1->v2:
>>
>> * Rebased to v5.7-rc1
>>
>> * Re-wrote the arm-smmu changes as suggested by Robin Murphy
>>
>> * Re-worked the Exynos patches to hopefully not break the
>> driver anymore
>>
>> * Fixed a missing mutex_unlock() reported by Marek Szyprowski,
>> thanks for that.
>>
>>There is also a git-branch available with these patches applied:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v2
>>
>>Please review.
>>
>>Thanks,
>>
>> Joerg
>>
>>Joerg Roedel (32):
>> iommu: Move default domain allocation to separate function
>> iommu/amd: Implement iommu_ops->def_domain_type call-back
>> iommu/vt-d: Wire up iommu_ops->def_domain_type
>> iommu/amd: Remove dma_mask check from check_device()
>> iommu/amd: Return -ENODEV in add_device when device is not handled by
>> IOMMU
>> iommu: Add probe_device() and remove_device() call-backs
>> iommu: Move default domain allocation to iommu_probe_device()
>> iommu: Keep a list of allocated groups in __iommu_probe_device()
>> iommu: Move new probe_device path to separate function
>> iommu: Split off default domain allocation from group assignment
>> iommu: Move iommu_group_create_direct_mappings() out of
>> iommu_group_add_device()
>> iommu: Export bus_iommu_probe() and make is safe for re-probing
>> iommu/amd: Remove dev_data->passthrough
>> iommu/amd: Convert to probe/release_device() call-backs
>> iommu/vt-d: Convert to probe/release_device() call-backs
>> iommu/arm-smmu: Convert to probe/release_device() call-backs
>> iommu/pamu: Convert to probe/release_device() call-backs
>> iommu/s390: Convert to probe/release_device() call-backs
>> iommu/virtio: Convert to probe/release_device() call-backs
>> iommu/msm: Convert to probe/release_device() call-backs
>> iommu/mediatek: Convert to probe/release_device() call-backs
>> iommu/mediatek-v1 Convert to probe/release_device() call-backs
>> iommu/qcom: Convert to probe/release_device() call-backs
>> iommu/rockchip: Convert to probe/release_device() call-backs
>> iommu/tegra: Convert to probe/release_device() call-backs
>> iommu/renesas: Convert to probe/release_device() call-backs
>> iommu/omap: Remove orphan_dev tracking
>> iommu/omap: Convert to probe/release_device() call-backs
>> iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
>> iommu/exynos: Convert to probe/release_device() call-backs
>> iommu: Remove add_device()/remove_device() code-paths
>> iommu: Unexport iommu_group_get_for_dev()
>>
>>Sai Praneeth Prakhya (1):
>> iommu: Add def_domain_type() callback in iommu_ops
>>
>>drivers/iommu/amd_iommu.c | 97 ++++----
>>drivers/iommu/amd_iommu_types.h | 1 -
>>drivers/iommu/arm-smmu-v3.c | 38 +--
>>drivers/iommu/arm-smmu.c | 39 ++--
>>drivers/iommu/exynos-iommu.c | 24 +-
>>drivers/iommu/fsl_pamu_domain.c | 22 +-
>>drivers/iommu/intel-iommu.c | 68 +-----
>>drivers/iommu/iommu.c | 393 +++++++++++++++++++++++++-------
>>drivers/iommu/ipmmu-vmsa.c | 60 ++---
>>drivers/iommu/msm_iommu.c | 34 +--
>>drivers/iommu/mtk_iommu.c | 24 +-
>>drivers/iommu/mtk_iommu_v1.c | 50 ++--
>>drivers/iommu/omap-iommu.c | 99 ++------
>>drivers/iommu/qcom_iommu.c | 24 +-
>>drivers/iommu/rockchip-iommu.c | 26 +--
>>drivers/iommu/s390-iommu.c | 22 +-
>>drivers/iommu/tegra-gart.c | 24 +-
>>drivers/iommu/tegra-smmu.c | 31 +--
>>drivers/iommu/virtio-iommu.c | 41 +---
>>include/linux/iommu.h | 21 +-
>>20 files changed, 533 insertions(+), 605 deletions(-)
>>
>>--
>>2.17.1
>>
>>_______________________________________________
>>iommu mailing list
>>[email protected]
>>https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>
>
>Hi Joerg,
>
>With this patchset, I have an epyc system where if I boot with
>iommu=nopt and force a dump I will see some io page faults for a nic
>on the system. The vmcore is harvested and the system reboots. I
>haven't reproduced it on other systems yet, but without the patchset I
>don't see the io page faults during the kdump.
>
>Regards,
>Jerry
I just hit an issue on a separate intel based system (kdump iommu=nopt),
where it panics in during intel_iommu_attach_device, in is_aux_domain,
due to device_domain_info being DEFER_DEVICE_DOMAIN_INFO. That doesn't
get set to a valid address until the domain_add_dev_info call.
Is it as simple as the following?
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 29d3940847d3..f1bbeed46a4c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5053,8 +5053,8 @@ is_aux_domain(struct device *dev, struct iommu_domain *domain)
{
struct device_domain_info *info = dev->archdata.iommu;
- return info && info->auxd_enabled &&
- domain->type == IOMMU_DOMAIN_UNMANAGED;
+ return info && info != DEFER_DEVICE_DOMAIN_INFO &&
+ info->auxd_enabled && domain->type == IOMMU_DOMAIN_UNMANAGED;
}
static void auxiliary_link_device(struct dmar_domain *domain,
Regards,
Jerry
On Mon Jun 01 20, Jerry Snitselaar wrote:
>On Fri May 29 20, Jerry Snitselaar wrote:
>>On Tue Apr 14 20, Joerg Roedel wrote:
>>>Hi,
>>>
>>>here is the second version of this patch-set. The first version with
>>>some more introductory text can be found here:
>>>
>>> https://lore.kernel.org/lkml/[email protected]/
>>>
>>>Changes v1->v2:
>>>
>>> * Rebased to v5.7-rc1
>>>
>>> * Re-wrote the arm-smmu changes as suggested by Robin Murphy
>>>
>>> * Re-worked the Exynos patches to hopefully not break the
>>> driver anymore
>>>
>>> * Fixed a missing mutex_unlock() reported by Marek Szyprowski,
>>> thanks for that.
>>>
>>>There is also a git-branch available with these patches applied:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v2
>>>
>>>Please review.
>>>
>>>Thanks,
>>>
>>> Joerg
>>>
>>>Joerg Roedel (32):
>>>iommu: Move default domain allocation to separate function
>>>iommu/amd: Implement iommu_ops->def_domain_type call-back
>>>iommu/vt-d: Wire up iommu_ops->def_domain_type
>>>iommu/amd: Remove dma_mask check from check_device()
>>>iommu/amd: Return -ENODEV in add_device when device is not handled by
>>> IOMMU
>>>iommu: Add probe_device() and remove_device() call-backs
>>>iommu: Move default domain allocation to iommu_probe_device()
>>>iommu: Keep a list of allocated groups in __iommu_probe_device()
>>>iommu: Move new probe_device path to separate function
>>>iommu: Split off default domain allocation from group assignment
>>>iommu: Move iommu_group_create_direct_mappings() out of
>>> iommu_group_add_device()
>>>iommu: Export bus_iommu_probe() and make is safe for re-probing
>>>iommu/amd: Remove dev_data->passthrough
>>>iommu/amd: Convert to probe/release_device() call-backs
>>>iommu/vt-d: Convert to probe/release_device() call-backs
>>>iommu/arm-smmu: Convert to probe/release_device() call-backs
>>>iommu/pamu: Convert to probe/release_device() call-backs
>>>iommu/s390: Convert to probe/release_device() call-backs
>>>iommu/virtio: Convert to probe/release_device() call-backs
>>>iommu/msm: Convert to probe/release_device() call-backs
>>>iommu/mediatek: Convert to probe/release_device() call-backs
>>>iommu/mediatek-v1 Convert to probe/release_device() call-backs
>>>iommu/qcom: Convert to probe/release_device() call-backs
>>>iommu/rockchip: Convert to probe/release_device() call-backs
>>>iommu/tegra: Convert to probe/release_device() call-backs
>>>iommu/renesas: Convert to probe/release_device() call-backs
>>>iommu/omap: Remove orphan_dev tracking
>>>iommu/omap: Convert to probe/release_device() call-backs
>>>iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
>>>iommu/exynos: Convert to probe/release_device() call-backs
>>>iommu: Remove add_device()/remove_device() code-paths
>>>iommu: Unexport iommu_group_get_for_dev()
>>>
>>>Sai Praneeth Prakhya (1):
>>>iommu: Add def_domain_type() callback in iommu_ops
>>>
>>>drivers/iommu/amd_iommu.c | 97 ++++----
>>>drivers/iommu/amd_iommu_types.h | 1 -
>>>drivers/iommu/arm-smmu-v3.c | 38 +--
>>>drivers/iommu/arm-smmu.c | 39 ++--
>>>drivers/iommu/exynos-iommu.c | 24 +-
>>>drivers/iommu/fsl_pamu_domain.c | 22 +-
>>>drivers/iommu/intel-iommu.c | 68 +-----
>>>drivers/iommu/iommu.c | 393 +++++++++++++++++++++++++-------
>>>drivers/iommu/ipmmu-vmsa.c | 60 ++---
>>>drivers/iommu/msm_iommu.c | 34 +--
>>>drivers/iommu/mtk_iommu.c | 24 +-
>>>drivers/iommu/mtk_iommu_v1.c | 50 ++--
>>>drivers/iommu/omap-iommu.c | 99 ++------
>>>drivers/iommu/qcom_iommu.c | 24 +-
>>>drivers/iommu/rockchip-iommu.c | 26 +--
>>>drivers/iommu/s390-iommu.c | 22 +-
>>>drivers/iommu/tegra-gart.c | 24 +-
>>>drivers/iommu/tegra-smmu.c | 31 +--
>>>drivers/iommu/virtio-iommu.c | 41 +---
>>>include/linux/iommu.h | 21 +-
>>>20 files changed, 533 insertions(+), 605 deletions(-)
>>>
>>>--
>>>2.17.1
>>>
>>>_______________________________________________
>>>iommu mailing list
>>>[email protected]
>>>https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>>
>>
>>Hi Joerg,
>>
>>With this patchset, I have an epyc system where if I boot with
>>iommu=nopt and force a dump I will see some io page faults for a nic
>>on the system. The vmcore is harvested and the system reboots. I
>>haven't reproduced it on other systems yet, but without the patchset I
>>don't see the io page faults during the kdump.
>>
>>Regards,
>>Jerry
>
>I just hit an issue on a separate intel based system (kdump iommu=nopt),
>where it panics in during intel_iommu_attach_device, in is_aux_domain,
>due to device_domain_info being DEFER_DEVICE_DOMAIN_INFO. That doesn't
>get set to a valid address until the domain_add_dev_info call.
>
>Is it as simple as the following?
>
>diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>index 29d3940847d3..f1bbeed46a4c 100644
>--- a/drivers/iommu/intel-iommu.c
>+++ b/drivers/iommu/intel-iommu.c
>@@ -5053,8 +5053,8 @@ is_aux_domain(struct device *dev, struct iommu_domain *domain)
> {
> struct device_domain_info *info = dev->archdata.iommu;
>- return info && info->auxd_enabled &&
>- domain->type == IOMMU_DOMAIN_UNMANAGED;
>+ return info && info != DEFER_DEVICE_DOMAIN_INFO &&
>+ info->auxd_enabled && domain->type == IOMMU_DOMAIN_UNMANAGED;
> }
> static void auxiliary_link_device(struct dmar_domain *domain,
>
>
>Regards,
>Jerry
>
With the patch, I avoid the panic, but I'm seeing an issue similar to the epyc system.
I'm getting dmar faults from a couple of nics and the hp ilo. The addresses in question
were in e820 reserved sections, but there aren't rmrr covering those addresses. The system
manages to harvest the vmcore and reboot like the epyc. Without the patches I don't see
the dmar faults. I needed to give this system back, but I'll try to poke at it some more
in the next couple of days.
Regards,
Jerry
Hi Jerry,
On 6/1/20 6:42 PM, Jerry Snitselaar wrote:
>>
>> Hi Joerg,
>>
>> With this patchset, I have an epyc system where if I boot with
>> iommu=nopt and force a dump I will see some io page faults for a nic
>> on the system. The vmcore is harvested and the system reboots. I
>> haven't reproduced it on other systems yet, but without the patchset I
>> don't see the io page faults during the kdump.
>>
>> Regards,
>> Jerry
>
> I just hit an issue on a separate intel based system (kdump iommu=nopt),
> where it panics in during intel_iommu_attach_device, in is_aux_domain,
> due to device_domain_info being DEFER_DEVICE_DOMAIN_INFO. That doesn't
> get set to a valid address until the domain_add_dev_info call.
>
> Is it as simple as the following?
I guess you won't hit this issue if you use iommu/next branch of Joerg's
tree. We've changed to use a generic helper to retrieve the valid per
device iommu data or NULL (if there's no).
Best regards,
baolu
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 29d3940847d3..f1bbeed46a4c 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -5053,8 +5053,8 @@ is_aux_domain(struct device *dev, struct
> iommu_domain *domain)
> {
> struct device_domain_info *info = dev->archdata.iommu;
>
> - return info && info->auxd_enabled &&
> - domain->type == IOMMU_DOMAIN_UNMANAGED;
> + return info && info != DEFER_DEVICE_DOMAIN_INFO &&
> + info->auxd_enabled && domain->type ==
> IOMMU_DOMAIN_UNMANAGED;
> }
>
> static void auxiliary_link_device(struct dmar_domain *domain,
>
>
> Regards,
> Jerry
Hi Jerry,
On 6/1/20 9:17 PM, Jerry Snitselaar wrote:
> On Mon Jun 01 20, Jerry Snitselaar wrote:
>> On Fri May 29 20, Jerry Snitselaar wrote:
>>> On Tue Apr 14 20, Joerg Roedel wrote:
>>>> Hi,
>>>>
>>>> here is the second version of this patch-set. The first version with
>>>> some more introductory text can be found here:
>>>>
>>>> https://lore.kernel.org/lkml/[email protected]/
>>>>
>>>> Changes v1->v2:
>>>>
>>>> * Rebased to v5.7-rc1
>>>>
>>>> * Re-wrote the arm-smmu changes as suggested by Robin Murphy
>>>>
>>>> * Re-worked the Exynos patches to hopefully not break the
>>>> driver anymore
>>>>
>>>> * Fixed a missing mutex_unlock() reported by Marek Szyprowski,
>>>> thanks for that.
>>>>
>>>> There is also a git-branch available with these patches applied:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v2
>>>>
>>>>
>>>> Please review.
>>>>
>>>> Thanks,
>>>>
>>>> Joerg
>>>>
>>>> Joerg Roedel (32):
>>>> iommu: Move default domain allocation to separate function
>>>> iommu/amd: Implement iommu_ops->def_domain_type call-back
>>>> iommu/vt-d: Wire up iommu_ops->def_domain_type
>>>> iommu/amd: Remove dma_mask check from check_device()
>>>> iommu/amd: Return -ENODEV in add_device when device is not handled by
>>>> IOMMU
>>>> iommu: Add probe_device() and remove_device() call-backs
>>>> iommu: Move default domain allocation to iommu_probe_device()
>>>> iommu: Keep a list of allocated groups in __iommu_probe_device()
>>>> iommu: Move new probe_device path to separate function
>>>> iommu: Split off default domain allocation from group assignment
>>>> iommu: Move iommu_group_create_direct_mappings() out of
>>>> iommu_group_add_device()
>>>> iommu: Export bus_iommu_probe() and make is safe for re-probing
>>>> iommu/amd: Remove dev_data->passthrough
>>>> iommu/amd: Convert to probe/release_device() call-backs
>>>> iommu/vt-d: Convert to probe/release_device() call-backs
>>>> iommu/arm-smmu: Convert to probe/release_device() call-backs
>>>> iommu/pamu: Convert to probe/release_device() call-backs
>>>> iommu/s390: Convert to probe/release_device() call-backs
>>>> iommu/virtio: Convert to probe/release_device() call-backs
>>>> iommu/msm: Convert to probe/release_device() call-backs
>>>> iommu/mediatek: Convert to probe/release_device() call-backs
>>>> iommu/mediatek-v1 Convert to probe/release_device() call-backs
>>>> iommu/qcom: Convert to probe/release_device() call-backs
>>>> iommu/rockchip: Convert to probe/release_device() call-backs
>>>> iommu/tegra: Convert to probe/release_device() call-backs
>>>> iommu/renesas: Convert to probe/release_device() call-backs
>>>> iommu/omap: Remove orphan_dev tracking
>>>> iommu/omap: Convert to probe/release_device() call-backs
>>>> iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
>>>> iommu/exynos: Convert to probe/release_device() call-backs
>>>> iommu: Remove add_device()/remove_device() code-paths
>>>> iommu: Unexport iommu_group_get_for_dev()
>>>>
>>>> Sai Praneeth Prakhya (1):
>>>> iommu: Add def_domain_type() callback in iommu_ops
>>>>
>>>> drivers/iommu/amd_iommu.c | 97 ++++----
>>>> drivers/iommu/amd_iommu_types.h | 1 -
>>>> drivers/iommu/arm-smmu-v3.c | 38 +--
>>>> drivers/iommu/arm-smmu.c | 39 ++--
>>>> drivers/iommu/exynos-iommu.c | 24 +-
>>>> drivers/iommu/fsl_pamu_domain.c | 22 +-
>>>> drivers/iommu/intel-iommu.c | 68 +-----
>>>> drivers/iommu/iommu.c | 393 +++++++++++++++++++++++++-------
>>>> drivers/iommu/ipmmu-vmsa.c | 60 ++---
>>>> drivers/iommu/msm_iommu.c | 34 +--
>>>> drivers/iommu/mtk_iommu.c | 24 +-
>>>> drivers/iommu/mtk_iommu_v1.c | 50 ++--
>>>> drivers/iommu/omap-iommu.c | 99 ++------
>>>> drivers/iommu/qcom_iommu.c | 24 +-
>>>> drivers/iommu/rockchip-iommu.c | 26 +--
>>>> drivers/iommu/s390-iommu.c | 22 +-
>>>> drivers/iommu/tegra-gart.c | 24 +-
>>>> drivers/iommu/tegra-smmu.c | 31 +--
>>>> drivers/iommu/virtio-iommu.c | 41 +---
>>>> include/linux/iommu.h | 21 +-
>>>> 20 files changed, 533 insertions(+), 605 deletions(-)
>>>>
>>>> --
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> iommu mailing list
>>>> [email protected]
>>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>>>
>>>
>>> Hi Joerg,
>>>
>>> With this patchset, I have an epyc system where if I boot with
>>> iommu=nopt and force a dump I will see some io page faults for a nic
>>> on the system. The vmcore is harvested and the system reboots. I
>>> haven't reproduced it on other systems yet, but without the patchset I
>>> don't see the io page faults during the kdump.
>>>
>>> Regards,
>>> Jerry
>>
>> I just hit an issue on a separate intel based system (kdump iommu=nopt),
>> where it panics in during intel_iommu_attach_device, in is_aux_domain,
>> due to device_domain_info being DEFER_DEVICE_DOMAIN_INFO. That doesn't
>> get set to a valid address until the domain_add_dev_info call.
>>
>> Is it as simple as the following?
>>
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 29d3940847d3..f1bbeed46a4c 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -5053,8 +5053,8 @@ is_aux_domain(struct device *dev, struct
>> iommu_domain *domain)
>> {
>> struct device_domain_info *info = dev->archdata.iommu;
>> - return info && info->auxd_enabled &&
>> - domain->type == IOMMU_DOMAIN_UNMANAGED;
>> + return info && info != DEFER_DEVICE_DOMAIN_INFO &&
>> + info->auxd_enabled && domain->type ==
>> IOMMU_DOMAIN_UNMANAGED;
>> }
>> static void auxiliary_link_device(struct dmar_domain *domain,
>>
>>
>> Regards,
>> Jerry
>>
>
> With the patch, I avoid the panic, but I'm seeing an issue similar to
> the epyc system.
> I'm getting dmar faults from a couple of nics and the hp ilo. The
> addresses in question
> were in e820 reserved sections, but there aren't rmrr covering those
> addresses. The system
> manages to harvest the vmcore and reboot like the epyc. Without the
> patches I don't see
> the dmar faults. I needed to give this system back, but I'll try to poke
> at it some more
> in the next couple of days.
Thanks and looking forward to further debugging information.
Best regards,
baolu
On Tue Jun 02 20, Lu Baolu wrote:
>Hi Jerry,
>
>On 6/1/20 6:42 PM, Jerry Snitselaar wrote:
>>>
>>>Hi Joerg,
>>>
>>>With this patchset, I have an epyc system where if I boot with
>>>iommu=nopt and force a dump I will see some io page faults for a nic
>>>on the system. The vmcore is harvested and the system reboots. I
>>>haven't reproduced it on other systems yet, but without the patchset I
>>>don't see the io page faults during the kdump.
>>>
>>>Regards,
>>>Jerry
>>
>>I just hit an issue on a separate intel based system (kdump iommu=nopt),
>>where it panics in during intel_iommu_attach_device, in is_aux_domain,
>>due to device_domain_info being DEFER_DEVICE_DOMAIN_INFO. That doesn't
>>get set to a valid address until the domain_add_dev_info call.
>>
>>Is it as simple as the following?
>
>I guess you won't hit this issue if you use iommu/next branch of Joerg's
>tree. We've changed to use a generic helper to retrieve the valid per
>device iommu data or NULL (if there's no).
>
>Best regards,
>baolu
>
Yeah, that will solve the panic.
>>
>>diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>>index 29d3940847d3..f1bbeed46a4c 100644
>>--- a/drivers/iommu/intel-iommu.c
>>+++ b/drivers/iommu/intel-iommu.c
>>@@ -5053,8 +5053,8 @@ is_aux_domain(struct device *dev, struct
>>iommu_domain *domain)
>> {
>> struct device_domain_info *info = dev->archdata.iommu;
>>
>>- return info && info->auxd_enabled &&
>>- domain->type == IOMMU_DOMAIN_UNMANAGED;
>>+ return info && info != DEFER_DEVICE_DOMAIN_INFO &&
>>+ info->auxd_enabled && domain->type ==
>>IOMMU_DOMAIN_UNMANAGED;
>> }
>>
>> static void auxiliary_link_device(struct dmar_domain *domain,
>>
>>
>>Regards,
>>Jerry
>_______________________________________________
>iommu mailing list
>[email protected]
>https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi Jerry,
On Mon, Jun 01, 2020 at 05:02:36PM -0700, Jerry Snitselaar wrote:
>
> Yeah, that will solve the panic.
>
If you still see the kdump faults, can you please try with the attached
diff? I was not able to reproduce them in my setup.
Regards,
Joerg
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b5ea203f6c68..5a6d509f72b6 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1680,8 +1680,12 @@ static void probe_alloc_default_domain(struct bus_type *bus,
static int iommu_group_do_dma_attach(struct device *dev, void *data)
{
struct iommu_domain *domain = data;
+ int ret = 0;
- return __iommu_attach_device(domain, dev);
+ if (!iommu_is_attach_deferred(group->domain, dev))
+ ret = __iommu_attach_device(group->domain, dev);
+
+ return ret;
}
static int __iommu_group_dma_attach(struct iommu_group *group)
On Tue Jun 02 20, Joerg Roedel wrote:
>Hi Jerry,
>
>On Mon, Jun 01, 2020 at 05:02:36PM -0700, Jerry Snitselaar wrote:
>>
>> Yeah, that will solve the panic.
>>
>
>If you still see the kdump faults, can you please try with the attached
>diff? I was not able to reproduce them in my setup.
>
>Regards,
>
> Joerg
>
I have another hp proliant server now, and reproduced. I will have the
patch below tested shortly. Minor change, I switched group->domain to
domain since group isn't an argument, and *data being passed in comes
from group->domain anyways.
>diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>index b5ea203f6c68..5a6d509f72b6 100644
>--- a/drivers/iommu/iommu.c
>+++ b/drivers/iommu/iommu.c
>@@ -1680,8 +1680,12 @@ static void probe_alloc_default_domain(struct bus_type *bus,
> static int iommu_group_do_dma_attach(struct device *dev, void *data)
> {
> struct iommu_domain *domain = data;
>+ int ret = 0;
>
>- return __iommu_attach_device(domain, dev);
>+ if (!iommu_is_attach_deferred(group->domain, dev))
>+ ret = __iommu_attach_device(group->domain, dev);
>+
>+ return ret;
> }
>
> static int __iommu_group_dma_attach(struct iommu_group *group)
>_______________________________________________
>iommu mailing list
>[email protected]
>https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
On Tue Jun 02 20, Jerry Snitselaar wrote:
>On Tue Jun 02 20, Joerg Roedel wrote:
>>Hi Jerry,
>>
>>On Mon, Jun 01, 2020 at 05:02:36PM -0700, Jerry Snitselaar wrote:
>>>
>>>Yeah, that will solve the panic.
>>>
>>
>>If you still see the kdump faults, can you please try with the attached
>>diff? I was not able to reproduce them in my setup.
>>
>>Regards,
>>
>> Joerg
>>
>
>I have another hp proliant server now, and reproduced. I will have the
>patch below tested shortly. Minor change, I switched group->domain to
>domain since group isn't an argument, and *data being passed in comes
>from group->domain anyways.
>
Looks like it solves problem for both the epyc system, and the hp proliant
server,
>>diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>index b5ea203f6c68..5a6d509f72b6 100644
>>--- a/drivers/iommu/iommu.c
>>+++ b/drivers/iommu/iommu.c
>>@@ -1680,8 +1680,12 @@ static void probe_alloc_default_domain(struct bus_type *bus,
>>static int iommu_group_do_dma_attach(struct device *dev, void *data)
>>{
>> struct iommu_domain *domain = data;
>>+ int ret = 0;
>>
>>- return __iommu_attach_device(domain, dev);
>>+ if (!iommu_is_attach_deferred(group->domain, dev))
>>+ ret = __iommu_attach_device(group->domain, dev);
>>+
>>+ return ret;
>>}
>>
>>static int __iommu_group_dma_attach(struct iommu_group *group)
>>_______________________________________________
>>iommu mailing list
>>[email protected]
>>https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>