Add support to relocate open-amp resource table

[emfend]

There are some remote processing scenarios where the resource table needs
to be moved to another location. This can be either because there is no
way to exchange the location of the resource table between the two
processors and they have to agree on a fixed predefined location, or
because the memory segment where the current resource table resides is not
is accessible/writable by the remote processor.
A real example of such a case can be found in the Linux rproc driver for
i.MX SoCs. A memory area containing the resource table can be specified
there in the device tree. Since this feature is application dependent, it
can optionally be configured in the device tree along with the other
open-amp related nodes.

This device tree snippet is intended to illustrate a possible use case:

chosen {
    zephyr,ipc_shm = &ipc_shm;
    zephyr,rsc_table = &rsc_table;
    zephyr,ipc = &mailbox0;
};

reserved-memory {
    #address-cells = <1>;
    #size-cells = <1>;
    ranges;

    ipc_shm: memory@55000000 {
        reg = <0x55000000 0x500000>;
    };

    rsc_table: rsc-table@550ff000 {
        reg = <0x550ff000 0x1000>;
    };
};

[carlocaione]

uhm, are the addresses reflecting a real use-case scenario with the RSC table in the middle of the SHM?

[emfend]

uhm, are the addresses reflecting a real use-case scenario with the RSC table in the middle of the SHM?

To get an easily reproducible example, I tried to align with the NXP Linux reference demo for the i.MX8MP.
The SHM layout used can be found here:
https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8mp-evk-rpmsg.dts?h=lf-5.15.y#n27
So the resource table is between the vrings and the buffer area.

[carlocaione]

To get an easily reproducible example, I tried to align with the NXP Linux reference demo for the i.MX8MP. The SHM layout used can be found here: https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8mp-evk-rpmsg.dts?h=lf-5.15.y#n27 So the resource table is between the vrings and the buffer area.

Right. In the zephyr case where is the SRAM mapped? I'm asking this because in your example you are basically writing directly in memory using pointers but there is nothing that guarantees you that you have exclusive access to that memory right? Or the SRAM is mapped somewhere else?

[emfend]

To get an easily reproducible example, I tried to align with the NXP Linux reference demo for the i.MX8MP. The SHM layout used can be found here: https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8mp-evk-rpmsg.dts?h=lf-5.15.y#n27 So the resource table is between the vrings and the buffer area.

Right. In the zephyr case where is the SRAM mapped? I'm asking this because in your example you are basically writing directly in memory using pointers but there is nothing that guarantees you that you have exclusive access to that memory right? Or the SRAM is mapped somewhere else?

Good question. In the example setup the resource table is moved to the DDR region:
https://elixir.bootlin.com/zephyr/latest/source/soc/arm/nxp_imx/mimx8ml8_m7/mpu_regions.c#L58

I'm not sure if I understand you correctly, but since both processors need to read/write from the resource table area, it's not possible to grant exclusive access to this area.

As far as I can see in the openamp_rsc_table example, there seem to be no mapping.
The virtual and physical addresses of the libmetal regions are simply both initialized with the physical address.
This starts here:
https://github.com/zephyrproject-rtos/zephyr/blob/main/samples/subsys/ipc/openamp_rsc_table/src/main_remote.c#L170

It also looks likes that Zephyr libmetal port does not implement mapping at all.

TLDR:
No, there is no special mapping for the new resource table region.

[carlocaione]

My concern is slightly different. Let's say that you have only one SRAM area for Zephyr, so Zephyr is allowed to make use of this area as it pleases. If your regions for SHM and the resource table are directly pointing somewhere in the middle of this region, then it is possible that Zephyr will silently corrupt these regions writing on it.

What you usually want to do is to define several regions: one for the SRAM area for zephyr, and then separate memory regions for SHM and resource table that are well far apart.

Now, I'm not familiar with the memory map of this SoC so I was just wondering how did you choose the addresses and whether those are safe enough.

[emfend]

Okay, let's see if I can get closer to your point.
In this example, there is a section in DDR that is reserved for Zephyr. Since the entire DDR is shared between the two processors, this section is explicitly separated into the Linux device tree.
See the definition in Zephyr:
https://github.com/zephyrproject-rtos/zephyr/blob/main/soc/arm/nxp_imx/mimx8ml8_m7/mpu_regions.c#L62
And the related Linux device tree node:
https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8mp-evk-rpmsg.dts?h=lf-5.15.y#n22

