| Message ID | 1596766948-29767-3-git-send-email-neal.liu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/2] dt-bindings: devapc: add bindings for mtk-devapc | expand |
Hi, Neal: Neal Liu <neal.liu@mediatek.com> 於 2020年8月7日 週五 上午10:34寫道： > > MediaTek bus fabric provides TrustZone security support and data > protection to prevent slaves from being accessed by unexpected > masters. > The security violation is logged and sent to the processor for > further analysis or countermeasures. > > Any occurrence of security violation would raise an interrupt, and > it will be handled by mtk-devapc driver. The violation > information is printed in order to find the murderer. > > Signed-off-by: Neal Liu <neal.liu@mediatek.com> > --- [snip] > + > +#define PHY_DEVAPC_TIMEOUT 0x10000 > + > +/* > + * devapc_sync_vio_dbg - do "shift" mechansim" to get full violation information. > + * shift mechanism is depends on devapc hardware design. > + * Mediatek devapc set multiple slaves as a group. > + * When violation is triggered, violation info is kept > + * inside devapc hardware. > + * Driver should do shift mechansim to sync full violation > + * info to VIO_DBGs registers. > + * > + */ > +static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx) > +{ > + void __iomem *pd_vio_shift_sta_reg; > + void __iomem *pd_vio_shift_sel_reg; > + void __iomem *pd_vio_shift_con_reg; > + int min_shift_group; > + int ret; > + u32 val; > + > + pd_vio_shift_sta_reg = ctx->infra_base + > + ctx->data->vio_shift_sta_offset; > + pd_vio_shift_sel_reg = ctx->infra_base + > + ctx->data->vio_shift_sel_offset; > + pd_vio_shift_con_reg = ctx->infra_base + > + ctx->data->vio_shift_con_offset; > + > + /* Find the minimum shift group which has violation */ > + val = readl(pd_vio_shift_sta_reg); > + if (!val) > + return false; > + > + min_shift_group = __ffs(val); > + > + /* Assign the group to sync */ > + writel(0x1 << min_shift_group, pd_vio_shift_sel_reg); > + > + /* Start syncing */ > + writel(0x1, pd_vio_shift_con_reg); > + > + ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0, > + PHY_DEVAPC_TIMEOUT); > + if (ret) { > + dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__); > + return false; > + } > + > + /* Stop syncing */ > + writel(0x0, pd_vio_shift_con_reg); > + writel(0x0, pd_vio_shift_sel_reg); This is redundant because you set this register before start syncing. > + writel(0x1 << min_shift_group, pd_vio_shift_sta_reg); You read this register to find minimum shift group, but you write it back into this register, so this function would get the same minimum shift group in next time, isn't it? > + > + return true; > +} > + > +/* > + * devapc_extract_vio_dbg - extract full violation information after doing > + * shift mechanism. > + */ > +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx) > +{ > + struct mtk_devapc_vio_dbgs *vio_dbgs; struct mtk_devapc_vio_dbgs vio_dbgs; Use stack instead of allocating from heap. > + void __iomem *vio_dbg0_reg; > + void __iomem *vio_dbg1_reg; > + > + vio_dbgs = devm_kzalloc(ctx->dev, sizeof(struct mtk_devapc_vio_dbgs), > + GFP_KERNEL); > + if (!vio_dbgs) > + return; > + > + vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset; > + vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset; > + > + vio_dbgs->vio_dbg0 = readl(vio_dbg0_reg); > + vio_dbgs->vio_dbg1 = readl(vio_dbg1_reg); > + > + /* Print violation information */ > + if (vio_dbgs->dbg0_bits.vio_w) > + dev_info(ctx->dev, "Write Violation\n"); > + else if (vio_dbgs->dbg0_bits.vio_r) > + dev_info(ctx->dev, "Read Violation\n"); > + > + dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n", > + vio_dbgs->dbg0_bits.mstid, vio_dbgs->dbg0_bits.dmnid, > + vio_dbgs->vio_dbg1); > +} > + [snip] > + > +/* > + * start_devapc - unmask slave's irq to start receiving devapc violation. > + */ > +static void start_devapc(struct mtk_devapc_context *ctx) > +{ > + void __iomem *pd_apc_con_reg; > + > + pd_apc_con_reg = ctx->infra_base + ctx->data->apc_con_offset; > + writel(BIT(31), pd_apc_con_reg); pd_apc_con_reg is used once, so writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset); > + > + mask_module_irq(ctx, false); > +} > + [snip] > + > +static int mtk_devapc_probe(struct platform_device *pdev) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct mtk_devapc_context *ctx; > + u32 devapc_irq; > + int ret; > + > + if (IS_ERR(node)) > + return -ENODEV; > + > + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + ctx->data = of_device_get_match_data(&pdev->dev); > + ctx->dev = &pdev->dev; > + > + ctx->infra_base = of_iomap(node, 0); > + if (!ctx->infra_base) > + return -EINVAL; > + > + devapc_irq = irq_of_parse_and_map(node, 0); > + if (!devapc_irq) > + return -EINVAL; > + > + ctx->infra_clk = devm_clk_get(&pdev->dev, "devapc-infra-clock"); > + if (IS_ERR(ctx->infra_clk)) > + return -EINVAL; > + > + if (clk_prepare_enable(ctx->infra_clk)) > + return -EINVAL; > + > + ret = devm_request_irq(&pdev->dev, devapc_irq, > + (irq_handler_t)devapc_violation_irq, > + IRQF_TRIGGER_NONE, "devapc", ctx); > + if (ret) { > + clk_disable_unprepare(ctx->infra_clk); > + return ret; > + } > + > + platform_set_drvdata(pdev, ctx); > + > + start_devapc(ctx); > + > + return 0; > +} > + > +static int mtk_devapc_remove(struct platform_device *pdev) > +{ > + struct mtk_devapc_context *ctx = platform_get_drvdata(pdev); > + stop_devapc(ctx); Regards, Chun-Kuang. > + if (ctx->infra_clk) > + clk_disable_unprepare(ctx->infra_clk); > + > + return 0; > +} > +
Hi Chun-Kuang, On Fri, 2020-08-07 at 23:52 +0800, Chun-Kuang Hu wrote: > Hi, Neal: > > Neal Liu <neal.liu@mediatek.com> 於 2020年8月7日 週五 上午10:34寫道： > > > > MediaTek bus fabric provides TrustZone security support and data > > protection to prevent slaves from being accessed by unexpected > > masters. > > The security violation is logged and sent to the processor for > > further analysis or countermeasures. > > > > Any occurrence of security violation would raise an interrupt, and > > it will be handled by mtk-devapc driver. The violation > > information is printed in order to find the murderer. > > > > Signed-off-by: Neal Liu <neal.liu@mediatek.com> > > --- > > [snip] > > > + > > +#define PHY_DEVAPC_TIMEOUT 0x10000 > > + > > +/* > > + * devapc_sync_vio_dbg - do "shift" mechansim" to get full violation information. > > + * shift mechanism is depends on devapc hardware design. > > + * Mediatek devapc set multiple slaves as a group. > > + * When violation is triggered, violation info is kept > > + * inside devapc hardware. > > + * Driver should do shift mechansim to sync full violation > > + * info to VIO_DBGs registers. > > + * > > + */ > > +static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx) > > +{ > > + void __iomem *pd_vio_shift_sta_reg; > > + void __iomem *pd_vio_shift_sel_reg; > > + void __iomem *pd_vio_shift_con_reg; > > + int min_shift_group; > > + int ret; > > + u32 val; > > + > > + pd_vio_shift_sta_reg = ctx->infra_base + > > + ctx->data->vio_shift_sta_offset; > > + pd_vio_shift_sel_reg = ctx->infra_base + > > + ctx->data->vio_shift_sel_offset; > > + pd_vio_shift_con_reg = ctx->infra_base + > > + ctx->data->vio_shift_con_offset; > > + > > + /* Find the minimum shift group which has violation */ > > + val = readl(pd_vio_shift_sta_reg); > > + if (!val) > > + return false; > > + > > + min_shift_group = __ffs(val); > > + > > + /* Assign the group to sync */ > > + writel(0x1 << min_shift_group, pd_vio_shift_sel_reg); > > + > > + /* Start syncing */ > > + writel(0x1, pd_vio_shift_con_reg); > > + > > + ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0, > > + PHY_DEVAPC_TIMEOUT); > > + if (ret) { > > + dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__); > > + return false; > > + } > > + > > + /* Stop syncing */ > > + writel(0x0, pd_vio_shift_con_reg); > > + writel(0x0, pd_vio_shift_sel_reg); > > This is redundant because you set this register before start syncing. No, we don't set this reg before start syncing. > > > + writel(0x1 << min_shift_group, pd_vio_shift_sta_reg); > > You read this register to find minimum shift group, but you write it > back into this register, so this function would get the same minimum > shift group in next time, isn't it? No. The operation means write clear. We won't get the same minimum shift group after clear this bit. > > > + > > + return true; > > +} > > + > > +/* > > + * devapc_extract_vio_dbg - extract full violation information after doing > > + * shift mechanism. > > + */ > > +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx) > > +{ > > + struct mtk_devapc_vio_dbgs *vio_dbgs; > > struct mtk_devapc_vio_dbgs vio_dbgs; > > Use stack instead of allocating from heap. Why it cannot use heap if the memory is handled correctly? > > > + void __iomem *vio_dbg0_reg; > > + void __iomem *vio_dbg1_reg; > > + > > + vio_dbgs = devm_kzalloc(ctx->dev, sizeof(struct mtk_devapc_vio_dbgs), > > + GFP_KERNEL); > > + if (!vio_dbgs) > > + return; > > + > > + vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset; > > + vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset; > > + > > + vio_dbgs->vio_dbg0 = readl(vio_dbg0_reg); > > + vio_dbgs->vio_dbg1 = readl(vio_dbg1_reg); > > + > > + /* Print violation information */ > > + if (vio_dbgs->dbg0_bits.vio_w) > > + dev_info(ctx->dev, "Write Violation\n"); > > + else if (vio_dbgs->dbg0_bits.vio_r) > > + dev_info(ctx->dev, "Read Violation\n"); > > + > > + dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n", > > + vio_dbgs->dbg0_bits.mstid, vio_dbgs->dbg0_bits.dmnid, > > + vio_dbgs->vio_dbg1); > > +} > > + > > [snip] > > > + > > +/* > > + * start_devapc - unmask slave's irq to start receiving devapc violation. > > + */ > > +static void start_devapc(struct mtk_devapc_context *ctx) > > +{ > > + void __iomem *pd_apc_con_reg; > > + > > + pd_apc_con_reg = ctx->infra_base + ctx->data->apc_con_offset; > > + writel(BIT(31), pd_apc_con_reg); > > pd_apc_con_reg is used once, so > > writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset); Okay, I'll merge it on next patch. Thanks ! > > > + > > + mask_module_irq(ctx, false); > > +} > > + > > [snip] > > > + > > +static int mtk_devapc_probe(struct platform_device *pdev) > > +{ > > + struct device_node *node = pdev->dev.of_node; > > + struct mtk_devapc_context *ctx; > > + u32 devapc_irq; > > + int ret; > > + > > + if (IS_ERR(node)) > > + return -ENODEV; > > + > > + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > > + if (!ctx) > > + return -ENOMEM; > > + > > + ctx->data = of_device_get_match_data(&pdev->dev); > > + ctx->dev = &pdev->dev; > > + > > + ctx->infra_base = of_iomap(node, 0); > > + if (!ctx->infra_base) > > + return -EINVAL; > > + > > + devapc_irq = irq_of_parse_and_map(node, 0); > > + if (!devapc_irq) > > + return -EINVAL; > > + > > + ctx->infra_clk = devm_clk_get(&pdev->dev, "devapc-infra-clock"); > > + if (IS_ERR(ctx->infra_clk)) > > + return -EINVAL; > > + > > + if (clk_prepare_enable(ctx->infra_clk)) > > + return -EINVAL; > > + > > + ret = devm_request_irq(&pdev->dev, devapc_irq, > > + (irq_handler_t)devapc_violation_irq, > > + IRQF_TRIGGER_NONE, "devapc", ctx); > > + if (ret) { > > + clk_disable_unprepare(ctx->infra_clk); > > + return ret; > > + } > > + > > + platform_set_drvdata(pdev, ctx); > > + > > + start_devapc(ctx); > > + > > + return 0; > > +} > > + > > +static int mtk_devapc_remove(struct platform_device *pdev) > > +{ > > + struct mtk_devapc_context *ctx = platform_get_drvdata(pdev); > > + > > stop_devapc(ctx); We don't have to do any further operations to stop devapc hw. > > Regards, > Chun-Kuang. > > > + if (ctx->infra_clk) > > + clk_disable_unprepare(ctx->infra_clk); > > + > > + return 0; > > +} > > +
Hi, Neal: Neal Liu <neal.liu@mediatek.com> 於 2020年8月10日 週一 上午11:43寫道： > > Hi Chun-Kuang, > > On Fri, 2020-08-07 at 23:52 +0800, Chun-Kuang Hu wrote: > > Hi, Neal: > > > > Neal Liu <neal.liu@mediatek.com> 於 2020年8月7日 週五 上午10:34寫道： > > > > > > MediaTek bus fabric provides TrustZone security support and data > > > protection to prevent slaves from being accessed by unexpected > > > masters. > > > The security violation is logged and sent to the processor for > > > further analysis or countermeasures. > > > > > > Any occurrence of security violation would raise an interrupt, and > > > it will be handled by mtk-devapc driver. The violation > > > information is printed in order to find the murderer. > > > > > > Signed-off-by: Neal Liu <neal.liu@mediatek.com> > > > --- > > > > [snip] > > > > > + > > > +#define PHY_DEVAPC_TIMEOUT 0x10000 > > > + > > > +/* > > > + * devapc_sync_vio_dbg - do "shift" mechansim" to get full violation information. > > > + * shift mechanism is depends on devapc hardware design. > > > + * Mediatek devapc set multiple slaves as a group. > > > + * When violation is triggered, violation info is kept > > > + * inside devapc hardware. > > > + * Driver should do shift mechansim to sync full violation > > > + * info to VIO_DBGs registers. > > > + * > > > + */ > > > +static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx) > > > +{ > > > + void __iomem *pd_vio_shift_sta_reg; > > > + void __iomem *pd_vio_shift_sel_reg; > > > + void __iomem *pd_vio_shift_con_reg; > > > + int min_shift_group; > > > + int ret; > > > + u32 val; > > > + > > > + pd_vio_shift_sta_reg = ctx->infra_base + > > > + ctx->data->vio_shift_sta_offset; > > > + pd_vio_shift_sel_reg = ctx->infra_base + > > > + ctx->data->vio_shift_sel_offset; > > > + pd_vio_shift_con_reg = ctx->infra_base + > > > + ctx->data->vio_shift_con_offset; > > > + > > > + /* Find the minimum shift group which has violation */ > > > + val = readl(pd_vio_shift_sta_reg); > > > + if (!val) > > > + return false; > > > + > > > + min_shift_group = __ffs(val); > > > + > > > + /* Assign the group to sync */ > > > + writel(0x1 << min_shift_group, pd_vio_shift_sel_reg); > > > + > > > + /* Start syncing */ > > > + writel(0x1, pd_vio_shift_con_reg); > > > + > > > + ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0, > > > + PHY_DEVAPC_TIMEOUT); > > > + if (ret) { > > > + dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__); > > > + return false; > > > + } > > > + > > > + /* Stop syncing */ > > > + writel(0x0, pd_vio_shift_con_reg); > > > + writel(0x0, pd_vio_shift_sel_reg); > > > > This is redundant because you set this register before start syncing. > > No, we don't set this reg before start syncing. > I'm talking about pd_vio_shift_sel_reg, and I find this before start syncing: /* Assign the group to sync */ writel(0x1 << min_shift_group, pd_vio_shift_sel_reg); /* Start syncing */ writel(0x1, pd_vio_shift_con_reg); > > > > > + writel(0x1 << min_shift_group, pd_vio_shift_sta_reg); > > > > You read this register to find minimum shift group, but you write it > > back into this register, so this function would get the same minimum > > shift group in next time, isn't it? > > No. The operation means write clear. We won't get the same minimum shift > group after clear this bit. > Add comment for this because this is not trivial. > > > > > + > > > + return true; > > > +} > > > + > > > +/* > > > + * devapc_extract_vio_dbg - extract full violation information after doing > > > + * shift mechanism. > > > + */ > > > +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx) > > > +{ > > > + struct mtk_devapc_vio_dbgs *vio_dbgs; > > > > struct mtk_devapc_vio_dbgs vio_dbgs; > > > > Use stack instead of allocating from heap. > > Why it cannot use heap if the memory is handled correctly? > You could use heap but allocating memory from heap would cost much time. In the worst case, it would trigger buddy system to break a page for slub. Using stack cost almost no time, but it has some limitation. Stack memory is small and it should be used for local variable, and vio_dbgs match this limitation, so stack is better than heap. > > > > > + void __iomem *vio_dbg0_reg; > > > + void __iomem *vio_dbg1_reg; > > > + > > > + vio_dbgs = devm_kzalloc(ctx->dev, sizeof(struct mtk_devapc_vio_dbgs), > > > + GFP_KERNEL); > > > + if (!vio_dbgs) > > > + return; > > > + > > > + vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset; > > > + vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset; > > > + > > > + vio_dbgs->vio_dbg0 = readl(vio_dbg0_reg); > > > + vio_dbgs->vio_dbg1 = readl(vio_dbg1_reg); > > > + > > > + /* Print violation information */ > > > + if (vio_dbgs->dbg0_bits.vio_w) > > > + dev_info(ctx->dev, "Write Violation\n"); > > > + else if (vio_dbgs->dbg0_bits.vio_r) > > > + dev_info(ctx->dev, "Read Violation\n"); > > > + > > > + dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n", > > > + vio_dbgs->dbg0_bits.mstid, vio_dbgs->dbg0_bits.dmnid, > > > + vio_dbgs->vio_dbg1); > > > +} > > > + > > > > [snip] > > > > > + > > > +static int mtk_devapc_probe(struct platform_device *pdev) > > > +{ > > > + struct device_node *node = pdev->dev.of_node; > > > + struct mtk_devapc_context *ctx; > > > + u32 devapc_irq; > > > + int ret; > > > + > > > + if (IS_ERR(node)) > > > + return -ENODEV; > > > + > > > + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > > > + if (!ctx) > > > + return -ENOMEM; > > > + > > > + ctx->data = of_device_get_match_data(&pdev->dev); > > > + ctx->dev = &pdev->dev; > > > + > > > + ctx->infra_base = of_iomap(node, 0); > > > + if (!ctx->infra_base) > > > + return -EINVAL; > > > + > > > + devapc_irq = irq_of_parse_and_map(node, 0); > > > + if (!devapc_irq) > > > + return -EINVAL; > > > + > > > + ctx->infra_clk = devm_clk_get(&pdev->dev, "devapc-infra-clock"); > > > + if (IS_ERR(ctx->infra_clk)) > > > + return -EINVAL; > > > + > > > + if (clk_prepare_enable(ctx->infra_clk)) > > > + return -EINVAL; > > > + > > > + ret = devm_request_irq(&pdev->dev, devapc_irq, > > > + (irq_handler_t)devapc_violation_irq, > > > + IRQF_TRIGGER_NONE, "devapc", ctx); > > > + if (ret) { > > > + clk_disable_unprepare(ctx->infra_clk); > > > + return ret; > > > + } > > > + > > > + platform_set_drvdata(pdev, ctx); > > > + > > > + start_devapc(ctx); > > > + > > > + return 0; > > > +} > > > + > > > +static int mtk_devapc_remove(struct platform_device *pdev) > > > +{ > > > + struct mtk_devapc_context *ctx = platform_get_drvdata(pdev); > > > + > > > > stop_devapc(ctx); > > We don't have to do any further operations to stop devapc hw. > After this driver is removed, I think we should restore hardware to the status before probing. Before probe(), devapc hardware is stopped (pd_apc_con_reg is a default value and all vio irq is masked), so it should be the same status after remove(). This concept is the same as what you do for infra_clk. Regards, Chun-Kuang. > > > > Regards, > > Chun-Kuang. > > > > > + if (ctx->infra_clk) > > > + clk_disable_unprepare(ctx->infra_clk); > > > + > > > + return 0; > > > +} > > > + >
Hi Chun-Kuang, On Tue, 2020-08-11 at 07:14 +0800, Chun-Kuang Hu wrote: > Hi, Neal: > > Neal Liu <neal.liu@mediatek.com> 於 2020年8月10日 週一 上午11:43寫道： > > > > Hi Chun-Kuang, > > > > On Fri, 2020-08-07 at 23:52 +0800, Chun-Kuang Hu wrote: > > > Hi, Neal: > > > > > > Neal Liu <neal.liu@mediatek.com> 於 2020年8月7日 週五 上午10:34寫道： > > > > > > > > MediaTek bus fabric provides TrustZone security support and data > > > > protection to prevent slaves from being accessed by unexpected > > > > masters. > > > > The security violation is logged and sent to the processor for > > > > further analysis or countermeasures. > > > > > > > > Any occurrence of security violation would raise an interrupt, and > > > > it will be handled by mtk-devapc driver. The violation > > > > information is printed in order to find the murderer. > > > > > > > > Signed-off-by: Neal Liu <neal.liu@mediatek.com> > > > > --- > > > > > > [snip] > > > > > > > + > > > > +#define PHY_DEVAPC_TIMEOUT 0x10000 > > > > + > > > > +/* > > > > + * devapc_sync_vio_dbg - do "shift" mechansim" to get full violation information. > > > > + * shift mechanism is depends on devapc hardware design. > > > > + * Mediatek devapc set multiple slaves as a group. > > > > + * When violation is triggered, violation info is kept > > > > + * inside devapc hardware. > > > > + * Driver should do shift mechansim to sync full violation > > > > + * info to VIO_DBGs registers. > > > > + * > > > > + */ > > > > +static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx) > > > > +{ > > > > + void __iomem *pd_vio_shift_sta_reg; > > > > + void __iomem *pd_vio_shift_sel_reg; > > > > + void __iomem *pd_vio_shift_con_reg; > > > > + int min_shift_group; > > > > + int ret; > > > > + u32 val; > > > > + > > > > + pd_vio_shift_sta_reg = ctx->infra_base + > > > > + ctx->data->vio_shift_sta_offset; > > > > + pd_vio_shift_sel_reg = ctx->infra_base + > > > > + ctx->data->vio_shift_sel_offset; > > > > + pd_vio_shift_con_reg = ctx->infra_base + > > > > + ctx->data->vio_shift_con_offset; > > > > + > > > > + /* Find the minimum shift group which has violation */ > > > > + val = readl(pd_vio_shift_sta_reg); > > > > + if (!val) > > > > + return false; > > > > + > > > > + min_shift_group = __ffs(val); > > > > + > > > > + /* Assign the group to sync */ > > > > + writel(0x1 << min_shift_group, pd_vio_shift_sel_reg); > > > > + > > > > + /* Start syncing */ > > > > + writel(0x1, pd_vio_shift_con_reg); > > > > + > > > > + ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0, > > > > + PHY_DEVAPC_TIMEOUT); > > > > + if (ret) { > > > > + dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__); > > > > + return false; > > > > + } > > > > + > > > > + /* Stop syncing */ > > > > + writel(0x0, pd_vio_shift_con_reg); > > > > + writel(0x0, pd_vio_shift_sel_reg); > > > > > > This is redundant because you set this register before start syncing. > > > > No, we don't set this reg before start syncing. > > > > I'm talking about pd_vio_shift_sel_reg, and I find this before start syncing: > > /* Assign the group to sync */ > writel(0x1 << min_shift_group, pd_vio_shift_sel_reg); > > /* Start syncing */ > writel(0x1, pd_vio_shift_con_reg); > We set 0 to make sure all bits are cleared. But it won't cause any problem if we remove it. I'll update on next patch. Thanks ! > > > > > > > + writel(0x1 << min_shift_group, pd_vio_shift_sta_reg); > > > > > > You read this register to find minimum shift group, but you write it > > > back into this register, so this function would get the same minimum > > > shift group in next time, isn't it? > > > > No. The operation means write clear. We won't get the same minimum shift > > group after clear this bit. > > > > Add comment for this because this is not trivial. Okay. > > > > > > > > + > > > > + return true; > > > > +} > > > > + > > > > +/* > > > > + * devapc_extract_vio_dbg - extract full violation information after doing > > > > + * shift mechanism. > > > > + */ > > > > +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx) > > > > +{ > > > > + struct mtk_devapc_vio_dbgs *vio_dbgs; > > > > > > struct mtk_devapc_vio_dbgs vio_dbgs; > > > > > > Use stack instead of allocating from heap. > > > > Why it cannot use heap if the memory is handled correctly? > > > > You could use heap but allocating memory from heap would cost much > time. In the worst case, it would trigger buddy system to break a page > for slub. Using stack cost almost no time, but it has some limitation. > Stack memory is small and it should be used for local variable, and > vio_dbgs match this limitation, so stack is better than heap. > Okay, it make sense. I'll update on next patch. Thanks ! > > > > > > > + void __iomem *vio_dbg0_reg; > > > > + void __iomem *vio_dbg1_reg; > > > > + > > > > + vio_dbgs = devm_kzalloc(ctx->dev, sizeof(struct mtk_devapc_vio_dbgs), > > > > + GFP_KERNEL); > > > > + if (!vio_dbgs) > > > > + return; > > > > + > > > > + vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset; > > > > + vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset; > > > > + > > > > + vio_dbgs->vio_dbg0 = readl(vio_dbg0_reg); > > > > + vio_dbgs->vio_dbg1 = readl(vio_dbg1_reg); > > > > + > > > > + /* Print violation information */ > > > > + if (vio_dbgs->dbg0_bits.vio_w) > > > > + dev_info(ctx->dev, "Write Violation\n"); > > > > + else if (vio_dbgs->dbg0_bits.vio_r) > > > > + dev_info(ctx->dev, "Read Violation\n"); > > > > + > > > > + dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n", > > > > + vio_dbgs->dbg0_bits.mstid, vio_dbgs->dbg0_bits.dmnid, > > > > + vio_dbgs->vio_dbg1); > > > > +} > > > > + > > > > > > [snip] > > > > > > > + > > > > +static int mtk_devapc_probe(struct platform_device *pdev) > > > > +{ > > > > + struct device_node *node = pdev->dev.of_node; > > > > + struct mtk_devapc_context *ctx; > > > > + u32 devapc_irq; > > > > + int ret; > > > > + > > > > + if (IS_ERR(node)) > > > > + return -ENODEV; > > > > + > > > > + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > > > > + if (!ctx) > > > > + return -ENOMEM; > > > > + > > > > + ctx->data = of_device_get_match_data(&pdev->dev); > > > > + ctx->dev = &pdev->dev; > > > > + > > > > + ctx->infra_base = of_iomap(node, 0); > > > > + if (!ctx->infra_base) > > > > + return -EINVAL; > > > > + > > > > + devapc_irq = irq_of_parse_and_map(node, 0); > > > > + if (!devapc_irq) > > > > + return -EINVAL; > > > > + > > > > + ctx->infra_clk = devm_clk_get(&pdev->dev, "devapc-infra-clock"); > > > > + if (IS_ERR(ctx->infra_clk)) > > > > + return -EINVAL; > > > > + > > > > + if (clk_prepare_enable(ctx->infra_clk)) > > > > + return -EINVAL; > > > > + > > > > + ret = devm_request_irq(&pdev->dev, devapc_irq, > > > > + (irq_handler_t)devapc_violation_irq, > > > > + IRQF_TRIGGER_NONE, "devapc", ctx); > > > > + if (ret) { > > > > + clk_disable_unprepare(ctx->infra_clk); > > > > + return ret; > > > > + } > > > > + > > > > + platform_set_drvdata(pdev, ctx); > > > > + > > > > + start_devapc(ctx); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int mtk_devapc_remove(struct platform_device *pdev) > > > > +{ > > > > + struct mtk_devapc_context *ctx = platform_get_drvdata(pdev); > > > > + > > > > > > stop_devapc(ctx); > > > > We don't have to do any further operations to stop devapc hw. > > > > After this driver is removed, I think we should restore hardware to > the status before probing. Before probe(), devapc hardware is stopped > (pd_apc_con_reg is a default value and all vio irq is masked), so it > should be the same status after remove(). This concept is the same as > what you do for infra_clk. Okay, it make sense. I'll add stop_devapc() on next patch. Thanks ! > > Regards, > Chun-Kuang. > > > > > > > Regards, > > > Chun-Kuang. > > > > > > > + if (ctx->infra_clk) > > > > + clk_disable_unprepare(ctx->infra_clk); > > > > + > > > > + return 0; > > > > +} > > > > + > >
diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig index 59a56cd..1177c98 100644 --- a/drivers/soc/mediatek/Kconfig +++ b/drivers/soc/mediatek/Kconfig @@ -17,6 +17,15 @@ config MTK_CMDQ time limitation, such as updating display configuration during the vblank. +config MTK_DEVAPC + tristate "Mediatek Device APC Support" + help + Say yes here to enable support for Mediatek Device APC driver. + This driver is mainly used to handle the violation which catches + unexpected transaction. + The violation information is logged for further analysis or + countermeasures. + config MTK_INFRACFG bool "MediaTek INFRACFG Support" select REGMAP diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile index 01f9f87..abfd4ba 100644 --- a/drivers/soc/mediatek/Makefile +++ b/drivers/soc/mediatek/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o +obj-$(CONFIG_MTK_DEVAPC) += mtk-devapc.o obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c new file mode 100644 index 0000000..73287cf --- /dev/null +++ b/drivers/soc/mediatek/mtk-devapc.c @@ -0,0 +1,315 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2020 MediaTek Inc. + */ + +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/iopoll.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/of_device.h> +#include <linux/of_irq.h> +#include <linux/of_address.h> + +#define VIO_MOD_TO_REG_IND(m) ((m) / 32) +#define VIO_MOD_TO_REG_OFF(m) ((m) % 32) + +struct mtk_devapc_vio_dbgs { + union { + u32 vio_dbg0; + struct { + u32 mstid:16; + u32 dmnid:6; + u32 vio_w:1; + u32 vio_r:1; + u32 addr_h:4; + u32 resv:4; + } dbg0_bits; + }; + + u32 vio_dbg1; +}; + +struct mtk_devapc_data { + u32 vio_idx_num; + u32 vio_mask_offset; + u32 vio_sta_offset; + u32 vio_dbg0_offset; + u32 vio_dbg1_offset; + u32 apc_con_offset; + u32 vio_shift_sta_offset; + u32 vio_shift_sel_offset; + u32 vio_shift_con_offset; +}; + +struct mtk_devapc_context { + struct device *dev; + void __iomem *infra_base; + struct clk *infra_clk; + const struct mtk_devapc_data *data; +}; + +static void clear_vio_status(struct mtk_devapc_context *ctx) +{ + void __iomem *reg; + int i; + + reg = ctx->infra_base + ctx->data->vio_sta_offset; + + for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num - 1); i++) + writel(GENMASK(31, 0), reg + 4 * i); + + writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num - 1), 0), + reg + 4 * i); +} + +static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask) +{ + void __iomem *reg; + u32 val; + int i; + + reg = ctx->infra_base + ctx->data->vio_mask_offset; + + if (mask) + val = GENMASK(31, 0); + else + val = 0; + + for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num - 1); i++) + writel(val, reg + 4 * i); + + val = readl(reg + 4 * i); + if (mask) + val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num - 1), + 0); + else + val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num - 1), + 0); + + writel(val, reg + 4 * i); +} + +#define PHY_DEVAPC_TIMEOUT 0x10000 + +/* + * devapc_sync_vio_dbg - do "shift" mechansim" to get full violation information. + * shift mechanism is depends on devapc hardware design. + * Mediatek devapc set multiple slaves as a group. + * When violation is triggered, violation info is kept + * inside devapc hardware. + * Driver should do shift mechansim to sync full violation + * info to VIO_DBGs registers. + * + */ +static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx) +{ + void __iomem *pd_vio_shift_sta_reg; + void __iomem *pd_vio_shift_sel_reg; + void __iomem *pd_vio_shift_con_reg; + int min_shift_group; + int ret; + u32 val; + + pd_vio_shift_sta_reg = ctx->infra_base + + ctx->data->vio_shift_sta_offset; + pd_vio_shift_sel_reg = ctx->infra_base + + ctx->data->vio_shift_sel_offset; + pd_vio_shift_con_reg = ctx->infra_base + + ctx->data->vio_shift_con_offset; + + /* Find the minimum shift group which has violation */ + val = readl(pd_vio_shift_sta_reg); + if (!val) + return false; + + min_shift_group = __ffs(val); + + /* Assign the group to sync */ + writel(0x1 << min_shift_group, pd_vio_shift_sel_reg); + + /* Start syncing */ + writel(0x1, pd_vio_shift_con_reg); + + ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0, + PHY_DEVAPC_TIMEOUT); + if (ret) { + dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__); + return false; + } + + /* Stop syncing */ + writel(0x0, pd_vio_shift_con_reg); + writel(0x0, pd_vio_shift_sel_reg); + writel(0x1 << min_shift_group, pd_vio_shift_sta_reg); + + return true; +} + +/* + * devapc_extract_vio_dbg - extract full violation information after doing + * shift mechanism. + */ +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx) +{ + struct mtk_devapc_vio_dbgs *vio_dbgs; + void __iomem *vio_dbg0_reg; + void __iomem *vio_dbg1_reg; + + vio_dbgs = devm_kzalloc(ctx->dev, sizeof(struct mtk_devapc_vio_dbgs), + GFP_KERNEL); + if (!vio_dbgs) + return; + + vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset; + vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset; + + vio_dbgs->vio_dbg0 = readl(vio_dbg0_reg); + vio_dbgs->vio_dbg1 = readl(vio_dbg1_reg); + + /* Print violation information */ + if (vio_dbgs->dbg0_bits.vio_w) + dev_info(ctx->dev, "Write Violation\n"); + else if (vio_dbgs->dbg0_bits.vio_r) + dev_info(ctx->dev, "Read Violation\n"); + + dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n", + vio_dbgs->dbg0_bits.mstid, vio_dbgs->dbg0_bits.dmnid, + vio_dbgs->vio_dbg1); +} + +/* + * devapc_violation_irq - the devapc Interrupt Service Routine (ISR) will dump + * violation information including which master violates + * access slave. + */ +static irqreturn_t devapc_violation_irq(int irq_number, + struct mtk_devapc_context *ctx) +{ + /* + * Mask slave's irq before clearing vio status. + * Must do it to avoid nested interrupt and prevent + * unexpected behavior. + */ + mask_module_irq(ctx, true); + + while (devapc_sync_vio_dbg(ctx)) + devapc_extract_vio_dbg(ctx); + + /* + * Ensure that violation info are written + * before further operations + */ + smp_mb(); + + clear_vio_status(ctx); + mask_module_irq(ctx, false); + + return IRQ_HANDLED; +} + +/* + * start_devapc - unmask slave's irq to start receiving devapc violation. + */ +static void start_devapc(struct mtk_devapc_context *ctx) +{ + void __iomem *pd_apc_con_reg; + + pd_apc_con_reg = ctx->infra_base + ctx->data->apc_con_offset; + writel(BIT(31), pd_apc_con_reg); + + mask_module_irq(ctx, false); +} + +static const struct mtk_devapc_data devapc_mt6779 = { + .vio_idx_num = 511, + .vio_mask_offset = 0x0, + .vio_sta_offset = 0x400, + .vio_dbg0_offset = 0x900, + .vio_dbg1_offset = 0x904, + .apc_con_offset = 0xF00, + .vio_shift_sta_offset = 0xF10, + .vio_shift_sel_offset = 0xF14, + .vio_shift_con_offset = 0xF20, +}; + +static const struct of_device_id mtk_devapc_dt_match[] = { + { + .compatible = "mediatek,mt6779-devapc", + .data = &devapc_mt6779, + }, { + }, +}; + +static int mtk_devapc_probe(struct platform_device *pdev) +{ + struct device_node *node = pdev->dev.of_node; + struct mtk_devapc_context *ctx; + u32 devapc_irq; + int ret; + + if (IS_ERR(node)) + return -ENODEV; + + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + ctx->data = of_device_get_match_data(&pdev->dev); + ctx->dev = &pdev->dev; + + ctx->infra_base = of_iomap(node, 0); + if (!ctx->infra_base) + return -EINVAL; + + devapc_irq = irq_of_parse_and_map(node, 0); + if (!devapc_irq) + return -EINVAL; + + ctx->infra_clk = devm_clk_get(&pdev->dev, "devapc-infra-clock"); + if (IS_ERR(ctx->infra_clk)) + return -EINVAL; + + if (clk_prepare_enable(ctx->infra_clk)) + return -EINVAL; + + ret = devm_request_irq(&pdev->dev, devapc_irq, + (irq_handler_t)devapc_violation_irq, + IRQF_TRIGGER_NONE, "devapc", ctx); + if (ret) { + clk_disable_unprepare(ctx->infra_clk); + return ret; + } + + platform_set_drvdata(pdev, ctx); + + start_devapc(ctx); + + return 0; +} + +static int mtk_devapc_remove(struct platform_device *pdev) +{ + struct mtk_devapc_context *ctx = platform_get_drvdata(pdev); + + if (ctx->infra_clk) + clk_disable_unprepare(ctx->infra_clk); + + return 0; +} + +static struct platform_driver mtk_devapc_driver = { + .probe = mtk_devapc_probe, + .remove = mtk_devapc_remove, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = mtk_devapc_dt_match, + }, +}; + +module_platform_driver(mtk_devapc_driver); + +MODULE_DESCRIPTION("Mediatek Device APC Driver"); +MODULE_AUTHOR("Neal Liu <neal.liu@mediatek.com>"); +MODULE_LICENSE("GPL");
MediaTek bus fabric provides TrustZone security support and data protection to prevent slaves from being accessed by unexpected masters. The security violation is logged and sent to the processor for further analysis or countermeasures. Any occurrence of security violation would raise an interrupt, and it will be handled by mtk-devapc driver. The violation information is printed in order to find the murderer. Signed-off-by: Neal Liu <neal.liu@mediatek.com> --- drivers/soc/mediatek/Kconfig | 9 ++ drivers/soc/mediatek/Makefile | 1 + drivers/soc/mediatek/mtk-devapc.c | 315 +++++++++++++++++++++++++++++++++++++ 3 files changed, 325 insertions(+) create mode 100644 drivers/soc/mediatek/mtk-devapc.c