[v6,2/2] soc: mediatek: add mt6779 devapc driver
diff mbox series

Message ID 1597289564-17030-3-git-send-email-neal.liu@mediatek.com
State New
Headers show
Series
  • [v6,1/2] dt-bindings: devapc: add bindings for mtk-devapc
Related show

Commit Message

Neal Liu Aug. 13, 2020, 3:32 a.m. UTC
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 |  320 +++++++++++++++++++++++++++++++++++++
 3 files changed, 330 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-devapc.c

Comments

Chun-Kuang Hu Aug. 15, 2020, 3:03 a.m. UTC | #1
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
Neal Liu Aug. 17, 2020, 4:02 a.m. UTC | #2
Hi Chun-Kuang,

On Sat, 2020-08-15 at 11:03 +0800, Chun-Kuang Hu wrote:
> 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?

If there is another violation happened before previous violation is
fully handled, nested interrupt would happen.

Let's me take an example:
vio A happen
enter A ISR
...		vio B happen
finish A ISR	enter B ISR
		...
		finish B ISR

We mask all module's irq to avoid nested interrupt.

> 
> > +
> > +       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.

Does it mean that remove function would be called only if probe function
is returned successfully?
Is there any chance this function would be called directly?

> 
> 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
Chun-Kuang Hu Aug. 17, 2020, 3:13 p.m. UTC | #3
Hi, Neal:

Neal Liu <neal.liu@mediatek.com> 於 2020年8月17日 週一 下午12:02寫道:
>
> Hi Chun-Kuang,
>
> On Sat, 2020-08-15 at 11:03 +0800, Chun-Kuang Hu wrote:
> > 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?
>
> If there is another violation happened before previous violation is
> fully handled, nested interrupt would happen.
>
> Let's me take an example:
> vio A happen
> enter A ISR
> ...             vio B happen
> finish A ISR    enter B ISR
>                 ...
>                 finish B ISR
>
> We mask all module's irq to avoid nested interrupt.

This is not 'nested' interrupt. After A ISR is finished, B ISR happen.
So A ISR and B ISR are consecutive interrupt, not nested interrupt.
To compare mask irq and no mask irq, Let's consider this situation:

1. 1000 consecutive violation happen, the time period between two
violation is 0.01 ms, so the total time is 10ms. (In 10ms, 1000
violation happen)
2. One ISR handle time is 1 ms, so in one ISR handler, 100 violation happen.

For mask irq solution, 10 ISR handler is trigger. For no mask irq
solution, 11 ISR handler is trigger.
I think these two solution have similar result, and no mask irq
solution print more information (If these 1000 violation is trigger by
20 different driver, no mask solution may show one more driver than
mask solution)
So I think it's not necessary to mask irq in irq handler.

>
> >
> > > +
> > > +       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.
>
> Does it mean that remove function would be called only if probe function
> is returned successfully?

Yes.

> Is there any chance this function would be called directly?

No.

Regards,
Chun-Kuang.

>
> >
> > 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
>
Neal Liu Aug. 18, 2020, 2:44 a.m. UTC | #4
Hi Chun-Kuang,

On Mon, 2020-08-17 at 23:13 +0800, Chun-Kuang Hu wrote:
> Hi, Neal:
> 
> Neal Liu <neal.liu@mediatek.com> 於 2020年8月17日 週一 下午12:02寫道:
> >
> > Hi Chun-Kuang,
> >
> > On Sat, 2020-08-15 at 11:03 +0800, Chun-Kuang Hu wrote:
> > > 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?
> >
> > If there is another violation happened before previous violation is
> > fully handled, nested interrupt would happen.
> >
> > Let's me take an example:
> > vio A happen
> > enter A ISR
> > ...             vio B happen
> > finish A ISR    enter B ISR
> >                 ...
> >                 finish B ISR
> >
> > We mask all module's irq to avoid nested interrupt.
> 
> This is not 'nested' interrupt. After A ISR is finished, B ISR happen.
> So A ISR and B ISR are consecutive interrupt, not nested interrupt.
> To compare mask irq and no mask irq, Let's consider this situation:
> 
> 1. 1000 consecutive violation happen, the time period between two
> violation is 0.01 ms, so the total time is 10ms. (In 10ms, 1000
> violation happen)
> 2. One ISR handle time is 1 ms, so in one ISR handler, 100 violation happen.
> 
> For mask irq solution, 10 ISR handler is trigger. For no mask irq
> solution, 11 ISR handler is trigger.
> I think these two solution have similar result, and no mask irq
> solution print more information (If these 1000 violation is trigger by
> 20 different driver, no mask solution may show one more driver than
> mask solution)
> So I think it's not necessary to mask irq in irq handler.
> 

