| 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:
<VE1PR04MB72132DBDCD7422B546A2BD3B87AAA@VE1PR04MB7213.eurprd04.prod.outlook.com>
Date: Tue, 16 Dec 2025 02:29:03 +0000
From: Jacky Bai <ping.bai@....com>
To: Iuliana Prodan <iuliana.prodan@....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" <linux-remoteproc@...r.kernel.org>,
"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Peng Fan <peng.fan@....com>
Subject: RE: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback
for i.MX7ULP suspend
HI Luliana,
> 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..1127c56388fd94f296c80db
> 5c01a
> > 5d431d669c4d 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.
Please ignore this patch, changes is not perfect and too complex. Will try to
Fix the issue from mailbox.
Thx for your comments.
BR
Jacky Bai
> > +
> > + 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..0e3e460cef4ed9340fb4977
> 183e0
> > 3143c84764af 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