LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Neal Liu <neal.liu@mediatek.com>
To: Chun-Kuang Hu <chunkuang.hu@kernel.org>
Cc: Neal Liu <neal.liu@mediatek.com>,
	Rob Herring <robh+dt@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	<devicetree@vger.kernel.org>,
	wsd_upstream <wsd_upstream@mediatek.com>,
	lkml <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@lists.infradead.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v4 2/2] soc: mediatek: add mtk-devapc driver
Date: Tue, 4 Aug 2020 10:18:51 +0800
Message-ID: <1596507531.17917.10.camel@mtkswgap22> (raw)
In-Reply-To: <CAAOTY_9UcnSDTaVPDPyPLsWEYcrcq5MY=z520MWtFdeLw_FqGQ@mail.gmail.com>


On Tue, 2020-08-04 at 00:13 +0800, Chun-Kuang Hu wrote:
> Hi, Neal:
> 
> Neal Liu <neal.liu@mediatek.com> 於 2020年8月3日 週一 上午11:32寫道:
> >
> > Hi Chun-Kuang,
> >
> > On Fri, 2020-07-31 at 23:03 +0800, Chun-Kuang Hu wrote:
> > > Hi, Neal:
> > >
> > > Neal Liu <neal.liu@mediatek.com> 於 2020年7月31日 週五 上午10:44寫道:
> > > >
> > > > Hi Chun-Kuang,
> > > >
> > > >
> > > > On Thu, 2020-07-30 at 00:38 +0800, Chun-Kuang Hu wrote:
> > > > > Hi, Neal:
> > > > >
> > > > > Neal Liu <neal.liu@mediatek.com> 於 2020年7月29日 週三 下午4:29寫道:
> > > > > >
> > > > > > 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_extract_vio_dbg - extract full violation information after doing
> > > > > > + *                          shift mechanism.
> > > > > > + */
> > > > > > +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> > > > > > +{
> > > > > > +       const struct mtk_devapc_vio_dbgs *vio_dbgs;
> > > > > > +       struct mtk_devapc_vio_info *vio_info;
> > > > > > +       void __iomem *vio_dbg0_reg;
> > > > > > +       void __iomem *vio_dbg1_reg;
> > > > > > +       u32 dbg0;
> > > > > > +
> > > > > > +       vio_dbg0_reg = ctx->devapc_pd_base + ctx->offset->vio_dbg0;
> > > > > > +       vio_dbg1_reg = ctx->devapc_pd_base + ctx->offset->vio_dbg1;
> > > > > > +
> > > > > > +       vio_dbgs = ctx->vio_dbgs;
> > > > > > +       vio_info = ctx->vio_info;
> > > > > > +
> > > > > > +       /* Starts to extract violation information */
> > > > > > +       dbg0 = readl(vio_dbg0_reg);
> > > > > > +       vio_info->vio_addr = readl(vio_dbg1_reg);
> > > > > > +
> > > > > > +       vio_info->master_id = (dbg0 & vio_dbgs->mstid.mask) >>
> > > > > > +                             vio_dbgs->mstid.start;
> > > > >
> > > > > What is master_id? How could we use it to debug? For example, if we
> > > > > get a master_id = 1, what should we do for this?
> > > > >
> > > > > > +       vio_info->domain_id = (dbg0 & vio_dbgs->dmnid.mask) >>
> > > > > > +                             vio_dbgs->dmnid.start;
> > > > >
> > > > > What is domain_id? How could we use it to debug? For example, if we
> > > > > get a domain_id = 2, what should we do for this?
> > > > >
> > > >
> > > > master_id and domain_id belongs our bus side-band signal info. It can
> > > > help us to find the violation master.
> > >
> > > Does 'violation master' means the hardware could access the protected
> > > register? (ex. CPU, GCE, ...) If so, I think it's better to add
> > > comment to explain how to map (master_id, domain_id) to a hardware
> > > (maybe the device in device tree) because every body does not know
> > > what the number means. Don't try to translate the number to a string
> > > because this would cost much time to do this. Just print a number and
> > > we could find out the master by the comment.
> >
> > 'violation master' means the master which violates the permission
> > control. For example, if we set permission 'Secure R/W only' as CPU to
> > spi register. When violation is triggered, it means CPU access spi
> > register through normal world instead of secure world, which is not
> > allowed.
> >
> > 'master_id' cannot use the simple comments to describe which master it
> > is. It depends on violation slaves. For example, if there are two
> > violations:
> > 1. CPU access spi reg
> > 2. CPU access timer reg
> > It might be different 'master_id' for CPU on these two cases.
> > I would prefer to remain the id number if translate to a string is a bad
> > idea.
> > Thanks !
> 
> It seams that master_id and domain_id does not help for debug. When we
> get master_id = 1 and domain_id = 2, we don't know what it mean. I
> think we just need violation address because we could find the driver
> that write this address and the bug would be inside this driver. So
> need not to process master_id and domain_id.
> 

Actually, it does help us for debug. violation master is not CPU only.
It might be any other master in our SoC. So the bug might not be inside
the kernel driver.
I'll prefer to remain this information.
Thanks !

> Regards,
> Chun-Kuang.
> 
> >
> > >
> > > >
> > > > > > +       vio_info->write = ((dbg0 & vio_dbgs->vio_w.mask) >>
> > > > > > +                           vio_dbgs->vio_w.start) == 1;
> > > > > > +       vio_info->read = ((dbg0 & vio_dbgs->vio_r.mask) >>
> > > > > > +                         vio_dbgs->vio_r.start) == 1;
> > > > > > +       vio_info->vio_addr_high = (dbg0 & vio_dbgs->addr_h.mask) >>
> > > > > > +                                 vio_dbgs->addr_h.start;
> > > > >
> > > > > What is vio_addr_high? As I know all register address are 32 bits, is
> > > > > vio_addr_high the address above 32 bits?
> > > >
> > > > Yes, you are right. In MT6779, all register base are 32 bits. We can
> > > > ignore this info for current driver. I'll update on next patch.
> > > > Thanks !
> > >
> > > Such a strange hardware, all register is 32 bits but it has a
> > > vio_addr_high in its register. OK, just drop this.
> > >
> > > >
> > > > >
> > > > > > +
> > > > > > +       devapc_vio_info_print(ctx);
> > > > > > +}
> > > > > > +
> > > > >


  reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29  8:18 [PATCH v4] Add MediaTek MT6779 devapc driver Neal Liu
2020-07-29  8:18 ` [PATCH v4 1/2] dt-bindings: devapc: add bindings for mtk-devapc Neal Liu
2020-07-29  8:18 ` [PATCH v4 2/2] soc: mediatek: add mtk-devapc driver Neal Liu
2020-07-29 16:38   ` Chun-Kuang Hu
2020-07-31  2:44     ` Neal Liu
2020-07-31 15:03       ` Chun-Kuang Hu
2020-08-03  3:32         ` Neal Liu
2020-08-03 16:13           ` Chun-Kuang Hu
2020-08-04  2:18             ` Neal Liu [this message]
2020-08-04 15:27               ` Chun-Kuang Hu
2020-07-29 22:47   ` Chun-Kuang Hu
2020-07-31  2:47     ` Neal Liu
2020-07-30 16:14   ` Chun-Kuang Hu
2020-07-31  2:52     ` Neal Liu
2020-07-31 15:55       ` Chun-Kuang Hu
2020-08-03  3:41         ` Neal Liu
2020-08-01  0:12   ` Chun-Kuang Hu
2020-08-03  4:01     ` Neal Liu
2020-08-03 16:04       ` Chun-Kuang Hu
2020-08-04  2:08         ` Neal Liu
2020-08-04 15:55           ` Chun-Kuang Hu
2020-08-01 23:50   ` Chun-Kuang Hu
2020-08-03  4:05     ` Neal Liu
2020-08-03 15:38       ` Chun-Kuang Hu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1596507531.17917.10.camel@mtkswgap22 \
    --to=neal.liu@mediatek.com \
    --cc=chunkuang.hu@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=wsd_upstream@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git