Re: [PATCH v4 3/4] memory: mtk-smi: Add enable IOMMU SMC command for MM master

On Tue, 2022-08-09 at 13:58 +0800, Yong Wu wrote:

On Mon, 2022-08-01 at 10:18 +0800, Chengci.Xu wrote:

quoted

For concerns about security, the register to enable/disable IOMMU
of
SMI LARB should only be configured in secure world. Thus, we add
some
SMC command for multimedia master to enable/disable MM IOMMU in ATF
by
setting the register of SMI LARB. This function is prepared for
MT8188.

Signed-off-by: Chengci.Xu <redacted>
---
 drivers/memory/mtk-smi.c                 | 19 +++++++++++++++++++
 include/linux/soc/mediatek/mtk_sip_svc.h |  3 +++
 include/soc/mediatek/smi.h               |  5 +++++
 3 files changed, 27 insertions(+)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 08c1668d47bf..28e1123f9d76 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c

@@ -3,6 +3,7 @@
  * Copyright (c) 2015-2016 MediaTek Inc.
  * Author: Yong Wu <yong.wu@mediatek.com>
  */
+#include <linux/arm-smccc.h>
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <linux/device.h>

@@ -14,6 +15,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/soc/mediatek/mtk_sip_svc.h>
 #include <soc/mediatek/smi.h>
 #include <dt-bindings/memory/mt2701-larb-port.h>
 #include <dt-bindings/memory/mtk-memory-port.h>

@@ -89,6 +91,7 @@
 #define MTK_SMI_FLAG_THRT_UPDATE	BIT(0)
 #define MTK_SMI_FLAG_SW_FLAG		BIT(1)
 #define MTK_SMI_FLAG_SLEEP_CTL		BIT(2)
+#define MTK_SMI_FLAG_CFG_PORT_SEC_CTL	BIT(3)
 #define MTK_SMI_CAPS(flags, _x)		(!!((flags) & (_x)))
 
 struct mtk_smi_reg_pair {

@@ -238,6 +241,7 @@ static int

mtk_smi_larb_config_port_gen2_general(struct device *dev)
 	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
 	u32 reg, flags_general = larb->larb_gen->flags_general;
 	const u8 *larbostd = larb->larb_gen->ostd ? larb->larb_gen-

quoted

ostd[larb->larbid] : NULL;

+	struct arm_smccc_res res;
 	int i;
 
 	if (BIT(larb->larbid) & larb->larb_gen-

quoted

larb_direct_to_common_mask)

@@ -262,6 +266,21 @@ static int

mtk_smi_larb_config_port_gen2_general(struct device *dev)
 		reg |= BANK_SEL(larb->bank[i]);
 		writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
 	}
+
+	/*
+	 * We still need to write above NONSEC_CON register for bank
selection
+	 * even if enabling IOMMU is failed by SMC call. Otherwise, all
master
+	 * will be configured to bank 0 with a state of IOMMU enable
due to the
+	 * HW default value of register.

I don't understand this. Does bankX(X!=0) make sense if SMC fail? 

I guess the below segment should be before SMI_LARB_NONSEC_CON. It
means we shouldn't touch bank_sel while SMC fail.

If SMC call failed, we do have no need to configure bank_sel. All we
can do is stop following step and report an error. So it is a good
choice to put the code about MTK_SMI_FLAG_CFG_PORT_SEC_CTL in front of
SMI_LARB_NONSEC_CON. I will do it in next version.

Just add a comment like:
/* When mmu_en bits are in security world, the bank_sel still is in
the
LARB_NONSEC_CON below. and the mmu_en of LARB_NONSEC_CON have no
effect
in this case. */

OK, got it, thanks.

quoted

+	 */
+	if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_CFG_PORT_SEC_CTL))
{
+		arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONTROL,
IOMMU_ATF_CMD_CONFIG_SMI_LARB,
+			      larb->larbid, (u32)(*larb->mmu), 0, 0, 0,

The type of mmu is "u32", no need the "(u32)".

quoted

0, &res);
+		if (res.a0 != 0) {
+			dev_err(dev, "Enable iommu fail, ret %ld\n",
res.a0);
+			return -EINVAL;
+		}
+	}
 	return 0;
 }
 

diff --git a/include/linux/soc/mediatek/mtk_sip_svc.h

b/include/linux/soc/mediatek/mtk_sip_svc.h
index 082398e0cfb1..0761128b4354 100644

--- a/include/linux/soc/mediatek/mtk_sip_svc.h
+++ b/include/linux/soc/mediatek/mtk_sip_svc.h

@@ -22,4 +22,7 @@
 	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, MTK_SIP_SMC_CONVENTION,

\
 			   ARM_SMCCC_OWNER_SIP, fn_id)
 
+/* IOMMU related SMC call */
+#define MTK_SIP_KERNEL_IOMMU_CONTROL	MTK_SIP_SMC_CMD(0x514)
+
 #endif

diff --git a/include/soc/mediatek/smi.h

b/include/soc/mediatek/smi.h
index 11f7d6b59642..dfd8efca5e60 100644

--- a/include/soc/mediatek/smi.h
+++ b/include/soc/mediatek/smi.h

@@ -11,6 +11,11 @@
 
 #if IS_ENABLED(CONFIG_MTK_SMI)
 
+enum iommu_atf_cmd {
+	IOMMU_ATF_CMD_CONFIG_SMI_LARB,		/* For mm master to

en/disable iommu */
+	IOMMU_ATF_CMD_MAX,
+};
+
 #define MTK_SMI_MMU_EN(port)	BIT(port)
 
 struct mtk_smi_larb_iommu {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel