linux-kernel - RE: [PATCH] mailbox: imx: fix TXDB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <AM6PR04MB594156E8588648AEA72698C488DF2@AM6PR04MB5941.eurprd04.prod.outlook.com>
Date: Fri, 5 Jul 2024 09:34:43 +0000
From: Peng Fan <peng.fan@....com>
To: Peng Fan <peng.fan@....com>, "Peng Fan (OSS)" <peng.fan@....nxp.com>,
	Jassi Brar <jassisinghbrar@...il.com>, Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team
	<kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>
CC: Ranjani Vaidyanathan <ranjani.vaidyanathan@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH] mailbox: imx: fix TXDB_V2 channel race condition

Hi Jassi,

> Subject: RE: [PATCH] mailbox: imx: fix TXDB_V2 channel race condition
> 
> Hi Jassi,
> 
> > Subject: [PATCH] mailbox: imx: fix TXDB_V2 channel race condition
> 
> Ping..

Ping again..

Thanks,
Peng.

> 
> Thanks,
> Peng.
> 
> >
> > From: Peng Fan <peng.fan@....com>
> >
> > Two TXDB_V2 channels are used between Linux and System
> Manager(SM).
> > Channel0 for normal TX, Channel 1 for notification completion.
> > The TXDB_V2 trigger logic is using imx_mu_xcr_rmw which uses
> > read/modify/update logic.
> >
> > Note: clear MUB GSR BITs, the MUA side GCR BITs will also got
> cleared
> > per hardware design.
> > Channel0 Linux
> > read GCR->modify GCR->write GCR->M33 SM->read GSR----->clear
> GSR
> >                                                 |-(1)-|
> > Channel1 Linux start in time slot(1)
> > read GCR->modify GCR->write GCR->M33 SM->read GSR->clear GSR
> So
> > Channel1 read GCR will read back the GCR that Channel0 wrote,
> because
> > M33 has not finish clear GSR, this means Channel1 GCR writing will
> > trigger Channel1 and Channel0 interrupt both which is wrong.
> >
> > Channel0 will be freed(SCMI channel status set to FREE) in M33 SM
> when
> > processing the 1st Channel0 interrupt. So when 2nd interrupt trigger
> > (channel 0/1 trigger together), SM will see a freed Channel0, and
> > report protocol error.
> >
> > To address the issue, not using read/modify/update logic, just use
> > write, because write 0 to GCR will be ignored. And after write MUA
> > GCR, wait the SM to clear MUB GSR by looping MUA GCR value.
> >
> > Fixes: 5bfe4067d350 ("mailbox: imx: support channel type tx
> doorbell
> > v2")
> > Reviewed-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@....com>
> > Signed-off-by: Peng Fan <peng.fan@....com>
> > ---
> >
> > V1: This patch has got R-b inside NXP and could be directly applied to
> >     upstream linux, so I keep the R-b tag from Ranjani.
> >
> >  drivers/mailbox/imx-mailbox.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-
> > mailbox.c index 5c1d09cad761..38abe07babdf 100644
> > --- a/drivers/mailbox/imx-mailbox.c
> > +++ b/drivers/mailbox/imx-mailbox.c
> > @@ -224,6 +224,8 @@ static int imx_mu_generic_tx(struct
> imx_mu_priv
> > *priv,
> >  			     void *data)
> >  {
> >  	u32 *arg = data;
> > +	u32 val;
> > +	int ret;
> >
> >  	switch (cp->type) {
> >  	case IMX_MU_TYPE_TX:
> > @@ -235,7 +237,13 @@ static int imx_mu_generic_tx(struct
> imx_mu_priv
> > *priv,
> >  		tasklet_schedule(&cp->txdb_tasklet);
> >  		break;
> >  	case IMX_MU_TYPE_TXDB_V2:
> > -		imx_mu_xcr_rmw(priv, IMX_MU_GCR,
> > IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx), 0);
> > +		imx_mu_write(priv, IMX_MU_xCR_GIRn(priv->dcfg-
> > >type, cp->idx),
> > +			     priv->dcfg->xCR[IMX_MU_GCR]);
> > +		ret = readl_poll_timeout(priv->base + priv->dcfg-
> > >xCR[IMX_MU_GCR], val,
> > +					 !(val &
> > IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx)),
> > +					 0, 1000);
> > +		if (ret)
> > +			dev_warn_ratelimited(priv->dev, "channel
> > type: %d failure\n",
> > +cp->type);
> >  		break;
> >  	default:
> >  		dev_warn_ratelimited(priv->dev, "Send data on wrong
> channel type:
> > %d\n", cp->type);
> > --
> > 2.37.1
>