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: <CAEnQRZDKUHfWMXC5oe+kSpLpR5manxx63cxDOMrFEqi6a4VxPg@mail.gmail.com>
Date: Thu, 22 May 2025 09:09:57 +0300
From: Daniel Baluta <daniel.baluta@...il.com>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>
Cc: jassisinghbrar@...il.com, shawnguo@...nel.org, s.hauer@...gutronix.de, 
	kernel@...gutronix.de, festevam@...il.com, linux-kernel@...r.kernel.org, 
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org, 
	mailbox@...ts.linux.dev, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH] mailbox: imx: Fix TXDB_V2 sending

On Fri, Apr 25, 2025 at 4:51 AM Peng Fan (OSS) <peng.fan@....nxp.com> wrote:
>
> From: Peng Fan <peng.fan@....com>
>
> i.MX95 features several processing domains, Cortex-M7, Cortex-A55
> secure, Cortex-A55 non-secure. Each domain could communicate with
> SCMI firmware with a dedicated MU. But the current NXP SCMI firmware
> is not a RTOS, all processing logic codes are in interrupt context.
> So if high priority Cortex-M7 is communicating with SCMI firmware and
> requires a bit more time to handle the SCMI call, Linux MU TXDB_V2
> will be timeout with high possiblity in 1000us(the current value in
> imx-mailbox.c). Per NXP SCMI firmware design, if timeout, there is
> no recover logic, so SCMI agents should never timeout and always
> wait until the check condition met.
>
> Based on the upper reason, enlarge the timeout value to 10ms which
> is less chance to timeout, and retry if timeout really happends.
>
> Fixes: 5bfe4067d350 ("mailbox: imx: support channel type tx doorbell v2")
> Signed-off-by: Peng Fan <peng.fan@....com>
> ---
>  drivers/mailbox/imx-mailbox.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 6ef8338add0d..aef8d572a27c 100644
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c
> @@ -226,7 +226,7 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
>  {
>         u32 *arg = data;
>         u32 val;
> -       int ret;
> +       int ret, count;
>
>         switch (cp->type) {
>         case IMX_MU_TYPE_TX:
> @@ -240,11 +240,20 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
>         case IMX_MU_TYPE_TXDB_V2:
>                 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);
> +               ret = -ETIMEDOUT;
> +               count = 0;
> +               while (ret) {
> +                       ret =
> +                       readl_poll_timeout(priv->base + priv->dcfg->xCR[IMX_MU_GCR], val,
> +                                          !(val & IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx)),
> +                                          0, 10000);
> +
> +                       if (ret) {
> +                               dev_warn_ratelimited(priv->dev,
> +                                                    "channel type: %d timeout, %d times, retry\n",
> +                                                    cp->type, ++count);
> +                       }
> +               }

This could result in a infinite loop. I would try only a fixed number
of times then bail. Please use count to break the loop
after let say 10 tries.

>                 break;
>         default:
>                 dev_warn_ratelimited(priv->dev, "Send data on wrong channel type: %d\n", cp->type);
> --
> 2.37.1
>
>