So Zepyhr uses the DDR area:
0x80000000 (0x1000000)
And for SHM this one:
0x55000000 (0x500000)

In this case, these areas do not overlap.

But since in these multiprocessor scenarios memory usage is quite flexible and many things have to fit together (Zepyhr DT, Linux DT, linker script), it's easy to configure a system where nasty things happen...

[carlocaione]

Yup, that solves my issues. Thank you for putting up with me ;)

[hubertmis]

It looks to me that without this patch, resource table location can be configured in a linker script. With this patch we have two ways to configure resource table location in memory:

1. In linker script, what might be silently overridden by
2. Device tree entry

I'm not sure if silent overriding linker script configuration by a device tree node is fine. Also I would like to understand why do we have two mechanisms with similar functionality, and if both of them are required.

[hubertmis]

Is zephyr the correct vendor to use here? Resource table is more OpenAMP or Linux -defined than Zephyr

[emfend]

Since the two names 'zephyr,ipc' and 'zephyr,ipc_shm' are already used for open-amp, I thought it is consistent to use them for the resource table as well.
But if I'm wrong and there's a better suggestion, I'll be happy to use it.

[carlocaione]

I'm not sure if silent overriding linker script configuration by a device tree node is fine.

For the record, this is exactly what we do with zephyr,memory-region compatible.

[emfend]

If it's possible to place the resource table in a RAM segment at the address specified in the device tree by just controlling the linker, I think that would be fine and the explicit copying isn't necessary.
This needs to work for static and generated linker scripts, and it would also be nice if the SoC specific linker scripts didn't have to be changed.

Is such a system already being used for something comparable? Would that be the preferred option?

[carlocaione]

If it's possible to place the resource table in a RAM segment at the address specified in the device tree by just controlling the linker, I think that would be fine and the explicit copying isn't necessary. This needs to work for static and generated linker scripts, and it would also be nice if the SoC specific linker scripts didn't have to be changed.

Is such a system already being used for something comparable? Would that be the preferred option?

again, this is exactly what is done by using a node compatible with zephyr,memory-region compatible, it works for static and dynamic linker scripts and it will soon have support to automatically configure the MPU, see #43119

[emfend]

Yes, in theory I got your point. It was more about a request for practical help.
While I was able to define new sections with memory-region and place variables at the specified location, I was not able to change the .resource_table section [1] to a specific location in RAM (that get initialized as well).
My simple attempt at this looked like this:

rsctable: rsctable@550ff000 {
  compatible = "zephyr,memory-region";
  reg = <0x550ff000 0x1000>;
  zephyr,memory-region = ".resource_table";
};

[1] https://github.com/zephyrproject-rtos/zephyr/blob/main/soc/arm/nxp_imx/mimx8ml8_m7/linker.ld#L25

Should this work? Did I just do something wrong?

[carlocaione]

Should this work? Did I just do something wrong?

Try with compatible = "zephyr,memory-region", "mmio-sram";

Maybe cherry-pick #44412 as well. Didn't that work for you?

[emfend]

Hmm, after cherry-picking 'devicetree_regions: Fix fallback on token' and updating the DT node as suggested, the table is still not the desired location.

rsctable: rsctable@550ff000 {
  compatible = "zephyr,memory-region", "mmio-sram";
  reg = <0x550ff000 0x1000>;
  zephyr,memory-region = ".resource_table";
};

The output of readelf -s zephyr_openamp_rsc_table.elf | grep resource without the DT node:

 141: 00000000     0 FILE    LOCAL  DEFAULT  ABS resource_table.c
 144: 00006090    88 OBJECT  LOCAL  DEFAULT   26 resource_table

And here the ouput with the DT node present:

 141: 00000000     0 FILE    LOCAL  DEFAULT  ABS resource_table.c
 144: 00006090    88 OBJECT  LOCAL  DEFAULT   26 resource_table
 921: 550ff000     0 NOTYPE  GLOBAL DEFAULT  ABS ___resource_table_load_st