No, my example is B ISR is entered before A ISR finished.
Why this is not nested?
vio A happen
enter A ISR
...             vio B happen
...		enter B ISR
finish A ISR
                ...
		...
                finish B ISR

> >
> > >
> > > > +
> > > > +       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.
> >
> > Does it mean that remove function would be called only if probe function
> > is returned successfully?
> 
> Yes.
> 
> > Is there any chance this function would be called directly?
> 
> No.
> 
> Regards,
> Chun-Kuang.
> 
> >
> > >
> > > 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
> >
Neal Liu Aug. 18, 2020, 6:20 a.m. UTC | #5
Hi Chun-Kuang,

On Tue, 2020-08-18 at 10:44 +0800, Neal Liu wrote:
> Hi Chun-Kuang,
> 
> On Mon, 2020-08-17 at 23:13 +0800, Chun-Kuang Hu wrote:
> > Hi, Neal:
> > 
> > Neal Liu <neal.liu@mediatek.com> 於 2020年8月17日 週一 下午12:02寫道:
> > >
> > > Hi Chun-Kuang,
> > >
> > > On Sat, 2020-08-15 at 11:03 +0800, Chun-Kuang Hu wrote:
> > > > 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?
> > >
> > > If there is another violation happened before previous violation is
> > > fully handled, nested interrupt would happen.
> > >
> > > Let's me take an example:
> > > vio A happen
> > > enter A ISR
> > > ...             vio B happen
> > > finish A ISR    enter B ISR
> > >                 ...
> > >                 finish B ISR
> > >
> > > We mask all module's irq to avoid nested interrupt.
> > 
> > This is not 'nested' interrupt. After A ISR is finished, B ISR happen.
> > So A ISR and B ISR are consecutive interrupt, not nested interrupt.
> > To compare mask irq and no mask irq, Let's consider this situation:
> > 
> > 1. 1000 consecutive violation happen, the time period between two
> > violation is 0.01 ms, so the total time is 10ms. (In 10ms, 1000
> > violation happen)
> > 2. One ISR handle time is 1 ms, so in one ISR handler, 100 violation happen.
> > 
> > For mask irq solution, 10 ISR handler is trigger. For no mask irq
> > solution, 11 ISR handler is trigger.
> > I think these two solution have similar result, and no mask irq
> > solution print more information (If these 1000 violation is trigger by
> > 20 different driver, no mask solution may show one more driver than
> > mask solution)
> > So I think it's not necessary to mask irq in irq handler.
> > 
> 
> No, my example is B ISR is entered before A ISR finished.
> Why this is not nested?
> vio A happen
> enter A ISR
> ...             vio B happen
> ...		enter B ISR
> finish A ISR
>                 ...
> 		...
>                 finish B ISR
> 

I have some misunderstanding about how ARM CPU & GIC works. I'll confirm
it and get back to you. Please ignore previous mail thread.
Thanks !

> > >
> > > >
> > > > > +
> > > > > +       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.
> > >
> > > Does it mean that remove function would be called only if probe function
> > > is returned successfully?
> > 
> > Yes.
> > 
> > > Is there any chance this function would be called directly?
> > 
> > No.
> > 
> > Regards,
> > Chun-Kuang.
> > 
> > >
> > > >
> > > > 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
> > >
> 
>
Neal Liu Aug. 26, 2020, 9:59 a.m. UTC | #6
Hi Chun-Kuang,

