linux-kernel - Re: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx

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: <aUAzaXp1t+fTVfyT@lizhi-Precision-Tower-5810>
Date: Mon, 15 Dec 2025 11:12:25 -0500
From: Frank Li <Frank.li@....com>
To: Jacky Bai <ping.bai@....com>
Cc: Bjorn Andersson <andersson@...nel.org>,
	Mathieu Poirier <mathieu.poirier@...aro.org>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	linux-remoteproc@...r.kernel.org, imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	peng.fan@....com
Subject: Re: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback
 for i.MX7ULP suspend

On Mon, Dec 15, 2025 at 03:26:05PM +0800, Jacky Bai wrote:
> On i.MX7ULP platform, certain drivers that depend on rpmsg may need
> to send an rpmsg request and receive an acknowledgment from the
> remote core during the late_suspend stage. In the current imx_rproc
> implementation, rx_callback defers message handling to a workqueue.
> However, this deferred work may not execute before the system enters
> the noirq_suspend stage. When that happens, pending mailbox IRQs
> will cause the suspend sequence to abort.
>
> To address this, handle incoming messages synchronously within
> rx_callback when running on i.MX7ULP. This ensures the message is
> processed immediately, allows the mailbox IRQ to be cleared in time,
> and prevents suspend failures.
>
> Signed-off-by: Jacky Bai <ping.bai@....com>
> ---
>  drivers/remoteproc/imx_rproc.c | 49 ++++++++++++++++++++++++++++++++++++++++--
>  drivers/remoteproc/imx_rproc.h |  1 +
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
...
>
> -	queue_work(priv->workqueue, &priv->rproc_work);
> +	if (priv->use_sync_rx)
> +		idr_for_each(&rproc->notifyids, imx_rproc_notified_idr_cb, rproc);
> +	else
> +		queue_work(priv->workqueue, &priv->rproc_work);
>  }
>
>  static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)
> @@ -1009,6 +1015,38 @@ static int imx_rproc_sys_off_handler(struct sys_off_data *data)
>  	return NOTIFY_DONE;
>  }
>
> +static int imx_rproc_pm_notify(struct notifier_block *nb,
> +	unsigned long action, void *data)
> +{
> +	int ret;
> +	struct imx_rproc *priv = container_of(nb, struct imx_rproc, pm_nb);

	struct imx_rproc *priv = container_of(nb, struct imx_rproc, pm_nb);
	int ret;

> +
> +	imx_rproc_free_mbox(priv->rproc);
> +
> +	switch (action) {
> +	case PM_SUSPEND_PREPARE:
> +		ret = imx_rproc_xtr_mbox_init(priv->rproc, false);

Any risk condition if on going message during free() and _init()?

> +		if (ret) {
> +			dev_err(&priv->rproc->dev, "Failed to request non-blocking mbox\n");
> +			return NOTIFY_BAD;
> +		}
> +		priv->use_sync_rx = true;
> +		break;
> +	case PM_POST_SUSPEND:
> +		ret = imx_rproc_xtr_mbox_init(priv->rproc, true);
> +		if (ret) {
> +			dev_err(&priv->rproc->dev, "Failed to request blocking mbox\n");
> +			return NOTIFY_BAD;
> +		}
> +		priv->use_sync_rx = false;
> +		break;
> +	default:
> +		break;

imx_rproc_free_mbox() here? if other notify comming,  mbox is free() as
expected?

Frank
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
>  static void imx_rproc_destroy_workqueue(void *data)
>  {
>  	struct workqueue_struct *workqueue = data;
> @@ -1103,6 +1141,13 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  			return dev_err_probe(dev, ret, "register restart handler failure\n");
>  	}
>
> +	if (dcfg->flags & IMX_RPROC_NEED_PM_SYNC) {
> +		priv->pm_nb.notifier_call = imx_rproc_pm_notify;
> +		ret = register_pm_notifier(&priv->pm_nb);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "register pm notifier failure\n");
> +	}
> +
>  	pm_runtime_enable(dev);
>  	ret = pm_runtime_resume_and_get(dev);
>  	if (ret)
> @@ -1202,7 +1247,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx7ulp = {
>  	.att		= imx_rproc_att_imx7ulp,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7ulp),
> -	.flags		= IMX_RPROC_NEED_SYSTEM_OFF,
> +	.flags		= IMX_RPROC_NEED_SYSTEM_OFF | IMX_RPROC_NEED_PM_SYNC,
>  };
>
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
> index 1b2d9f4d6d19dcdc215be97f7e2ab3488281916a..0e3e460cef4ed9340fb4977183e03143c84764af 100644
> --- a/drivers/remoteproc/imx_rproc.h
> +++ b/drivers/remoteproc/imx_rproc.h
> @@ -18,6 +18,7 @@ struct imx_rproc_att {
>  /* dcfg flags */
>  #define IMX_RPROC_NEED_SYSTEM_OFF	BIT(0)
>  #define IMX_RPROC_NEED_CLKS		BIT(1)
> +#define IMX_RPROC_NEED_PM_SYNC		BIT(2)
>
>  struct imx_rproc_plat_ops {
>  	int (*start)(struct rproc *rproc);
>
> ---
> base-commit: 5ce74bc1b7cb2732b22f9c93082545bc655d6547
> change-id: 20251211-imx7ulp_rproc-57999329239b
>
> Best regards,
> --
> Jacky Bai <ping.bai@....com>
>