1054: 550ff000     0 NOTYPE  GLOBAL DEFAULT   25 ___resource_table_start
1178: 00000000     0 NOTYPE  GLOBAL DEFAULT  ABS ___resource_table_size
1201: 550ff000     0 NOTYPE  GLOBAL DEFAULT   25 ___resource_table_end

So it looks like the entry is somehow honored, but not quite as intended.

If I replace '.resource_table' entirely with e.g. 'xresource_table' I'm at least able to control the location with a ROMABLE_REGION (as required by the linker script).
When I try to use the originally desired position at 0x550ff000, the linker complains that this address is not in the "FLASH" region.
So there seem to be two problems: the leading dot in the linker section name, and the need to somehow override the "GROUP_LINK_IN(ROMABLE_REGION)" directive.

[carlocaione]

 141: 00000000     0 FILE    LOCAL  DEFAULT  ABS resource_table.c
 144: 00006090    88 OBJECT  LOCAL  DEFAULT   26 resource_table
 921: 550ff000     0 NOTYPE  GLOBAL DEFAULT  ABS ___resource_table_load_st
1054: 550ff000     0 NOTYPE  GLOBAL DEFAULT   25 ___resource_table_start
1178: 00000000     0 NOTYPE  GLOBAL DEFAULT  ABS ___resource_table_size
1201: 550ff000     0 NOTYPE  GLOBAL DEFAULT   25 ___resource_table_end

This looks correct to me. That says that you have a section at the correct address but you are simply failing to put the resource_table there. How are you placing the resource table?

Please consider that (as stated in the documentation) the . is converted to _, so you should put the data at the section _resource_table or simply remove the leading ..

If I replace '.resource_table' entirely with e.g. 'xresource_table' I'm at least able to control the location with a ROMABLE_REGION (as required by the linker script). When I try to use the originally desired position at 0x550ff000, the linker complains that this address is not in the "FLASH" region. So there seem to be two problems: the leading dot in the linker section name, and the need to somehow override the "GROUP_LINK_IN(ROMABLE_REGION)" directive.

I'm not fully following you here. Can you check the test sample in #44412? That should give you a pretty good idea on how this is supposed to work.

[emfend]

Unfortunately, it is not possible to change the name of the resource table section. When resource table relocation is not used and a remote processor is started by Linux, Linux searches the elf for a section called ".resource_table".
For example, if '_resource_table' is used as the section name instead, Linux would no longer find the resource table.

[carlocaione]

Unfortunately, it is not possible to change the name of the resource table section. When resource table relocation is not used and a remote processor is started by Linux, Linux searches the elf for a section called ".resource_table".
For example, if '_resource_table' is used as the section name instead, Linux would no longer find the resource table.

Well, this is really unfortunate. That would have been better discussed when we decided to sanitized the section names :(

[emfend]

Well, looks like I'm running late :/

Assuming the section name behavior is set, what is the desired way to continue here?
Should I keep the previous (copy) implementation, or are there other ideas?

[emfend]

As it turns out, simply copying the table may not be appropriate for all use cases. At least for the IMX platform, which I took a closer look at.
Although this copy is made this way in the MCUXpresso SDK sample to demonstrate its use with Linux, it does not work properly when the firmware is loaded from the user space.
Since in such a case Linux will automatically install the resource table in the new location and immediately start updating the fields of the table after starting the remote processor, this can lead to conflicts with the copy made in the firmware.

See also this ML thread.

So since it's unclear how and if this will be fixed in Linux, I'm currently using the updated commit from this RFC (which I just pushed).
Now I first check whether the resource table has already been installed by someone else and in such a case I skip the copy process.
Since some fields of the table may have already been modified by the remote host, it is not possible to compare the full table.

Now that there's a bit more logic, I'm not sure if hiding this magic in rsc_table_get() is still a good idea.
Perhaps it would be better to create a more explicit function for this, even if that would mean that it would also require changes in the examples.

What do you think?

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[Fladdan]

Hi @carlocaione @emfend,

I'm using the CM7 of the IMX8MP and tried to run the openamp_rsc_table sample.

I needed this patch to get the RPMSG communication functional. As well in my custom firmware.
Could you please reopen this PR @carlocaione ?