On Tue, 2020-08-18 at 14:20 +0800, Neal Liu wrote:
> Hi Chun-Kuang,
> 
> On Tue, 2020-08-18 at 10:44 +0800, Neal Liu wrote:
> > Hi Chun-Kuang,
> > 
> > On Mon, 2020-08-17 at 23:13 +0800, Chun-Kuang Hu wrote:
> > > Hi, Neal:
> > > 
> > > Neal Liu <neal.liu@mediatek.com> 於 2020年8月17日 週一 下午12:02寫道:
> > > >
> > > > Hi Chun-Kuang,
> > > >
> > > > On Sat, 2020-08-15 at 11:03 +0800, Chun-Kuang Hu wrote:
> > > > > 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?
> > > >
> > > > If there is another violation happened before previous violation is
> > > > fully handled, nested interrupt would happen.
> > > >
> > > > Let's me take an example:
> > > > vio A happen
> > > > enter A ISR
> > > > ...             vio B happen
> > > > finish A ISR    enter B ISR
> > > >                 ...
> > > >                 finish B ISR
> > > >
> > > > We mask all module's irq to avoid nested interrupt.
> > > 
> > > This is not 'nested' interrupt. After A ISR is finished, B ISR happen.
> > > So A ISR and B ISR are consecutive interrupt, not nested interrupt.
> > > To compare mask irq and no mask irq, Let's consider this situation:
> > > 
> > > 1. 1000 consecutive violation happen, the time period between two
> > > violation is 0.01 ms, so the total time is 10ms. (In 10ms, 1000
> > > violation happen)
> > > 2. One ISR handle time is 1 ms, so in one ISR handler, 100 violation happen.
> > > 
> > > For mask irq solution, 10 ISR handler is trigger. For no mask irq
> > > solution, 11 ISR handler is trigger.
> > > I think these two solution have similar result, and no mask irq
> > > solution print more information (If these 1000 violation is trigger by
> > > 20 different driver, no mask solution may show one more driver than
> > > mask solution)
> > > So I think it's not necessary to mask irq in irq handler.
> > > 
> > 
> > No, my example is B ISR is entered before A ISR finished.
> > Why this is not nested?
> > vio A happen
> > enter A ISR
> > ...             vio B happen
> > ...		enter B ISR
> > finish A ISR
> >                 ...
> > 		...
> >                 finish B ISR
> > 
> 
> I have some misunderstanding about how ARM CPU & GIC works. I'll confirm
> it and get back to you. Please ignore previous mail thread.
> Thanks !

Yes, you are right. There is only 1 CPU (CPU0) will process irq handler
in ARM CPU. Nested interrupt will never happen.
The reason why I have misunderstanding is that Mediatek has some
customization about irq handling for ARM CPUs. But it would not applied
in upstream kernel.

Let remove mask/unmask module irq during ISR. The diff would be:

diff --git a/drivers/soc/mediatek/mtk-devapc.c
b/drivers/soc/mediatek/mtk-devapc.c
index 5189b3f4d63f..0ba61d742e0e 100644
--- a/drivers/soc/mediatek/mtk-devapc.c
+++ b/drivers/soc/mediatek/mtk-devapc.c
@@ -183,24 +183,10 @@ static void devapc_extract_vio_dbg(struct
mtk_devapc_context *ctx)
 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;
 }

Is that okay for you?
Thanks !

> 
> > > >
> > > > >
> > > > > > +
> > > > > > +       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.
> > > >
> > > > Does it mean that remove function would be called only if probe function
> > > > is returned successfully?
> > > 
> > > Yes.
> > > 
> > > > Is there any chance this function would be called directly?
> > > 
> > > No.
> > > 
> > > Regards,
> > > Chun-Kuang.
> > > 
> > > >
> > > > >
> > > > > 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
> > > >
> > 
> > 
> 
>
Chun-Kuang Hu Aug. 26, 2020, 9:59 p.m. UTC | #7
Hi, Neal:

