Re: [PATCH v6 2/2] soc: mediatek: add mt6779 devapc driver

[Chun-Kuang Hu <chunkuang.hu@kernel.org>]

Hi, Neal:

Neal Liu <neal.liu@mediatek.com> 於 2020年8月13日 週四 上午11:33寫道：
>
> 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]

> +/*
> + * 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);

I still don't understand why nested interrupt happen. If two CPU
process different devapc interrupt at the same time, mask interrupt
could not prevent these two CPU to sync vio dbg at the same time. As I
know, in ARM CPU, only CPU0 process irq handler, and all devapc
interrupt has the same priority, so why nested interrupt happen? Could
you explain more detail about how nested interrupt happen?

> +
> +       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;
> +}
> +

[snip]

> +
> +static int mtk_devapc_remove(struct platform_device *pdev)
> +{
> +       struct mtk_devapc_context *ctx = platform_get_drvdata(pdev);
> +
> +       stop_devapc(ctx);
> +
> +       if (ctx->infra_clk)

This always true.

Regards,
Chun-Kuang.

> +               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");
> --
> 1.7.9.5
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek