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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <AM6PR04MB5941581D46DA309123EE528888CE2@AM6PR04MB5941.eurprd04.prod.outlook.com>
Date: Tue, 18 Jun 2024 15:13:04 +0000
From: Peng Fan <peng.fan@....com>
To: "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: [PATCH] mailbox: imx: fix TXDB_V2 channel race condition

Ping..

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