[PATCH 0/3] iommu/mediatek-v1: fix the driver + mt6572 support

[PATCH 0/3] iommu/mediatek-v1: fix the driver + mt6572 support

[Roman Vivchar via B4 Relay]

This patch series fixes silently broken IOMMU v1 driver and adds mt6572
SoC support.

It appears that currently the driver is not working at all... IOMMU core
will silently reject the component bind unless there's of_xlate callback.
The fix is pretty straightforward - provide of_xlate callback (based on
mtk_iommu gen 2 driver) to let IOMMU attach devices.

This makes DRM subsystem use IOMMU like all gen 2 SoCs do.
It can be verified by reading overlay engine input address.
Before:
~ # devmem 0x14007040 32
0xBB900000

After:
~ # devmem 0x14007040 32
0x00000000

Note the changes weren't tested on mt2701, because I don't have such
hardware.

It's worth mentioning that some bits may apply to mt6582 and 6589,
however, they seem to have L2 cache for IOMMU, therefore patch 3 has
MT6572 IOMMU type, and not MT65XX.

Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
Roman Vivchar (3):
      dt-bindings: iommu: mediatek: add mt6572
      iommu/mediatek-v1: fix IOMMU device binding
      iommu/mediatek-v1: add mt6572 support

 .../devicetree/bindings/iommu/mediatek,iommu.yaml  |   3 +
 drivers/iommu/mtk_iommu_v1.c                       | 158 ++++++++++++---------
 .../memory/mediatek,mt6572-memory-port.h           |  26 ++++
 3 files changed, 123 insertions(+), 64 deletions(-)
---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260724-6572-iommu-dae78adaa65c

Best regards,
--  
Roman Vivchar <rva333@protonmail.com>

[PATCH 1/3] dt-bindings: iommu: mediatek: add mt6572

[Roman Vivchar via B4 Relay]

From: Roman Vivchar <rva333@protonmail.com>

Add compatible string and header file for mt6572 IOMMU support.

Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
 .../devicetree/bindings/iommu/mediatek,iommu.yaml  |  3 +++
 .../memory/mediatek,mt6572-memory-port.h           | 26 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index 79c573c47b08..d0ac4bd71911 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -72,6 +72,7 @@ properties:
       - enum:
           - mediatek,mt2701-m4u  # generation one
           - mediatek,mt2712-m4u  # generation two
+          - mediatek,mt6572-m4u  # generation one
           - mediatek,mt6779-m4u  # generation two
           - mediatek,mt6795-m4u  # generation two
           - mediatek,mt6893-iommu-mm         # generation two
@@ -134,6 +135,7 @@ properties:
       dt-binding/memory/mediatek,mt8189-memory-port.h for mt8189,
       dt-binding/memory/mt2701-larb-port.h for mt2701 and mt7623,
       dt-binding/memory/mt2712-larb-port.h for mt2712,
+      dt-binding/memory/mediatek,mt6572-memory-port.h for mt6572,
       dt-binding/memory/mt6779-larb-port.h for mt6779,
       dt-binding/memory/mt6795-larb-port.h for mt6795,
       dt-binding/memory/mediatek,mt6893-memory-port.h for mt6893,
@@ -162,6 +164,7 @@ allOf:
             enum:
               - mediatek,mt2701-m4u
               - mediatek,mt2712-m4u
+              - mediatek,mt6572-m4u
               - mediatek,mt6795-m4u
               - mediatek,mt6893-iommu-mm
               - mediatek,mt8173-m4u
