| 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
| ||
|
Message-ID: <8c00c09f-da9e-4630-a3d7-586ed29116f3@nxp.com>
Date: Mon, 15 Dec 2025 20:32:27 +0200
From: Iuliana Prodan <iuliana.prodan@....com>
To: Jacky Bai <ping.bai@....com>, 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>
Cc: 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 12/15/2025 9:26 AM, 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(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 3be8790c14a2ccc789dd40508ec274000ec09978..1127c56388fd94f296c80db5c01a5d431d669c4d 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -22,6 +22,7 @@
> #include <linux/reboot.h>
> #include <linux/regmap.h>
> #include <linux/remoteproc.h>
> +#include <linux/suspend.h>
> #include <linux/workqueue.h>
>
> #include "imx_rproc.h"
> @@ -111,11 +112,13 @@ struct imx_rproc {
> void __iomem *rsc_table;
> struct imx_sc_ipc *ipc_handle;
> struct notifier_block rproc_nb;
> + struct notifier_block pm_nb;
> u32 rproc_pt; /* partition id */
> u32 rsrc_id; /* resource id */
> u32 entry; /* cpu start address */
> u32 core_index;
> struct dev_pm_domain_list *pd_list;
> + bool use_sync_rx;
> };
>
> static const struct imx_rproc_att imx_rproc_att_imx93[] = {
> @@ -725,7 +728,10 @@ static void imx_rproc_rx_callback(struct mbox_client *cl, void *msg)
> struct rproc *rproc = dev_get_drvdata(cl->dev);
> struct imx_rproc *priv = rproc->priv;
>
> - 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);
> +
> + imx_rproc_free_mbox(priv->rproc);
Here you free mailbox unconditinally.
> +
> + switch (action) {
> + case PM_SUSPEND_PREPARE:
When switching to sync mode, any pending work in the workqueue should be
flushed to avoid processing messages twice.
> + ret = imx_rproc_xtr_mbox_init(priv->rproc, false);
> + 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;
> + }
If imx_rproc_xtr_mbox_init() fails in either case, the mailbox channels
remain freed but the function returns NOTIFY_BAD. The system may
continue with freed mailbox channels.
Only free the mailbox if re-initialization succeeds, or ensure proper
cleanup on failure.
> + priv->use_sync_rx = false;
> + break;
> + default:
> + break;
> + }
> +
> + 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");
> + }
> +
The PM notifier is registered in probe but never unregistered in remove
or error paths.
Thanks,
Iulia
> 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,
Powered by blists - more mailing lists