| 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: <20250709082652.GA15057@nxa18884-linux>
Date: Wed, 9 Jul 2025 16:26:52 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: Hiago De Franco <hiagofranco@...il.com>
Cc: Mathieu Poirier <mathieu.poirier@...aro.org>,
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
On Tue, Jul 08, 2025 at 02:29:53PM -0300, Hiago De Franco wrote:
>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.
I am not aware of the size limitation if image is built to run in DDR.
It maybe MCUXpresso team just reuse the linker script for TCM and only
update the link address.
You could update the linker script to build larger image.
In final elf, .data is put just after .text, but the related section
loading address should be specified as my understanding. See below,
.data is at 0x20000000 for M7.
xx-gcc -S imx8mn_m7_TCM_rpmsg_lite_str_echo_rtos.elf
There are 20 section headers, starting at offset 0xc998:
Section Headers:
[Nr] Name Type Addr Off Size ES Flg Lk Inf Al
[ 0] NULL 00000000 000000 000000 00 0 0 0
[ 1] .interrupts PROGBITS 00000000 001000 000240 00 A 0 0 4
[ 2] .resource_table PROGBITS 00000240 001240 000058 00 A 0 0 1
[ 3] .text PROGBITS 000002a0 0012a0 0046b0 00 AX 0 0 16
[ 4] .ARM ARM_EXIDX 00004950 005950 000008 00 AL 3 0 4
[ 5] .init_array INIT_ARRAY 00004958 005958 000004 04 WA 0 0 4
[ 6] .fini_array FINI_ARRAY 0000495c 00595c 000004 04 WA 0 0 4
[ 7] .data PROGBITS 20000000 006000 00000c 00 WA 0 0 4
[ 8] .ncache.init PROGBITS 80000000 00600c 000000 00 W 0 0 1
[ 9] .ncache PROGBITS 80000000 00600c 000000 00 W 0 0 1
[10] .bss NOBITS 2000000c 00600c 00a4ac 00 WA 0 0 4
[11] .heap NOBITS 2000a4b8 00600c 000400 00 WA 0 0 1
[12] .stack NOBITS 2000a8b8 00600c 000400 00 WA 0 0 1
[13] .ARM.attributes ARM_ATTRIBUTES 00000000 00600c 000030 00 0 0 1
[14] .debug_line_str PROGBITS 00000000 00603c 0002b1 01 MS 0 0 1
[15] .comment PROGBITS 00000000 0062ed 000055 01 MS 0 0 1
[16] .debug_frame PROGBITS 00000000 006344 000260 00 0 0 4
[17] .symtab SYMTAB 00000000 0065a4 003cd0 10 18 586 4
[18] .strtab STRTAB 00000000 00a274 002664 00 0 0 1
[19] .shstrtab STRTAB 00000000 00c8d8 0000bd 00 0 0 1
>
>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.
If you wanna image in DDR, you could specifiy the address of data section
in your linker script.
But for support ddr elf file, you need patches as below:
https://lore.kernel.org/linux-arm-kernel/CAEnQRZC5t=qmo+OJLW+dqZg4gH9cAN=paWDSGbrJb2AvkKBqxg@mail.gmail.com/T/#ec54c42b70416b002936a643b44b79661dd2a8483
This patchset was rejected, because we need to get stack/pc from .interrupts
section and store to ITCM.
Latest NXP m7 demo has included a new section for stack/pc, but
this will only be public in 2025 Q3 release, for pre-2025-Q3 releases,
still need the upper patchset.
Regards,
Peng
>
>Thanks,
>Hiago.
>
>>
>> > Thanks,
>> > Hiago.
>> >
>> > >
>> > > Thanks,
>> > > Mathieu
>> > >
>> > > > /* OCRAM_S - alias */
>> > > > { 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
>> > > > /* OCRAM */
>> > > > --
>> > > > 2.39.5
>> > > >
Powered by blists - more mailing lists