diff --git a/include/dt-bindings/memory/mediatek,mt6572-memory-port.h b/include/dt-bindings/memory/mediatek,mt6572-memory-port.h
new file mode 100644
index 000000000000..6f8ddc98f1b1
--- /dev/null
+++ b/include/dt-bindings/memory/mediatek,mt6572-memory-port.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2026 Roman Vivchar <rva333@protonmail.com>
+ */
+
+#ifndef _DT_BINDINGS_MEMORY_MT6572_MEMORY_PORT_H_
+#define _DT_BINDINGS_MEMORY_MT6572_MEMORY_PORT_H_
+
+#define M4U_PORT_DISP_OVL		0
+#define M4U_PORT_DISP_RDMA		1
+#define M4U_PORT_DISP_WDMA		2
+#define M4U_PORT_DISP_DBI		3
+#define M4U_PORT_CAM_WDMA		4
+#define M4U_PORT_CMDQ			5
+#define M4U_PORT_VENC_BSDMA_VDEC_POST0	6
+#define M4U_PORT_MDP_RDMA		7
+#define M4U_PORT_MDP_WDMA		8
+#define M4U_PORT_MDP_ROTO		9
+#define M4U_PORT_MDP_ROTCO		10
+#define M4U_PORT_MDP_ROTVO		11
+#define M4U_PORT_VENC_MVQP		12
+#define M4U_PORT_VENC_MC		13
+#define M4U_PORT_VENC_CDMA_VDEC_CDMA	14
+#define M4U_PORT_VENC_REC_VDEC_WDMA	15
+
+#endif

-- 
2.55.0

Re: [PATCH 1/3] dt-bindings: iommu: mediatek: add mt6572

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing SMI (Smart Multimedia Interface) bindings for mt6572 alongside the new IOMMU binding.
--

commit a4582297bd41ae5d3cd46c52f5db31a076c1b128
Author: Roman Vivchar <rva333@protonmail.com>

dt-bindings: iommu: mediatek: add mt6572
    
Add compatible string and header file for mt6572 IOMMU support.

> diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> index 79c573c47b08e..d0ac4bd719116 100644
> --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> @@ -72,6 +72,7 @@ properties:
>        - enum:
>            - mediatek,mt2701-m4u  # generation one
>            - mediatek,mt2712-m4u  # generation two
> +          - mediatek,mt6572-m4u  # generation one

[Severity: Medium]
Will device trees for MT6572 fail dt_binding_check because the required SMI
larb nodes lack documented compatible strings for this SoC?

The schema for mediatek,iommu.yaml strictly requires mediatek,larbs for
this IOMMU in its allOf block:

  - if: # The IOMMUs don't have larbs.
      not:
        properties:
          compatible:
            contains:
              enum:
                - mediatek,mt8188-iommu-infra
                - mediatek,mt8189-iommu-apu
                - mediatek,mt8189-iommu-infra
                - mediatek,mt8195-iommu-infra
    then:
      required:
        - mediatek,larbs

However, the corresponding SMI bindings (mediatek,smi-larb.yaml and
mediatek,smi-common.yaml) do not appear to have been updated to include
the mt6572 compatible strings needed to define those required larb nodes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-6572-iommu-v1-0-de261da09fb2@protonmail.com?part=1

Re: [PATCH 1/3] dt-bindings: iommu: mediatek: add mt6572

[Krzysztof Kozlowski]

On Wed, Sep 09, 2026 at 05:14:05PM +0300, Roman Vivchar wrote:
> Add compatible string and header file for mt6572 IOMMU support.
> 
> Signed-off-by: Roman Vivchar <rva333@protonmail.com>
> ---
>  .../devicetree/bindings/iommu/mediatek,iommu.yaml  |  3 +++
>  .../memory/mediatek,mt6572-memory-port.h           | 26 ++++++++++++++++++++++
>  2 files changed, 29 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH 2/3] iommu/mediatek-v1: fix IOMMU device binding

[Roman Vivchar via B4 Relay]

From: Roman Vivchar <rva333@protonmail.com>

Currently the driver silently fails to bind devices and DRM subsystem
(which is the primary IOMMU user on MediaTek SoCs) falls back to PA mode.

Fix this by providing of_xlate callback, which lets the driver actually
bind devices and use VA mode.

The fix can be verified by reading overlay engine input address.
Before:
~ # devmem 0x14007040 32
0xBB900000

After:
~ # devmem 0x14007040 32
0x00000000

Fixes: b17336c55d89 ("iommu/mediatek: add support for mtk iommu generation one HW")
Cc: stable@vger.kernel.org
Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
Untested on mt2701.
---
 drivers/iommu/mtk_iommu_v1.c | 86 ++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 47 deletions(-)

diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index ac97dd2868d4..72355f41c36a 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -413,38 +413,10 @@ static const struct iommu_ops mtk_iommu_v1_ops;
  * MTK generation one iommu HW only support one iommu domain, and all the client
  * sharing the same iova address space.
  */
