linux-kernel - RE: [PATCH V2 6/6] remoteproc: imx_rproc: set address of .interrupts section as bootaddr

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]

Date:   Fri, 3 Feb 2023 00:12:15 +0000
From:   Peng Fan <peng.fan@....com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>,
        "Peng Fan (OSS)" <peng.fan@....nxp.com>
CC:     "andersson@...nel.org" <andersson@...nel.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "arnaud.pouliquen@...s.st.com" <arnaud.pouliquen@...s.st.com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        "linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Iuliana Prodan <iuliana.prodan@....com>,
        Daniel Baluta <daniel.baluta@....com>
Subject: RE: [PATCH V2 6/6] remoteproc: imx_rproc: set address of .interrupts
 section as bootaddr

> Subject: Re: [PATCH V2 6/6] remoteproc: imx_rproc: set address
> of .interrupts section as bootaddr
> 
> On Fri, Jan 27, 2023 at 05:22:46PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@....com>
> >
> > i.MX93 M33 has ROM, it needs the ".interrupts" section address to
> > start
> > M33 firmware. In current design, the Arm Trusted Firmware(ATF) use
> > TCML start address when the 2nd arg is 0 when SMC call. So When the
> > M33 firmware is built with TCML address, it works well.
> >
> > However when M33 firmware is built to run in DDR, we need pass the
> > ".interrupts" address as 2nd arg to ATF to start M33 firmwrae.
> >
> > Signed-off-by: Peng Fan <peng.fan@....com>
> > ---
> >  drivers/remoteproc/imx_rproc.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c
> > b/drivers/remoteproc/imx_rproc.c index f5ee0c9bb09d..59cca5ac3045
> > 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -374,7 +374,8 @@ static int imx_rproc_start(struct rproc *rproc)
> >  					 dcfg->src_start);
> >  		break;
> >  	case IMX_RPROC_SMC:
> > -		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0,
> 0, 0, 0, 0, 0, &res);
> > +		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START,
> rproc->bootaddr,
> > +			      0, 0, 0, 0, 0, &res);
> >  		ret = res.a0;
> >  		break;
> >  	case IMX_RPROC_SCU_API:
> > @@ -664,6 +665,13 @@ static u64 imx_rproc_get_boot_addr(struct rproc
> *rproc, const struct firmware *f
> >  		 */
> >  		writel(*(u32 *)(elf_data + offset), va);
> >  		writel(*(u32 *)(elf_data + offset + 4), va + 4);
> > +	} else if (priv->dcfg->devtype == IMX_RPROC_IMX93) {
> > +		/* i.MX93 Cortex-M33 has ROM, it only needs the section
> address */
> > +		shdr = rproc_elf_find_shdr(rproc, fw, ".interrupts");
> > +		if (!shdr)
> > +			return bootaddr;
> 
> This contradicts what you wrote in the cover letter of the patchset about an
> ".interrupts" section always being present.

Yes, from the initial beginning for supporting Cortex-M firmware, the section
is there. I just think whether people build their own firmware, not has
this section, and just put firmware in TCM, there is no need to explicitly set
word 0 and 4 again. This maybe a fake assumption, I could refine this piece
code.

> 
> There is enough in this patchset to make me look for a second opinion.  As
> such I am CC'ing Iuliana and Daniel.  Please respin this, adding both of them
> to the recipient list.  I will do another revision only when they have provided
> an RB tag.

They not work on other areas, not same as me. Anyway I could ask them to
help review. 

Thanks,
Peng.

> 
> Thanks,
> Mathieu
> 
> > +
> > +		return elf_shdr_get_sh_addr(class, shdr);
> >  	}
> >
> >  	return bootaddr;
> > --
> > 2.37.1
> >