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:
 <DB9PR04MB846152D0289467468CE0077588B32@DB9PR04MB8461.eurprd04.prod.outlook.com>
Date: Fri, 2 Aug 2024 04:59:45 +0000
From: Peng Fan <peng.fan@....com>
To: Mathieu Poirier <mathieu.poirier@...aro.org>
CC: "Peng Fan (OSS)" <peng.fan@....nxp.com>, Bjorn Andersson
	<andersson@...nel.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer
	<s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, Daniel Baluta <daniel.baluta@....com>,
	Iuliana Prodan <iuliana.prodan@....com>, Marek Vasut <marex@...x.de>,
	"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>
Subject: RE: [PATCH v2 2/2] remoteproc: imx_rproc: handle system off for
 i.MX7ULP

> Subject: Re: [PATCH v2 2/2] remoteproc: imx_rproc: handle system off
> for i.MX7ULP
> 
> On Tue, Jul 30, 2024 at 08:06:22AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH v2 2/2] remoteproc: imx_rproc: handle system
> off
> > > for i.MX7ULP
> > >
> > > On Fri, Jul 19, 2024 at 04:49:04PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@....com>
> > > >
> > > > The i.MX7ULP Cortex-A7 is under control of Cortex-M4. The
> > > i.MX7ULP
> > > > Linux poweroff and restart rely on rpmsg driver to send a message
> > > > to
> > > > Cortex-M4 firmware. Then Cortex-A7 could poweroff or restart by
> > > > Cortex-M4 to configure the i.MX7ULP power controller properly.
> > > >
> > > > However the reboot and restart kernel common code use atomic
> > > notifier,
> > > > so with blocking tx mailbox will trigger kernel dump, because of
> > > > blocking mailbox will use wait_for_completion_timeout. In such
> > > > case, linux no need to wait for completion.
> > > >
> > > > Current patch is to use non-blocking tx mailbox channel when
> > > > system
> > > is
> > > > going to poweroff or restart.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@....com>
> > > > ---
> > > >  drivers/remoteproc/imx_rproc.c | 36
> > > > ++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 36 insertions(+)
> > > >
> > > > diff --git a/drivers/remoteproc/imx_rproc.c
> > > > b/drivers/remoteproc/imx_rproc.c index
> > > 01cf1dfb2e87..e1abf110abc9
> > > > 100644
> > > > --- a/drivers/remoteproc/imx_rproc.c
> > > > +++ b/drivers/remoteproc/imx_rproc.c
> > > > @@ -18,6 +18,7 @@
> > > >  #include <linux/of_reserved_mem.h>  #include
> > > > <linux/platform_device.h>  #include <linux/pm_domain.h>
> > > > +#include <linux/reboot.h>
> > > >  #include <linux/regmap.h>
> > > >  #include <linux/remoteproc.h>
> > > >  #include <linux/workqueue.h>
> > > > @@ -114,6 +115,7 @@ struct imx_rproc {
> > > >  	u32				entry;		/* cpu start
> > > address */
> > > >  	u32				core_index;
> > > >  	struct dev_pm_domain_list	*pd_list;
> > > > +	struct sys_off_data		data;
> > >
> > > What is this for?  I don't see it used in this patch.
> >
> > Oh, it was added when I was developing this feature, but in the end
> > this seems not needed.
> >
> > >
> > > >  };
> > > >
> > > >  static const struct imx_rproc_att imx_rproc_att_imx93[] = { @@
> > > > -1050,6 +1052,22 @@ static int imx_rproc_clk_enable(struct
> > > imx_rproc *priv)
> > > >  	return 0;
> > > >  }
> > > >
> > > > +static int imx_rproc_sys_off_handler(struct sys_off_data *data) {
> > > > +	struct rproc *rproc = data->cb_data;
> > > > +	int ret;
> > > > +
> > > > +	imx_rproc_free_mbox(rproc);
> > > > +
> > > > +	ret = imx_rproc_xtr_mbox_init(rproc, false);
> > > > +	if (ret) {
> > > > +		dev_err(&rproc->dev, "Failed to request non-blocking
> > > mbox\n");
> > > > +		return NOTIFY_BAD;
> > > > +	}
> > > > +
> > > > +	return NOTIFY_DONE;
> > > > +}
> > > > +
> > > >  static int imx_rproc_probe(struct platform_device *pdev)  {
> > > >  	struct device *dev = &pdev->dev; @@ -1104,6 +1122,24 @@
> static
> > > > int imx_rproc_probe(struct
> > > platform_device *pdev)
> > > >  	if (rproc->state != RPROC_DETACHED)
> > > >  		rproc->auto_boot = of_property_read_bool(np,
> > > "fsl,auto-boot");
> > > >
> > > > +	if (of_device_is_compatible(dev->of_node, "fsl,imx7ulp-cm4"))
> > > {
> > > > +		ret = devm_register_sys_off_handler(dev,
> > > SYS_OFF_MODE_POWER_OFF_PREPARE,
> > > > +
> > > SYS_OFF_PRIO_DEFAULT,
> > > > +
> > > imx_rproc_sys_off_handler, rproc);
> > >
> > > Why does the mailbox needs to be set up again when the system is
> > > going down...
> >
> > As wrote in commit message:
> > "i.MX7ULP Linux poweroff and restart rely on rpmsg driver to send a
> > message," so need to set up mailbox in non-blocking way to send a
> > message to M4 side.
> >
> > >
> > > > +		if (ret) {
> > > > +			dev_err(dev, "register power off handler
> > > failure\n");
> > > > +			goto err_put_clk;
> > > > +		}
> > > > +
> > > > +		ret = devm_register_sys_off_handler(dev,
> > > SYS_OFF_MODE_RESTART_PREPARE,
> > > > +
> > > SYS_OFF_PRIO_DEFAULT,
> > > > +
> > > imx_rproc_sys_off_handler, rproc);
> > >
> > > ... and why does it need to be free'd when the system is going up?
> >
> >
> > Sorry, I not get your point. The free is in imx_rproc_sys_off_handler.
> > During system booting, the mailbox is not freed.
> 
> Why is the same operation done at both startup and shutdown - that is
> not clear.

The below commit shows request/free done in startup and shutdown.
Hope this explains what you ask.

commit 99b142cf7191b08adcd23f700ea0a3d7dffdd0c1
Author: Peng Fan <peng.fan@....com>
Date:   Fri Oct 21 12:15:25 2022 +0800

    remoteproc: imx_rproc: Request mbox channel later
    
    It is possible that when remote processor crash, the communication
    channel will be broken with garbage value in mailbox, such as
    when Linux is issuing a message through mailbox, remote processor
    crashes, we need free & rebuild the mailbox channels to make sure
    no garbage value in mailbox channels.
    
    So move the request/free to start/stop for managing remote procesosr in
    Linux, move to attach/detach for remote processor is out of control of
    Linux.
    
    Previous, we just request mbox when attach for CM4 boot early before
    Linux, but if mbox defer probe, remoteproc core will do resource cleanup
    and corrupt resource table for later probe.
    
    So move request mbox ealier and still keep mbox request when attach
    for self recovery case, but keep a check when request/free mbox.

> 
> I am currently away from the office, returning on August 12th.  As such
> I will not be following up on this thread until then.

sure. Thanks for letting me know.

Thanks,
Peng.

> 
> >
> > Thanks,
> > Peng.
> >
> > >
> > > > +		if (ret) {
> > > > +			dev_err(dev, "register restart handler
> > > failure\n");
> > > > +			goto err_put_clk;
> > > > +		}
> > > > +	}
> > > > +
> > > >  	ret = rproc_add(rproc);
> > > >  	if (ret) {
> > > >  		dev_err(dev, "rproc_add failed\n");
> > > >
> > > > --
> > > > 2.37.1
> > > >
> > > >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