| Message ID | 20220831125502.7818-3-chengci.xu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MT8188 IOMMU SUPPORT | expand |
Il 31/08/22 14:55, Chengci.Xu ha scritto: > The register which can enable IOMMU for INFRA master should be setted > in secure world for security concerns. Therefore, we add a SMC command > for INFRA master to enable/disable INFRA IOMMU in ATF. This function is > prepared for MT8188. > > Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com> > --- > drivers/iommu/mtk_iommu.c | 34 ++++++++++++++++++++++++++-------- > include/soc/mediatek/smi.h | 1 + > 2 files changed, 27 insertions(+), 8 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 7e363b1f24df..6fe780783ec8 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.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/bitfield.h> > #include <linux/bug.h> > #include <linux/clk.h> > @@ -28,6 +29,7 @@ > #include <linux/slab.h> > #include <linux/spinlock.h> > #include <linux/soc/mediatek/infracfg.h> > +#include <linux/soc/mediatek/mtk_sip_svc.h> > #include <asm/barrier.h> > #include <soc/mediatek/smi.h> > > @@ -138,6 +140,7 @@ > #define PM_CLK_AO BIT(15) > #define IFA_IOMMU_PCIE_SUPPORT BIT(16) > #define PGTABLE_PA_35_EN BIT(17) > +#define CFG_IFA_MASTER_IN_ATF BIT(18) > > #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \ > ((((pdata)->flags) & (mask)) == (_x)) > @@ -554,14 +557,29 @@ static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev, > else > larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid); > } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA)) { > - peri_mmuen_msk = BIT(portid); > - /* PCI dev has only one output id, enable the next writing bit for PCIe */ > - if (dev_is_pci(dev)) > - peri_mmuen_msk |= BIT(portid + 1); > - > - peri_mmuen = enable ? peri_mmuen_msk : 0; > - ret = regmap_update_bits(data->pericfg, PERICFG_IOMMU_1, > - peri_mmuen_msk, peri_mmuen); > + if (MTK_IOMMU_HAS_FLAG(data->plat_data, CFG_IFA_MASTER_IN_ATF)) { > + struct arm_smccc_res res; > + > + portid = MTK_M4U_TO_PORT(fwspec->ids[i]); This assignment is redundant, as portid is initialized to the same value just a few lines before. Please drop it. Everything else looks good. Regards, Angelo > + arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONTROL, > + IOMMU_ATF_CMD_CONFIG_INFRA_IOMMU, > + portid, enable, 0, 0, 0, 0, &res); > + ret = (int)res.a0;
On Wed, 2022-08-31 at 20:55 +0800, Chengci.Xu wrote: > The register which can enable IOMMU for INFRA master should be setted > in secure world for security concerns. Therefore, we add a SMC > command > for INFRA master to enable/disable INFRA IOMMU in ATF. This function > is > prepared for MT8188. > > Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com> > --- > drivers/iommu/mtk_iommu.c | 34 ++++++++++++++++++++++++++-------- > include/soc/mediatek/smi.h | 1 + > 2 files changed, 27 insertions(+), 8 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 7e363b1f24df..6fe780783ec8 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.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/bitfield.h> > #include <linux/bug.h> > #include <linux/clk.h> > @@ -28,6 +29,7 @@ > #include <linux/slab.h> > #include <linux/spinlock.h> > #include <linux/soc/mediatek/infracfg.h> > +#include <linux/soc/mediatek/mtk_sip_svc.h> > #include <asm/barrier.h> > #include <soc/mediatek/smi.h> > > @@ -138,6 +140,7 @@ > #define PM_CLK_AO BIT(15) > #define IFA_IOMMU_PCIE_SUPPORT BIT(16) > #define PGTABLE_PA_35_EN BIT(17) > +#define CFG_IFA_MASTER_IN_ATF BIT(18) > > #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \ > ((((pdata)->flags) & (mask)) == (_x)) > @@ -554,14 +557,29 @@ static int mtk_iommu_config(struct > mtk_iommu_data *data, struct device *dev, > else > larb_mmu->mmu &= > ~MTK_SMI_MMU_EN(portid); > } else if (MTK_IOMMU_IS_TYPE(data->plat_data, > MTK_IOMMU_TYPE_INFRA)) { > - peri_mmuen_msk = BIT(portid); > - /* PCI dev has only one output id, enable the > next writing bit for PCIe */ > - if (dev_is_pci(dev)) > - peri_mmuen_msk |= BIT(portid + 1); > - > - peri_mmuen = enable ? peri_mmuen_msk : 0; > - ret = regmap_update_bits(data->pericfg, > PERICFG_IOMMU_1, > - peri_mmuen_msk, > peri_mmuen); > + if (MTK_IOMMU_HAS_FLAG(data->plat_data, > CFG_IFA_MASTER_IN_ATF)) { > + struct arm_smccc_res res; > + > + portid = MTK_M4U_TO_PORT(fwspec- > >ids[i]); > + arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONT > ROL, > + IOMMU_ATF_CMD_CONFIG_INFR > A_IOMMU, > + portid, enable, 0, 0, 0, > 0, &res); > + ret = (int)res.a0; Too many indentations. Please create a new interface or even just use "if", like: if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) { xx } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA && MTK_IOMMU_HAS_FLAG(data->plat_data, CFG_IFA_MASTER_IN_ATF)) { /* Configure the mmu_en in the ATF for infra IOMMU */ xx } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) { } > + > + } else { > + peri_mmuen_msk = BIT(portid); > + /* PCI dev has only one output id, > + * enable the next writing bit for PCIe > + */ > + if (dev_is_pci(dev)) > + peri_mmuen_msk |= BIT(portid + > 1); > + > + peri_mmuen = enable ? peri_mmuen_msk : > 0; > + ret = regmap_update_bits(data->pericfg, > + PERICFG_IOMMU_ > 1, > + peri_mmuen_msk > , > + peri_mmuen); > + } > if (ret) > dev_err(dev, "%s iommu(%s) inframaster > 0x%x fail(%d).\n", > enable ? "enable" : "disable", > diff --git a/include/soc/mediatek/smi.h b/include/soc/mediatek/smi.h > index dfd8efca5e60..99f13b0e416d 100644 > --- a/include/soc/mediatek/smi.h > +++ b/include/soc/mediatek/smi.h > @@ -13,6 +13,7 @@ > > enum iommu_atf_cmd { > IOMMU_ATF_CMD_CONFIG_SMI_LARB, /* For mm master to > en/disable iommu */ > + IOMMU_ATF_CMD_CONFIG_INFRA_IOMMU, /* For infra master > en/disable iommu */ > IOMMU_ATF_CMD_MAX, > }; >
On Fri, 2022-09-02 at 11:00 +0200, AngeloGioacchino Del Regno wrote: > Il 31/08/22 14:55, Chengci.Xu ha scritto: > > The register which can enable IOMMU for INFRA master should be > > setted > > in secure world for security concerns. Therefore, we add a SMC > > command > > for INFRA master to enable/disable INFRA IOMMU in ATF. This > > function is > > prepared for MT8188. > > > > Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com> > > --- > > drivers/iommu/mtk_iommu.c | 34 ++++++++++++++++++++++++++------- > > - > > include/soc/mediatek/smi.h | 1 + > > 2 files changed, 27 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index 7e363b1f24df..6fe780783ec8 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.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/bitfield.h> > > #include <linux/bug.h> > > #include <linux/clk.h> > > @@ -28,6 +29,7 @@ > > #include <linux/slab.h> > > #include <linux/spinlock.h> > > #include <linux/soc/mediatek/infracfg.h> > > +#include <linux/soc/mediatek/mtk_sip_svc.h> > > #include <asm/barrier.h> > > #include <soc/mediatek/smi.h> > > > > @@ -138,6 +140,7 @@ > > #define PM_CLK_AO BIT(15) > > #define IFA_IOMMU_PCIE_SUPPORT BIT(16) > > #define PGTABLE_PA_35_EN BIT(17) > > +#define CFG_IFA_MASTER_IN_ATF BIT(18) > > > > #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \ > > ((((pdata)->flags) & (mask)) == (_x)) > > @@ -554,14 +557,29 @@ static int mtk_iommu_config(struct > > mtk_iommu_data *data, struct device *dev, > > else > > larb_mmu->mmu &= > > ~MTK_SMI_MMU_EN(portid); > > } else if (MTK_IOMMU_IS_TYPE(data->plat_data, > > MTK_IOMMU_TYPE_INFRA)) { > > - peri_mmuen_msk = BIT(portid); > > - /* PCI dev has only one output id, enable the > > next writing bit for PCIe */ > > - if (dev_is_pci(dev)) > > - peri_mmuen_msk |= BIT(portid + 1); > > - > > - peri_mmuen = enable ? peri_mmuen_msk : 0; > > - ret = regmap_update_bits(data->pericfg, > > PERICFG_IOMMU_1, > > - peri_mmuen_msk, > > peri_mmuen); > > + if (MTK_IOMMU_HAS_FLAG(data->plat_data, > > CFG_IFA_MASTER_IN_ATF)) { > > + struct arm_smccc_res res; > > + > > + portid = MTK_M4U_TO_PORT(fwspec- > > >ids[i]); > > This assignment is redundant, as portid is initialized to the same > value > just a few lines before. Please drop it. > > Everything else looks good. > > Regards, > Angelo Thanks for your review. This assignment will be dropped in the next version. Best regards, Chengci Xu > > > + arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONT > > ROL, > > + IOMMU_ATF_CMD_CONFIG_INFR > > A_IOMMU, > > + portid, enable, 0, 0, 0, > > 0, &res); > > + ret = (int)res.a0; > >
On Wed, 2022-09-07 at 11:37 +0800, Yong Wu wrote: > On Wed, 2022-08-31 at 20:55 +0800, Chengci.Xu wrote: > > The register which can enable IOMMU for INFRA master should be > > setted > > in secure world for security concerns. Therefore, we add a SMC > > command > > for INFRA master to enable/disable INFRA IOMMU in ATF. This > > function > > is > > prepared for MT8188. > > > > Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com> > > --- > > drivers/iommu/mtk_iommu.c | 34 ++++++++++++++++++++++++++-------- > > include/soc/mediatek/smi.h | 1 + > > 2 files changed, 27 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index 7e363b1f24df..6fe780783ec8 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.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/bitfield.h> > > #include <linux/bug.h> > > #include <linux/clk.h> > > @@ -28,6 +29,7 @@ > > #include <linux/slab.h> > > #include <linux/spinlock.h> > > #include <linux/soc/mediatek/infracfg.h> > > +#include <linux/soc/mediatek/mtk_sip_svc.h> > > #include <asm/barrier.h> > > #include <soc/mediatek/smi.h> > > > > @@ -138,6 +140,7 @@ > > #define PM_CLK_AO BIT(15) > > #define IFA_IOMMU_PCIE_SUPPORT BIT(16) > > #define PGTABLE_PA_35_EN BIT(17) > > +#define CFG_IFA_MASTER_IN_ATF BIT(18) > > > > #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \ > > ((((pdata)->flags) & (mask)) == (_x)) > > @@ -554,14 +557,29 @@ static int mtk_iommu_config(struct > > mtk_iommu_data *data, struct device *dev, > > else > > larb_mmu->mmu &= > > ~MTK_SMI_MMU_EN(portid); > > } else if (MTK_IOMMU_IS_TYPE(data->plat_data, > > MTK_IOMMU_TYPE_INFRA)) { > > - peri_mmuen_msk = BIT(portid); > > - /* PCI dev has only one output id, enable the > > next writing bit for PCIe */ > > - if (dev_is_pci(dev)) > > - peri_mmuen_msk |= BIT(portid + 1); > > - > > - peri_mmuen = enable ? peri_mmuen_msk : 0; > > - ret = regmap_update_bits(data->pericfg, > > PERICFG_IOMMU_1, > > - peri_mmuen_msk, > > peri_mmuen); > > + if (MTK_IOMMU_HAS_FLAG(data->plat_data, > > CFG_IFA_MASTER_IN_ATF)) { > > + struct arm_smccc_res res; > > + > > + portid = MTK_M4U_TO_PORT(fwspec- > > > ids[i]); > > > > + arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONT > > ROL, > > + IOMMU_ATF_CMD_CONFIG_INFR > > A_IOMMU, > > + portid, enable, 0, 0, 0, > > 0, &res); > > + ret = (int)res.a0; > > Too many indentations. Please create a new interface or even just use > "if", like: > > if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) { > xx > } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA && > MTK_IOMMU_HAS_FLAG(data->plat_data, > CFG_IFA_MASTER_IN_ATF)) > { /* Configure the mmu_en in the ATF for infra IOMMU */ > xx > } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) { > } > Thanks for your review. We will use the second way to reduce indentations. Best regards, Chengci Xu > > + > > + } else { > > + peri_mmuen_msk = BIT(portid); > > + /* PCI dev has only one output id, > > + * enable the next writing bit for PCIe > > + */ > > + if (dev_is_pci(dev)) > > + peri_mmuen_msk |= BIT(portid + > > 1); > > + > > + peri_mmuen = enable ? peri_mmuen_msk : > > 0; > > + ret = regmap_update_bits(data->pericfg, > > + PERICFG_IOMMU_ > > 1, > > + peri_mmuen_msk > > , > > + peri_mmuen); > > + } > > if (ret) > > dev_err(dev, "%s iommu(%s) inframaster > > 0x%x fail(%d).\n", > > enable ? "enable" : "disable", > > diff --git a/include/soc/mediatek/smi.h > > b/include/soc/mediatek/smi.h > > index dfd8efca5e60..99f13b0e416d 100644 > > --- a/include/soc/mediatek/smi.h > > +++ b/include/soc/mediatek/smi.h > > @@ -13,6 +13,7 @@ > > > > enum iommu_atf_cmd { > > IOMMU_ATF_CMD_CONFIG_SMI_LARB, /* For mm master to > > en/disable iommu */ > > + IOMMU_ATF_CMD_CONFIG_INFRA_IOMMU, /* For infra master > > en/disable iommu */ > > IOMMU_ATF_CMD_MAX, > > }; > > > >
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index 7e363b1f24df..6fe780783ec8 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.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/bitfield.h> #include <linux/bug.h> #include <linux/clk.h> @@ -28,6 +29,7 @@ #include <linux/slab.h> #include <linux/spinlock.h> #include <linux/soc/mediatek/infracfg.h> +#include <linux/soc/mediatek/mtk_sip_svc.h> #include <asm/barrier.h> #include <soc/mediatek/smi.h> @@ -138,6 +140,7 @@ #define PM_CLK_AO BIT(15) #define IFA_IOMMU_PCIE_SUPPORT BIT(16) #define PGTABLE_PA_35_EN BIT(17) +#define CFG_IFA_MASTER_IN_ATF BIT(18) #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \ ((((pdata)->flags) & (mask)) == (_x)) @@ -554,14 +557,29 @@ static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev, else larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid); } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA)) { - peri_mmuen_msk = BIT(portid); - /* PCI dev has only one output id, enable the next writing bit for PCIe */ - if (dev_is_pci(dev)) - peri_mmuen_msk |= BIT(portid + 1); - - peri_mmuen = enable ? peri_mmuen_msk : 0; - ret = regmap_update_bits(data->pericfg, PERICFG_IOMMU_1, - peri_mmuen_msk, peri_mmuen); + if (MTK_IOMMU_HAS_FLAG(data->plat_data, CFG_IFA_MASTER_IN_ATF)) { + struct arm_smccc_res res; + + portid = MTK_M4U_TO_PORT(fwspec->ids[i]); + arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONTROL, + IOMMU_ATF_CMD_CONFIG_INFRA_IOMMU, + portid, enable, 0, 0, 0, 0, &res); + ret = (int)res.a0; + + } else { + peri_mmuen_msk = BIT(portid); + /* PCI dev has only one output id, + * enable the next writing bit for PCIe + */ + if (dev_is_pci(dev)) + peri_mmuen_msk |= BIT(portid + 1); + + peri_mmuen = enable ? peri_mmuen_msk : 0; + ret = regmap_update_bits(data->pericfg, + PERICFG_IOMMU_1, + peri_mmuen_msk, + peri_mmuen); + } if (ret) dev_err(dev, "%s iommu(%s) inframaster 0x%x fail(%d).\n", enable ? "enable" : "disable", diff --git a/include/soc/mediatek/smi.h b/include/soc/mediatek/smi.h index dfd8efca5e60..99f13b0e416d 100644 --- a/include/soc/mediatek/smi.h +++ b/include/soc/mediatek/smi.h @@ -13,6 +13,7 @@ enum iommu_atf_cmd { IOMMU_ATF_CMD_CONFIG_SMI_LARB, /* For mm master to en/disable iommu */ + IOMMU_ATF_CMD_CONFIG_INFRA_IOMMU, /* For infra master en/disable iommu */ IOMMU_ATF_CMD_MAX, };
The register which can enable IOMMU for INFRA master should be setted in secure world for security concerns. Therefore, we add a SMC command for INFRA master to enable/disable INFRA IOMMU in ATF. This function is prepared for MT8188. Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com> --- drivers/iommu/mtk_iommu.c | 34 ++++++++++++++++++++++++++-------- include/soc/mediatek/smi.h | 1 + 2 files changed, 27 insertions(+), 8 deletions(-)