| 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: <20250708172953.rbsvk6gy4xuxo42l@hiagonb>
Date: Tue, 8 Jul 2025 14:29:53 -0300
From: Hiago De Franco <hiagofranco@...il.com>
To: Mathieu Poirier <mathieu.poirier@...aro.org>
Cc: Bjorn Andersson <andersson@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Iuliana Prodan <iuliana.prodan@....com>,
Peng Fan <peng.fan@....com>, Daniel Baluta <daniel.baluta@....com>,
linux-remoteproc@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Hiago De Franco <hiago.franco@...adex.com>,
Ritesh Kumar <ritesh.kumar@...adex.com>
Subject: Re: [PATCH] remoteproc: imx_rproc: merge ITCM and DTCM regions
Hi Peng, Mathieu,
On Mon, Jul 07, 2025 at 10:13:02AM -0600, Mathieu Poirier wrote:
> On Fri, Jul 04, 2025 at 04:08:16PM -0300, Hiago De Franco wrote:
> > Hi Mathieu,
> >
> > On Fri, Jul 04, 2025 at 10:25:19AM -0600, Mathieu Poirier wrote:
> > > Good morning,
> > >
> > > On Thu, Jul 03, 2025 at 10:08:31AM -0300, Hiago De Franco wrote:
> > > > From: Hiago De Franco <hiago.franco@...adex.com>
> > > >
> > > > Merge the contiguous ITCM and DTCM regions into a single region to
> > > > prevent failures when loading ELF files with large sections:
> > > >
> > > > remoteproc remoteproc0: powering up imx-rproc
> > > > remoteproc remoteproc0: Booting fw image rproc-imx-rproc-fw, size 151824
> > > > imx-rproc imx8mp-cm7: Translation failed: da = 0x1f48 len = 0x1fcb0
> > > > remoteproc remoteproc0: bad phdr da 0x1f48 mem 0x1fcb0
> > > > remoteproc remoteproc0: Failed to load program segments: -22
> > > > remoteproc remoteproc0: Boot failed: -22
> > > >
> > > > This approach is the same as commit 8749919defb8 ("remoteproc:
> > > > imx_rproc: Merge TCML/U").
> > > >
> > > > Suggested-by: Ritesh Kumar <ritesh.kumar@...adex.com>
> > > > Signed-off-by: Hiago De Franco <hiago.franco@...adex.com>
> > > > ---
> > > > Hi,
> > > >
> > > > The ELF I tested had the following data section:
> > > >
> > > > Memory region Used Size Region Size %age Used
> > > > m_interrupts: 680 B 1 KB 66.41%
> > > > m_text: 6984 B 127 KB 5.37%
> > > > m_data: 130224 B 128 KB 99.35%
> > > > m_data2: 0 GB 16 MB 0.00%
> > > > [100%] Built target hello_world_cm7.elf
> > > >
> > > > Which triggered the error. After this patch, remoteproc was able to boot
> > > > and work fine. Thanks!
> > > > ---
> > > > drivers/remoteproc/imx_rproc.c | 6 ++----
> > > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> > > > index 74299af1d7f1..bbf089ef48ee 100644
> > > > --- a/drivers/remoteproc/imx_rproc.c
> > > > +++ b/drivers/remoteproc/imx_rproc.c
> > > > @@ -166,8 +166,8 @@ static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = {
> > > >
> > > > static const struct imx_rproc_att imx_rproc_att_imx8mn[] = {
> > > > /* dev addr , sys addr , size , flags */
> > > > - /* ITCM */
> > > > - { 0x00000000, 0x007E0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> > > > + /* D/ITCM */
> > > > + { 0x00000000, 0x007E0000, 0x00040000, ATT_OWN | ATT_IOMEM },
> > > > /* OCRAM_S */
> > > > { 0x00180000, 0x00180000, 0x00009000, 0 },
> > > > /* OCRAM */
> > > > @@ -180,8 +180,6 @@ static const struct imx_rproc_att imx_rproc_att_imx8mn[] = {
> > > > { 0x08000000, 0x08000000, 0x08000000, 0 },
> > > > /* DDR (Code) - alias */
> > > > { 0x10000000, 0x40000000, 0x0FFE0000, 0 },
> > > > - /* DTCM */
> > > > - { 0x20000000, 0x00800000, 0x00020000, ATT_OWN | ATT_IOMEM },
> > >
> > > In commit 8749919defb8 "dev addr" and "sys addr" were both contiguous, but in
> > > this patch "dev addr" is not. How will this work with new kernel that use old
> > > FW images? Am I missing something?
> >
> > No, you are correct, I think the use case I tested was not good enough.
> >
> > If I understand correctly, this will break older firmware expecting
> > .data at 0x20000000 because dev_addr is no longer mapped for DTCM entry.
> >
>
> Correct. Older firmware would still expect DTCM at 0x20000000.
>
>
> > Do you think it is possible (or reccomend) another approach to fix this
> > issue? In this case to keep using the TCM, instead of going to OCRAM or
> > DDR.
> >
>
> To me the best way to proceed is understand why using the current mapping is a
> problem. The changelog describes a failure condition when dealing with large
> sections but does not indicate _why_ that is happening. I think that's what
> needs to be fixed rather than trying to move mappings around.
Thanks for the information you both provided, sorry for the noise, so
please Mathieu ignore this patch. I will work around this in a different
way.
By the way, Peng, I noticed the DDR linker from MCUXpresso does not work
if the firmware is larger than 128KB, since the .data is placed right
after .text and loaded later to DDR. The imx_rproc driver should instead
have a way to do the other way around: starting from the firwmare inside
DDR, we could set PC and stack from M7 to point to DDR and start the
execution. Probably will be slower, but it would make the DDR case
possible.
Correct me if I am wrong, but as my current understanding the DDR linker
is broken without this change to the driver. Anyway, maybe something for
a future patch.
Thanks,
Hiago.
>
> > Thanks,
> > Hiago.
> >
> > >
> > > Thanks,
> > > Mathieu
> > >
> > > > /* OCRAM_S - alias */
> > > > { 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
> > > > /* OCRAM */
> > > > --
> > > > 2.39.5
> > > >
Powered by blists - more mailing lists