Neal Liu <neal.liu@mediatek.com> 於 2020年8月26日 週三 下午5:59寫道:
>
> Hi Chun-Kuang,
>
> On Tue, 2020-08-18 at 14:20 +0800, Neal Liu wrote:
> > Hi Chun-Kuang,
> >
> > On Tue, 2020-08-18 at 10:44 +0800, Neal Liu wrote:
> > > Hi Chun-Kuang,
> > >
> > > On Mon, 2020-08-17 at 23:13 +0800, Chun-Kuang Hu wrote:
> > > > Hi, Neal:
> > > >
> > > > Neal Liu <neal.liu@mediatek.com> 於 2020年8月17日 週一 下午12:02寫道:
> > > > >
> > > > > Hi Chun-Kuang,
> > > > >
> > > > > On Sat, 2020-08-15 at 11:03 +0800, Chun-Kuang Hu wrote:
> > > > > > 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?
> > > > >
> > > > > If there is another violation happened before previous violation is
> > > > > fully handled, nested interrupt would happen.
> > > > >
> > > > > Let's me take an example:
> > > > > vio A happen
> > > > > enter A ISR
> > > > > ...             vio B happen
> > > > > finish A ISR    enter B ISR
> > > > >                 ...
> > > > >                 finish B ISR
> > > > >
> > > > > We mask all module's irq to avoid nested interrupt.
> > > >
> > > > This is not 'nested' interrupt. After A ISR is finished, B ISR happen.
> > > > So A ISR and B ISR are consecutive interrupt, not nested interrupt.
> > > > To compare mask irq and no mask irq, Let's consider this situation:
> > > >
> > > > 1. 1000 consecutive violation happen, the time period between two
> > > > violation is 0.01 ms, so the total time is 10ms. (In 10ms, 1000
> > > > violation happen)
> > > > 2. One ISR handle time is 1 ms, so in one ISR handler, 100 violation happen.
> > > >
> > > > For mask irq solution, 10 ISR handler is trigger. For no mask irq
> > > > solution, 11 ISR handler is trigger.
> > > > I think these two solution have similar result, and no mask irq
> > > > solution print more information (If these 1000 violation is trigger by
> > > > 20 different driver, no mask solution may show one more driver than
> > > > mask solution)
> > > > So I think it's not necessary to mask irq in irq handler.
> > > >
> > >
> > > No, my example is B ISR is entered before A ISR finished.
> > > Why this is not nested?
> > > vio A happen
> > > enter A ISR
> > > ...             vio B happen
> > > ...         enter B ISR
> > > finish A ISR
> > >                 ...
> > >             ...
> > >                 finish B ISR
> > >
> >
> > I have some misunderstanding about how ARM CPU & GIC works. I'll confirm
> > it and get back to you. Please ignore previous mail thread.
> > Thanks !
>
> Yes, you are right. There is only 1 CPU (CPU0) will process irq handler
> in ARM CPU. Nested interrupt will never happen.
> The reason why I have misunderstanding is that Mediatek has some
> customization about irq handling for ARM CPUs. But it would not applied
> in upstream kernel.
>
> Let remove mask/unmask module irq during ISR. The diff would be:
>
> diff --git a/drivers/soc/mediatek/mtk-devapc.c
> b/drivers/soc/mediatek/mtk-devapc.c
> index 5189b3f4d63f..0ba61d742e0e 100644
> --- a/drivers/soc/mediatek/mtk-devapc.c
> +++ b/drivers/soc/mediatek/mtk-devapc.c
> @@ -183,24 +183,10 @@ static void devapc_extract_vio_dbg(struct
> mtk_devapc_context *ctx)
>  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;
>  }
>
> Is that okay for you?

Looks good to me.

Regards,
Chun-Kuang.

> Thanks !
>
> >
> > > > >
> > > > > >
> > > > > > > +
> > > > > > > +       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.
> > > > >
> > > > > Does it mean that remove function would be called only if probe function
> > > > > is returned successfully?
> > > >
> > > > Yes.
> > > >
> > > > > Is there any chance this function would be called directly?
> > > >
> > > > No.
> > > >
> > > > Regards,
> > > > Chun-Kuang.
> > > >
> > > > >
> > > > > >
> > > > > > 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
> > > > >
> > >
> > >
> >
> >
>

Patch
diff mbox series

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..5189b3f
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-devapc.c
@@ -0,0 +1,320 @@ 
+// 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);
+
+	/* Write clear */
+	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_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)
+{
+	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
+
+	mask_module_irq(ctx, false);
+}
+
+/*
+ * stop_devapc - mask slave's irq to stop service.
+ */
+static void stop_devapc(struct mtk_devapc_context *ctx)
+{
+	mask_module_irq(ctx, true);
+
+	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
+}
+
+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);
+
+	stop_devapc(ctx);
+
+	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");