-static int mtk_iommu_v1_create_mapping(struct device *dev,
-				       const struct of_phandle_args *args)
+static int mtk_iommu_v1_create_mapping(struct device *dev)
 {
 	struct mtk_iommu_v1_data *data;
-	struct platform_device *m4updev;
 	struct dma_iommu_mapping *mtk_mapping;
-	int ret;
-
-	if (args->args_count != 1) {
-		dev_err(dev, "invalid #iommu-cells(%d) property for IOMMU\n",
-			args->args_count);
-		return -EINVAL;
-	}
-
-	ret = iommu_fwspec_init(dev, of_fwnode_handle(args->np));
-	if (ret)
-		return ret;
-
-	if (!dev_iommu_priv_get(dev)) {
-		/* Get the m4u device */
-		m4updev = of_find_device_by_node(args->np);
-		if (WARN_ON(!m4updev))
-			return -EINVAL;
-
-		dev_iommu_priv_set(dev, platform_get_drvdata(m4updev));
-
-		put_device(&m4updev->dev);
-	}
-
-	ret = iommu_fwspec_add_ids(dev, args->args, 1);
-	if (ret)
-		return ret;
 
 	data = dev_iommu_priv_get(dev);
 	mtk_mapping = data->mapping;
@@ -462,27 +434,12 @@ static int mtk_iommu_v1_create_mapping(struct device *dev,
 
 static struct iommu_device *mtk_iommu_v1_probe_device(struct device *dev)
 {
-	struct iommu_fwspec *fwspec = NULL;
-	struct of_phandle_args iommu_spec;
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 	struct mtk_iommu_v1_data *data;
-	int err, idx = 0, larbid, larbidx;
+	int idx, larbid, larbidx;
 	struct device_link *link;
 	struct device *larbdev;
 
-	while (!of_parse_phandle_with_args(dev->of_node, "iommus",
-					   "#iommu-cells",
-					   idx, &iommu_spec)) {
-
-		err = mtk_iommu_v1_create_mapping(dev, &iommu_spec);
-		of_node_put(iommu_spec.np);
-		if (err)
-			return ERR_PTR(err);
-
-		/* dev->iommu_fwspec might have changed */
-		fwspec = dev_iommu_fwspec_get(dev);
-		idx++;
-	}
-
 	if (!fwspec)
 		return ERR_PTR(-ENODEV);
 
@@ -519,9 +476,15 @@ static void mtk_iommu_v1_probe_finalize(struct device *dev)
 	__maybe_unused struct mtk_iommu_v1_data *data = dev_iommu_priv_get(dev);
 	int err;
 
+	err = mtk_iommu_v1_create_mapping(dev);
+	if (err) {
+		dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
+		return;
+	}
+
 	err = arm_iommu_attach_device(dev, data->mapping);
 	if (err)
-		dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
+		dev_err(dev, "Can't attach to IOMMU mapping - DMA-OPS will not work\n");
 }
 
 static void mtk_iommu_v1_release_device(struct device *dev)
@@ -537,6 +500,34 @@ static void mtk_iommu_v1_release_device(struct device *dev)
 	device_link_remove(dev, larbdev);
 }
 
+static int mtk_iommu_v1_of_xlate(struct device *dev,
+				 const struct of_phandle_args *args)
+{
+	struct platform_device *m4updev;
+	int ret;
+
+	if (args->args_count != 1) {
+		dev_err(dev, "invalid #iommu-cells(%d) property for IOMMU\n",
+			args->args_count);
+		return -EINVAL;
+	}
+
+	ret = iommu_fwspec_init(dev, of_fwnode_handle(args->np));
+	if (ret)
+		return ret;
+
+	if (!dev_iommu_priv_get(dev)) {
+		m4updev = of_find_device_by_node(args->np);
+		if (WARN_ON(!m4updev))
+			return -EINVAL;
+
+		dev_iommu_priv_set(dev, platform_get_drvdata(m4updev));
+		put_device(&m4updev->dev);
+	}
+
+	return iommu_fwspec_add_ids(dev, args->args, 1);
+}
+
 static int mtk_iommu_v1_hw_init(const struct mtk_iommu_v1_data *data)
 {
 	u32 regval;
@@ -585,6 +576,7 @@ static const struct iommu_ops mtk_iommu_v1_ops = {
 	.probe_finalize = mtk_iommu_v1_probe_finalize,
 	.release_device	= mtk_iommu_v1_release_device,
 	.device_group	= generic_device_group,
+	.of_xlate	= mtk_iommu_v1_of_xlate,
 	.owner          = THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= mtk_iommu_v1_attach_device,

-- 
2.55.0

Re: [PATCH 2/3] iommu/mediatek-v1: fix IOMMU device binding

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving `mtk_iommu_v1_create_mapping()` into `mtk_iommu_v1_probe_finalize()` introduces a data race that can corrupt the shared IOMMU mapping and domain, leading to memory leaks and incorrect domain assignments.
--

commit 506d7ce15633b781671040d746cf03c5adc6f9ca
Author: Roman Vivchar <rva333@protonmail.com>

iommu/mediatek-v1: fix IOMMU device binding

The driver silently fails to bind devices, causing DRM to fall back to PA mode.
This patch provides an of_xlate callback to properly bind devices and use VA
mode.

> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index ac97dd2868d4b..72355f41c36af 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c

[ ... ]

> @@ -519,9 +476,15 @@ static void mtk_iommu_v1_probe_finalize(struct device *dev)
>  	__maybe_unused struct mtk_iommu_v1_data *data = dev_iommu_priv_get(dev);
>  	int err;
>  
> +	err = mtk_iommu_v1_create_mapping(dev);

[Severity: High]
Does moving the call to mtk_iommu_v1_create_mapping() into
mtk_iommu_v1_probe_finalize() introduce a data race?

Previously, this was called from mtk_iommu_v1_probe_device(), which executes
under the iommu_probe_device_lock.  By moving it here, it executes without
synchronization.

If two client devices of the same MTK IOMMU probe concurrently (e.g., via
asynchronous probing or deferred probe), they can race in
mtk_iommu_v1_create_mapping():

drivers/iommu/mtk_iommu_v1.c:mtk_iommu_v1_create_mapping() {
        ...
        data = dev_iommu_priv_get(dev);
        mtk_mapping = data->mapping;
        if (!mtk_mapping) {
                /* MTK iommu support 4GB iova address space. */
                mtk_mapping = arm_iommu_create_mapping(dev, 0, 1ULL << 32);
                if (IS_ERR(mtk_mapping))
                        return PTR_ERR(mtk_mapping);

                data->mapping = mtk_mapping;
        }
        ...
}

Can this lead to a memory leak of the initial mapping and DMA failures for
the racing client device, as it might get attached to a software domain
that does not match the hardware page tables?

> +	if (err) {
> +		dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
> +		return;
> +	}
> +
>  	err = arm_iommu_attach_device(dev, data->mapping);
>  	if (err)
> -		dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
> +		dev_err(dev, "Can't attach to IOMMU mapping - DMA-OPS will not work\n");
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-6572-iommu-v1-0-de261da09fb2@protonmail.com?part=2

[PATCH 3/3] iommu/mediatek-v1: add mt6572 support

[Roman Vivchar via B4 Relay]

From: Roman Vivchar <rva333@protonmail.com>

mt6572 SoC utilizes an earlier version of the IOMMU, that shares the
same v1 architecture, but has minor hardware differences compared to
mt2701.

Introduce mtk_iommu_type to distinguish between mt2701 and mt6572
variant and apply the necessary logic.

Finally, add a platform data for the mt6572 SoC.

Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
Based on my downstream findings, this change may also support mt6582 SoC,
but I don't have the hardware to test if it actually works.
---
 drivers/iommu/mtk_iommu_v1.c | 72 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 55 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index 72355f41c36a..b6f547b100d1 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -7,6 +7,7 @@
  *
  * Based on driver/iommu/mtk_iommu.c
  */
+#include <linux/bitfield.h>
 #include <linux/bug.h>
 #include <linux/clk.h>
 #include <linux/component.h>
@@ -85,6 +86,13 @@ struct dma_iommu_mapping {
 #define F_DESC_NONSEC				BIT(3)
 #define MT2701_M4U_TF_LARB(TF)			(6 - (((TF) >> 13) & 0x7))
 #define MT2701_M4U_TF_PORT(TF)			(((TF) >> 8) & 0xF)
+
+#define MT6572_MMU_INT_ID_PORT_ID		GENMASK(12, 8)
+#define MT6572_MMU_INT_ID_LARB_ID		GENMASK(14, 13)
+
+#define MT6572_M4U_TF_PORT(TF)			FIELD_GET(MT6572_MMU_INT_ID_PORT_ID, TF)
+#define MT6572_M4U_TF_LARB(TF)			(FIELD_GET(MT6572_MMU_INT_ID_LARB_ID, TF) - 1)
+
 /* MTK generation one iommu HW only support 4K size mapping */
 #define MT2701_IOMMU_PAGE_SHIFT			12
 #define MT2701_IOMMU_PAGE_SIZE			(1UL << MT2701_IOMMU_PAGE_SHIFT)
@@ -96,6 +104,11 @@ struct dma_iommu_mapping {
  */
 #define M2701_IOMMU_PGT_SIZE			SZ_4M
 
+enum mtk_iommu_type {
+	MTK_IOMMU_MT6572,
+	MTK_IOMMU_V1,
+};
+
 struct mtk_iommu_v1_suspend_reg {
 	u32			standard_axi_mode;
 	u32			dcm_dis;
@@ -116,6 +129,8 @@ struct mtk_iommu_v1_data {
 	struct mtk_smi_larb_iommu	larb_imu[MTK_LARB_NR_MAX];
 
 	struct mtk_iommu_v1_suspend_reg	reg;
+
+	enum mtk_iommu_type type;
 };
 
 struct mtk_iommu_v1_domain {
@@ -170,8 +185,12 @@ static inline int mt2701_m4u_to_port(int id)
 
 static void mtk_iommu_v1_tlb_flush_all(struct mtk_iommu_v1_data *data)
 {
-	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
-			data->base + REG_MMU_INV_SEL);
+	u32 val = F_INVLD_EN0;
+
+	if (data->type == MTK_IOMMU_V1)
+		val |= F_INVLD_EN1;
+
+	writel_relaxed(val, data->base + REG_MMU_INV_SEL);
 	writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
 	wmb(); /* Make sure the tlb flush all done */
 }
@@ -180,25 +199,33 @@ static void mtk_iommu_v1_tlb_flush_range(struct mtk_iommu_v1_data *data,
 					 unsigned long iova, size_t size)
 {
 	int ret;
-	u32 tmp;
+	u32 tmp, val = F_INVLD_EN0;
 
-	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
-		data->base + REG_MMU_INV_SEL);
+	if (data->type == MTK_IOMMU_V1)
+		val |= F_INVLD_EN1;
+
+	writel_relaxed(val, data->base + REG_MMU_INV_SEL);
 	writel_relaxed(iova & F_MMU_FAULT_VA_MSK,
 		data->base + REG_MMU_INVLD_START_A);
 	writel_relaxed((iova + size - 1) & F_MMU_FAULT_VA_MSK,
 		data->base + REG_MMU_INVLD_END_A);
 	writel_relaxed(F_MMU_INV_RANGE, data->base + REG_MMU_INVALIDATE);
 
-	ret = readl_poll_timeout_atomic(data->base + REG_MMU_CPE_DONE,
-				tmp, tmp != 0, 10, 100000);
-	if (ret) {
-		dev_warn(data->dev,
-			 "Partial TLB flush timed out, falling back to full flush\n");
-		mtk_iommu_v1_tlb_flush_all(data);
+	if (data->type == MTK_IOMMU_V1) {
+		ret = readl_poll_timeout_atomic(data->base + REG_MMU_CPE_DONE,
+						tmp, tmp != 0, 10, 100000);
+		if (ret) {
+			dev_warn(data->dev,
+				 "Partial TLB flush timed out, falling back to full flush\n");
+			mtk_iommu_v1_tlb_flush_all(data);
+		}
+
+		/* Clear the CPE status */
+		writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
+	} else {
+		/* Make sure the TLB flush is done */
+		wmb();
 	}
-	/* Clear the CPE status */
-	writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
 }
 
 static irqreturn_t mtk_iommu_v1_isr(int irq, void *dev_id)
@@ -215,8 +242,14 @@ static irqreturn_t mtk_iommu_v1_isr(int irq, void *dev_id)
 	fault_iova &= F_MMU_FAULT_VA_MSK;
 	fault_pa = readl_relaxed(data->base + REG_MMU_INVLD_PA);
 	regval = readl_relaxed(data->base + REG_MMU_INT_ID);
-	fault_larb = MT2701_M4U_TF_LARB(regval);
-	fault_port = MT2701_M4U_TF_PORT(regval);
+
+	if (data->type == MTK_IOMMU_V1) {
+		fault_larb = MT2701_M4U_TF_LARB(regval);
+		fault_port = MT2701_M4U_TF_PORT(regval);
+	} else {
+		fault_larb = MT6572_M4U_TF_LARB(regval);
+		fault_port = MT6572_M4U_TF_PORT(regval);
+	}
 
 	/*
 	 * MTK v1 iommu HW could not determine whether the fault is read or
@@ -539,7 +572,10 @@ static int mtk_iommu_v1_hw_init(const struct mtk_iommu_v1_data *data)
 		return ret;
 	}
 
-	regval = F_MMU_CTRL_COHERENT_EN | F_MMU_TF_PROTECT_SEL(2);
+	regval = F_MMU_TF_PROTECT_SEL(2);
+	if (data->type == MTK_IOMMU_V1)
+		regval |= F_MMU_CTRL_COHERENT_EN;
+
 	writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
 
 	regval = F_INT_TRANSLATION_FAULT |
@@ -588,7 +624,8 @@ static const struct iommu_ops mtk_iommu_v1_ops = {
 };
 
 static const struct of_device_id mtk_iommu_v1_of_ids[] = {
-	{ .compatible = "mediatek,mt2701-m4u", },
+	{ .compatible = "mediatek,mt2701-m4u", .data = (void *)MTK_IOMMU_V1 },
+	{ .compatible = "mediatek,mt6572-m4u", .data = (void *)MTK_IOMMU_MT6572 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, mtk_iommu_v1_of_ids);
@@ -612,6 +649,7 @@ static int mtk_iommu_v1_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data->dev = dev;
+	data->type = (enum mtk_iommu_type)(uintptr_t)of_device_get_match_data(dev);
 
 	/* Protect memory. HW will access here while translation fault.*/
 	protect = devm_kcalloc(dev, 2, MTK_PROTECT_PA_ALIGN,

-- 
2.55.0

Re: [PATCH 3/3] iommu/mediatek-v1: add mt6572 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Insufficient hardware synchronization for MT6572 TLB flush completion

Pre-existing issues:
- [High] NULL pointer dereference in mtk_iommu_v1_domain_free() if domain was never attached
- [High] Race condition causing use-after-disable in hardware interrupt handling during teardown
- [High] Data corruption and Use-After-Free due to missing lock in TLB flush sequence
--

commit 29773ece1b87d664d6aa369fe95265b2f2898794
Author: Roman Vivchar <rva333@protonmail.com>

iommu/mediatek-v1: add mt6572 support

This commit adds support for the mt6572 SoC, which utilizes an earlier
version of the IOMMU that shares the same v1 architecture but has minor
hardware differences compared to mt2701. It introduces mtk_iommu_type to
distinguish between the mt2701 and mt6572 variants.

> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index 72355f41c36af..b6f547b100d10 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c

[ ... ]

> @@ -180,25 +199,33 @@ static void mtk_iommu_v1_tlb_flush_range(struct mtk_iommu_v1_data *data,
>  					 unsigned long iova, size_t size)
>  {
>  	int ret;
> -	u32 tmp;
> +	u32 tmp, val = F_INVLD_EN0;
>  
> -	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> -		data->base + REG_MMU_INV_SEL);
> +	if (data->type == MTK_IOMMU_V1)
> +		val |= F_INVLD_EN1;
> +
> +	writel_relaxed(val, data->base + REG_MMU_INV_SEL);
>  	writel_relaxed(iova & F_MMU_FAULT_VA_MSK,
>  		data->base + REG_MMU_INVLD_START_A);
>  	writel_relaxed((iova + size - 1) & F_MMU_FAULT_VA_MSK,
>  		data->base + REG_MMU_INVLD_END_A);
>  	writel_relaxed(F_MMU_INV_RANGE, data->base + REG_MMU_INVALIDATE);

[Severity: High]
This isn't a bug introduced by this patch, but does this sequence of global
register writes lack a lock?

In mtk_iommu_v1_unmap() and mtk_iommu_v1_map(), pgtlock is dropped before
calling mtk_iommu_v1_tlb_flush_range():

drivers/iommu/mtk_iommu_v1.c:mtk_iommu_v1_unmap() {
    ...
	spin_unlock_irqrestore(&dom->pgtlock, flags);

	mtk_iommu_v1_tlb_flush_range(dom->data, iova, size);
    ...
}

If multiple CPUs concurrently perform map/unmap operations, can the
non-atomic sequence of writel_relaxed() calls to these shared global MMIO
registers interleave, causing the IOMMU to flush incorrect IOVA ranges?

> -	ret = readl_poll_timeout_atomic(data->base + REG_MMU_CPE_DONE,
> -				tmp, tmp != 0, 10, 100000);
> -	if (ret) {
> -		dev_warn(data->dev,
> -			 "Partial TLB flush timed out, falling back to full flush\n");
> -		mtk_iommu_v1_tlb_flush_all(data);
> +	if (data->type == MTK_IOMMU_V1) {
> +		ret = readl_poll_timeout_atomic(data->base + REG_MMU_CPE_DONE,
> +						tmp, tmp != 0, 10, 100000);
> +		if (ret) {
> +			dev_warn(data->dev,
> +				 "Partial TLB flush timed out, falling back to full flush\n");
> +			mtk_iommu_v1_tlb_flush_all(data);
> +		}
> +
> +		/* Clear the CPE status */
> +		writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
> +	} else {
> +		/* Make sure the TLB flush is done */
> +		wmb();
>  	}
> -	/* Clear the CPE status */
> -	writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
>  }

[Severity: High]
Does a wmb() CPU barrier provide actual hardware synchronization for the
MT6572 TLB flush completion?

A wmb() only orders CPU memory writes; it does not ensure the write has
reached the IOMMU hardware, nor does it poll the hardware status to verify
the TLB flush has finished.

If the software unmap path returns and allows memory to be freed while the
hardware is still performing the TLB invalidation, can a device DMA into
freed memory before the TLB entry actually invalidates, causing
Use-After-Free?

[Severity: High]
This is a pre-existing issue, but is there a race condition during hardware
interrupt handling when tearing down the driver?

In the probe error path and in mtk_iommu_v1_remove(), clk_disable_unprepare()
is called while the hardware IRQ is still actively registered:

drivers/iommu/mtk_iommu_v1.c:mtk_iommu_v1_probe() {
    ...
out_clk_unprepare:
	clk_disable_unprepare(data->bclk);
out_put_larbs:
	for (i = 0; i < MTK_LARB_NR_MAX; i++)
		put_device(data->larb_imu[i].dev);

	return ret;
}

drivers/iommu/mtk_iommu_v1.c:mtk_iommu_v1_remove() {
    ...
	clk_disable_unprepare(data->bclk);
	devm_free_irq(&pdev->dev, data->irq, data);
    ...
}

If a hardware interrupt fires between the clock disable and the IRQ being
freed (or cleaned up by devres), could mtk_iommu_v1_isr() execute and
perform readl_relaxed() on the unclocked hardware, resulting in a
synchronous external abort?

[Severity: High]
This is a pre-existing issue, but can a NULL pointer dereference happen in
mtk_iommu_v1_domain_free() if the domain was never attached?

If an IOMMU domain is successfully allocated via
mtk_iommu_v1_domain_alloc_paging() but device attachment fails or is
skipped, dom->data remains NULL because it is only populated inside
mtk_iommu_v1_attach_device() (via mtk_iommu_v1_domain_finalise).

When the IOMMU core subsequently invokes iommu_domain_free(), it calls
mtk_iommu_v1_domain_free():

drivers/iommu/mtk_iommu_v1.c:mtk_iommu_v1_domain_free() {
    ...
	struct mtk_iommu_v1_data *data = dom->data;

	dma_free_coherent(data->dev, M2701_IOMMU_PGT_SIZE,
			dom->pgt_va, dom->pgt_pa);
    ...
}

Does unconditionally dereferencing data->dev here cause a kernel panic if
data is NULL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-6572-iommu-v1-0-de261da09fb2@protonmail.com?part=3