* [PATCH v6 1/6] iommu/mediatek: Add platform_device_put for recovering the device refcnt
2022-09-20 6:37 [PATCH v6 0/6] iommu/mediatek: Improve safety from invalid dts input Yong Wu
@ 2022-09-20 6:37 ` Yong Wu
2022-09-20 6:37 ` [PATCH v6 2/6] iommu/mediatek: Use component_match_add Yong Wu
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Yong Wu @ 2022-09-20 6:37 UTC (permalink / raw)
To: Joerg Roedel, Matthias Brugger, Will Deacon
Cc: Robin Murphy, iommu, linux-mediatek, linux-arm-kernel,
linux-kernel, Yong Wu, AngeloGioacchino Del Regno, mingyuan.ma,
yf.wang, libo.kang, chengci.xu, youlin.pei, anan.sun,
xueqi.zhang, Guenter Roeck, Dan Carpenter
Add platform_device_put to match with of_find_device_by_node.
Meanwhile, I add a new variable "pcommdev" which is for smi common device.
Otherwise, "platform_device_put(plarbdev)" for smi-common dev may be not
readable. And add a checking for whether pcommdev is NULL.
Fixes: d2e9a1102cfc ("iommu/mediatek: Contain MM IOMMU flow with the MM TYPE")
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/iommu/mtk_iommu.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 7e363b1f24df..21a8ce503a0e 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1041,7 +1041,7 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
struct mtk_iommu_data *data)
{
struct device_node *larbnode, *smicomm_node, *smi_subcomm_node;
- struct platform_device *plarbdev;
+ struct platform_device *plarbdev, *pcommdev;
struct device_link *link;
int i, larb_nr, ret;
@@ -1072,12 +1072,14 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
}
if (!plarbdev->dev.driver) {
of_node_put(larbnode);
+ platform_device_put(plarbdev);
return -EPROBE_DEFER;
}
data->larb_imu[id].dev = &plarbdev->dev;
component_match_add_release(dev, match, component_release_of,
component_compare_of, larbnode);
+ platform_device_put(plarbdev);
}
/* Get smi-(sub)-common dev from the last larb. */
@@ -1095,12 +1097,15 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
else
smicomm_node = smi_subcomm_node;
- plarbdev = of_find_device_by_node(smicomm_node);
+ pcommdev = of_find_device_by_node(smicomm_node);
of_node_put(smicomm_node);
- data->smicomm_dev = &plarbdev->dev;
+ if (!pcommdev)
+ return -ENODEV;
+ data->smicomm_dev = &pcommdev->dev;
link = device_link_add(data->smicomm_dev, dev,
DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
+ platform_device_put(pcommdev);
if (!link) {
dev_err(dev, "Unable to link %s.\n", dev_name(data->smicomm_dev));
return -EINVAL;
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v6 2/6] iommu/mediatek: Use component_match_add
2022-09-20 6:37 [PATCH v6 0/6] iommu/mediatek: Improve safety from invalid dts input Yong Wu
2022-09-20 6:37 ` [PATCH v6 1/6] iommu/mediatek: Add platform_device_put for recovering the device refcnt Yong Wu
@ 2022-09-20 6:37 ` Yong Wu
2022-09-20 6:37 ` [PATCH v6 3/6] iommu/mediatek: Add error path for loop of mm_dts_parse Yong Wu
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Yong Wu @ 2022-09-20 6:37 UTC (permalink / raw)
To: Joerg Roedel, Matthias Brugger, Will Deacon
Cc: Robin Murphy, iommu, linux-mediatek, linux-arm-kernel,
linux-kernel, Yong Wu, AngeloGioacchino Del Regno, mingyuan.ma,
yf.wang, libo.kang, chengci.xu, youlin.pei, anan.sun,
xueqi.zhang, Guenter Roeck, Dan Carpenter
In order to simplify the error patch(avoid call of_node_put), Use
component_match_add instead component_match_add_release since we are only
interested in the "device" here. Then we could always call of_node_put in
normal path.
Strictly this is not a fixes patch, but it is a prepare for adding the
error path, thus I add a Fixes tag too.
Fixes: d2e9a1102cfc ("iommu/mediatek: Contain MM IOMMU flow with the MM TYPE")
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/iommu/mtk_iommu.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 21a8ce503a0e..83b279432406 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1066,19 +1066,17 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
id = i;
plarbdev = of_find_device_by_node(larbnode);
- if (!plarbdev) {
- of_node_put(larbnode);
+ of_node_put(larbnode);
+ if (!plarbdev)
return -ENODEV;
- }
+
if (!plarbdev->dev.driver) {
- of_node_put(larbnode);
platform_device_put(plarbdev);
return -EPROBE_DEFER;
}
data->larb_imu[id].dev = &plarbdev->dev;
- component_match_add_release(dev, match, component_release_of,
- component_compare_of, larbnode);
+ component_match_add(dev, match, component_compare_dev, &plarbdev->dev);
platform_device_put(plarbdev);
}
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v6 3/6] iommu/mediatek: Add error path for loop of mm_dts_parse
2022-09-20 6:37 [PATCH v6 0/6] iommu/mediatek: Improve safety from invalid dts input Yong Wu
2022-09-20 6:37 ` [PATCH v6 1/6] iommu/mediatek: Add platform_device_put for recovering the device refcnt Yong Wu
2022-09-20 6:37 ` [PATCH v6 2/6] iommu/mediatek: Use component_match_add Yong Wu
@ 2022-09-20 6:37 ` Yong Wu
2022-09-20 6:37 ` [PATCH v6 4/6] iommu/mediatek: Validate number of phandles associated with "mediatek,larbs" Yong Wu
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Yong Wu @ 2022-09-20 6:37 UTC (permalink / raw)
To: Joerg Roedel, Matthias Brugger, Will Deacon
Cc: Robin Murphy, iommu, linux-mediatek, linux-arm-kernel,
linux-kernel, Yong Wu, AngeloGioacchino Del Regno, mingyuan.ma,
yf.wang, libo.kang, chengci.xu, youlin.pei, anan.sun,
xueqi.zhang, Guenter Roeck, Dan Carpenter
The mtk_iommu_mm_dts_parse will parse the smi larbs nodes. if the i+1
larb is parsed fail, we should put_device for the i..0 larbs.
There are two places need to comment:
1) The larbid may be not linear mapping, we should loop whole
the array in the error path.
2) I move this line position: "data->larb_imu[id].dev = &plarbdev->dev;"
before "if (!plarbdev->dev.driver)", That means set
data->larb_imu[id].dev before the error path. then we don't need
"platform_device_put(plarbdev)" again in probe_defer case. All depend
on "put_device" of the error path in error cases.
Fixes: d2e9a1102cfc ("iommu/mediatek: Contain MM IOMMU flow with the MM TYPE")
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/iommu/mtk_iommu.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 83b279432406..97b721006a47 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1053,8 +1053,10 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
u32 id;
larbnode = of_parse_phandle(dev->of_node, "mediatek,larbs", i);
- if (!larbnode)
- return -EINVAL;
+ if (!larbnode) {
+ ret = -EINVAL;
+ goto err_larbdev_put;
+ }
if (!of_device_is_available(larbnode)) {
of_node_put(larbnode);
@@ -1067,14 +1069,16 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
plarbdev = of_find_device_by_node(larbnode);
of_node_put(larbnode);
- if (!plarbdev)
- return -ENODEV;
+ if (!plarbdev) {
+ ret = -ENODEV;
+ goto err_larbdev_put;
+ }
+ data->larb_imu[id].dev = &plarbdev->dev;
if (!plarbdev->dev.driver) {
- platform_device_put(plarbdev);
- return -EPROBE_DEFER;
+ ret = -EPROBE_DEFER;
+ goto err_larbdev_put;
}
- data->larb_imu[id].dev = &plarbdev->dev;
component_match_add(dev, match, component_compare_dev, &plarbdev->dev);
platform_device_put(plarbdev);
@@ -1109,6 +1113,15 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
return -EINVAL;
}
return 0;
+
+err_larbdev_put:
+ /* id may be not linear mapping, loop whole the array */
+ for (i = MTK_LARB_NR_MAX - 1; i >= 0; i++) {
+ if (!data->larb_imu[i].dev)
+ continue;
+ put_device(data->larb_imu[i].dev);
+ }
+ return ret;
}
static int mtk_iommu_probe(struct platform_device *pdev)
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v6 4/6] iommu/mediatek: Validate number of phandles associated with "mediatek,larbs"
2022-09-20 6:37 [PATCH v6 0/6] iommu/mediatek: Improve safety from invalid dts input Yong Wu
` (2 preceding siblings ...)
2022-09-20 6:37 ` [PATCH v6 3/6] iommu/mediatek: Add error path for loop of mm_dts_parse Yong Wu
@ 2022-09-20 6:37 ` Yong Wu
2022-09-20 6:37 ` [PATCH v6 5/6] iommu/mediatek: Improve safety for mediatek,smi property in larb nodes Yong Wu
2022-09-20 6:37 ` [PATCH v6 6/6] iommu/mediatek: Remove unused "mapping" member from mtk_iommu_data Yong Wu
5 siblings, 0 replies; 7+ messages in thread
From: Yong Wu @ 2022-09-20 6:37 UTC (permalink / raw)
To: Joerg Roedel, Matthias Brugger, Will Deacon
Cc: Robin Murphy, iommu, linux-mediatek, linux-arm-kernel,
linux-kernel, Yong Wu, AngeloGioacchino Del Regno, mingyuan.ma,
yf.wang, libo.kang, chengci.xu, youlin.pei, anan.sun,
xueqi.zhang, Guenter Roeck, Dan Carpenter
From: Guenter Roeck <groeck@chromium.org>
Fix the smatch warnings:
drivers/iommu/mtk_iommu.c:878 mtk_iommu_mm_dts_parse() error: uninitialized
symbol 'larbnode'.
If someone abuse the dtsi node(Don't follow the definition of dt-binding),
for example "mediatek,larbs" is provided as boolean property, "larb_nr"
will be zero and cause abnormal.
To fix this problem and improve the code safety, add some checking
for the invalid input from dtsi, e.g. checking the larb_nr/larbid valid
range, and avoid "mediatek,larb-id" property conflicts in the smi-larb
nodes.
Fixes: d2e9a1102cfc ("iommu/mediatek: Contain MM IOMMU flow with the MM TYPE")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/iommu/mtk_iommu.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 97b721006a47..01b31b09bd8d 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1048,6 +1048,8 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
larb_nr = of_count_phandle_with_args(dev->of_node, "mediatek,larbs", NULL);
if (larb_nr < 0)
return larb_nr;
+ if (larb_nr == 0 || larb_nr > MTK_LARB_NR_MAX)
+ return -EINVAL;
for (i = 0; i < larb_nr; i++) {
u32 id;
@@ -1066,6 +1068,11 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
ret = of_property_read_u32(larbnode, "mediatek,larb-id", &id);
if (ret)/* The id is consecutive if there is no this property */
id = i;
+ if (id >= MTK_LARB_NR_MAX) {
+ of_node_put(larbnode);
+ ret = -EINVAL;
+ goto err_larbdev_put;
+ }
plarbdev = of_find_device_by_node(larbnode);
of_node_put(larbnode);
@@ -1073,6 +1080,11 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
ret = -ENODEV;
goto err_larbdev_put;
}
+ if (data->larb_imu[id].dev) {
+ platform_device_put(plarbdev);
+ ret = -EEXIST;
+ goto err_larbdev_put;
+ }
data->larb_imu[id].dev = &plarbdev->dev;
if (!plarbdev->dev.driver) {
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v6 5/6] iommu/mediatek: Improve safety for mediatek,smi property in larb nodes
2022-09-20 6:37 [PATCH v6 0/6] iommu/mediatek: Improve safety from invalid dts input Yong Wu
` (3 preceding siblings ...)
2022-09-20 6:37 ` [PATCH v6 4/6] iommu/mediatek: Validate number of phandles associated with "mediatek,larbs" Yong Wu
@ 2022-09-20 6:37 ` Yong Wu
2022-09-20 6:37 ` [PATCH v6 6/6] iommu/mediatek: Remove unused "mapping" member from mtk_iommu_data Yong Wu
5 siblings, 0 replies; 7+ messages in thread
From: Yong Wu @ 2022-09-20 6:37 UTC (permalink / raw)
To: Joerg Roedel, Matthias Brugger, Will Deacon
Cc: Robin Murphy, iommu, linux-mediatek, linux-arm-kernel,
linux-kernel, Yong Wu, AngeloGioacchino Del Regno, mingyuan.ma,
yf.wang, libo.kang, chengci.xu, youlin.pei, anan.sun,
xueqi.zhang, Guenter Roeck, Dan Carpenter
No functional change. Just improve safety from dts.
All the larbs that connect to one IOMMU must connect with the same
smi-common. This patch checks all the mediatek,smi property for each
larb, If their mediatek,smi are different, it will return fails.
Also avoid there is no available smi-larb nodes.
Suggested-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/iommu/mtk_iommu.c | 53 +++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 01b31b09bd8d..f92b8c0bb373 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1040,7 +1040,7 @@ static const struct component_master_ops mtk_iommu_com_ops = {
static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **match,
struct mtk_iommu_data *data)
{
- struct device_node *larbnode, *smicomm_node, *smi_subcomm_node;
+ struct device_node *larbnode, *frst_avail_smicomm_node = NULL;
struct platform_device *plarbdev, *pcommdev;
struct device_link *link;
int i, larb_nr, ret;
@@ -1052,6 +1052,7 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
return -EINVAL;
for (i = 0; i < larb_nr; i++) {
+ struct device_node *smicomm_node, *smi_subcomm_node;
u32 id;
larbnode = of_parse_phandle(dev->of_node, "mediatek,larbs", i);
@@ -1092,27 +1093,47 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
goto err_larbdev_put;
}
+ /* Get smi-(sub)-common dev from the last larb. */
+ smi_subcomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0);
+ if (!smi_subcomm_node) {
+ ret = -EINVAL;
+ goto err_larbdev_put;
+ }
+
+ /*
+ * It may have two level smi-common. the node is smi-sub-common if it
+ * has a new mediatek,smi property. otherwise it is smi-commmon.
+ */
+ smicomm_node = of_parse_phandle(smi_subcomm_node, "mediatek,smi", 0);
+ if (smicomm_node)
+ of_node_put(smi_subcomm_node);
+ else
+ smicomm_node = smi_subcomm_node;
+
+ /*
+ * All the larbs that connect to one IOMMU must connect with the same
+ * smi-common.
+ */
+ if (!frst_avail_smicomm_node) {
+ frst_avail_smicomm_node = smicomm_node;
+ } else if (frst_avail_smicomm_node != smicomm_node) {
+ dev_err(dev, "mediatek,smi property is not right @larb%d.", id);
+ of_node_put(smicomm_node);
+ ret = -EINVAL;
+ goto err_larbdev_put;
+ } else {
+ of_node_put(smicomm_node);
+ }
+
component_match_add(dev, match, component_compare_dev, &plarbdev->dev);
platform_device_put(plarbdev);
}
- /* Get smi-(sub)-common dev from the last larb. */
- smi_subcomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0);
- if (!smi_subcomm_node)
+ if (!frst_avail_smicomm_node)
return -EINVAL;
- /*
- * It may have two level smi-common. the node is smi-sub-common if it
- * has a new mediatek,smi property. otherwise it is smi-commmon.
- */
- smicomm_node = of_parse_phandle(smi_subcomm_node, "mediatek,smi", 0);
- if (smicomm_node)
- of_node_put(smi_subcomm_node);
- else
- smicomm_node = smi_subcomm_node;
-
- pcommdev = of_find_device_by_node(smicomm_node);
- of_node_put(smicomm_node);
+ pcommdev = of_find_device_by_node(frst_avail_smicomm_node);
+ of_node_put(frst_avail_smicomm_node);
if (!pcommdev)
return -ENODEV;
data->smicomm_dev = &pcommdev->dev;
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v6 6/6] iommu/mediatek: Remove unused "mapping" member from mtk_iommu_data
2022-09-20 6:37 [PATCH v6 0/6] iommu/mediatek: Improve safety from invalid dts input Yong Wu
` (4 preceding siblings ...)
2022-09-20 6:37 ` [PATCH v6 5/6] iommu/mediatek: Improve safety for mediatek,smi property in larb nodes Yong Wu
@ 2022-09-20 6:37 ` Yong Wu
5 siblings, 0 replies; 7+ messages in thread
From: Yong Wu @ 2022-09-20 6:37 UTC (permalink / raw)
To: Joerg Roedel, Matthias Brugger, Will Deacon
Cc: Robin Murphy, iommu, linux-mediatek, linux-arm-kernel,
linux-kernel, Yong Wu, AngeloGioacchino Del Regno, mingyuan.ma,
yf.wang, libo.kang, chengci.xu, youlin.pei, anan.sun,
xueqi.zhang, Guenter Roeck, Dan Carpenter
Just remove a unused variable that only is for mtk_iommu_v1.
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
---
drivers/iommu/mtk_iommu.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index f92b8c0bb373..eb3ace99beff 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -221,10 +221,7 @@ struct mtk_iommu_data {
struct device *smicomm_dev;
struct mtk_iommu_bank_data *bank;
-
- struct dma_iommu_mapping *mapping; /* For mtk_iommu_v1.c */
struct regmap *pericfg;
-
struct mutex mutex; /* Protect m4u_group/m4u_dom above */
/*
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread