ipc: icmsg & icbmsg: Add support for the nrf5340bsim

[aescolar]

Add support in these IPC backends for the nrf5340bsim.
And enable these backends samples/tests for this board so they are also runtime tested in CI.

Fixes #78099

The driver of this PR is that these IPC backends are configured from Device Tree at build time, with whatever hardcoded address is given on it.
But in the "native" (POSIX arch) targets (when we built for workstation testing) we cannot write to some hardcoded address. All addresses are determined at link time.
In this PR, we ifdef calls during the backend init to replace the hardcoded address with something which can be used in workstation testing. (And together with it, we need to conditionally remove the const in the structures which hold those pointers we modify at runtime).
The change does not have any effect for embedded targets.

This PR supersedes #78109. Unlike that PR, this replaces the pointers during init. As trying to replace them in the macros was not really possible in a generic way.

The PR contains:

• 1 commit with a minor fix for a type that needs to be exposed always in icmsg_me.c a906eb7
• A minor improvement in the pbuf interface, so icbmsg does not need to know the details on how to initialize the Rx pbuf by hand: 5ec172b
• An update to the nRF HW models version
• A commit, that for native (POSIX arch based) targets, adds a function to remap a hardcoded real device memory address into a corresponding address in the host simulated memory space.
• The main change itself d6c8a5f , in which, during init we replace the hard pointer addresses which otherwise would come from DT, with whatever address is known to be good to be used for this simulated targets. (Just by calling the function from the previous commit). Note this change is conditional only for the POSIX arch targets.
• And 3 commits enabling these IPC sample/test (multi_endpoint, icmsg, static_vrings) in the nrf5340bsim simulated target, so we have them runtime tested in CI also.

[zephyrbot]

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
nrf_hw_models	zephyrproject-rtos/nrf_hw_models@d2a119a	zephyrproject-rtos/nrf_hw_models@85944c6	zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

[aescolar]

CC @dchat-nordic @ilhanates

[pdunaj]

@maje-emb , please have a look.

[pdunaj]

I am against doing it the way proposed. This is quite intrusive into ipc service with next to no value in itself. We will now need to track if the new posix related macros don't break compatibility and security of the ipc.

@aescolar, isn't it enough to remap buffers defined on dts level (i.e., entire memory passed to ipc service)? I expect you already tried this but I wonder why every pointer needs to be mapped separately.

blocking until we understand the impact

[pdunaj]

If I understand correctly the problem comes from the fact that addresses passed in dts are from HW world and do not exist in virtual space mapping. If this is a problem then fixing address on configuration of buffers should be enough as all addresses used in ipc are calculated from it (offset from start).

I wonder if you could not simply mmap some memory into this specific range for the process running the simulation. If it is not used it should work. You could try to mmap a (ram based) file there. This could be done at this fake sim board initialization time and so nothing in the system would be aware that any mapping actually took place.

[pdunaj]

something like that

#include <sys/mman.h>
#include <stdio.h>
#include <stdint.h>
#include <errno.h>

int main(int argc, char *argv[])
{
        void *inaddr = (void*)(intptr_t)0x20000000;
        void *addr = mmap(inaddr, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

        printf("errno: %d, inaddr: %p, addr: %p\n", errno, inaddr, addr);

        return 0;
}
pdunaj@mystuff:~/work/tests/mmap$ gcc mmapme.c && ./a.out
errno: 0, inaddr: 0x20000000, addr: 0x20000000

[aescolar]

If I understand correctly the problem comes from the fact that addresses passed in dts are from HW world

Yes, the problem is that addresses are indeed the HW addresses which come from DT.

I wonder if you could not simply mmap some memory into this specific range for the process..

A proof of concept of this can be seen here #78295. But unfortunately this direction is not tenable. It became immediately evident that ASan breaks with this approach. And it is likely that other libraries or tooling may also, if not now, some time in the future.

Another option which was tried was to change (at preprocessor time) the addresses when building for these simulated targets. So instead of using the DT provided address, we'd refer to a constant pointer known at link time. This is how it is done today for static vrings. Unfortunately given the amount of macrology around the initialization of icbmsg, this proved much worse than one could hope. Specially as the Tx and Rx buffers are crossed in each side of the channel. A quick trial can be seen here: #78109. Generalizing this to N MCUs with the current design would lead to way too many macros too difficult to maintain. (If instead of defining 2 buffers in DT, we'd define just one buffer, and a direction (so the buffer is used as two halves by the code at init), it would be easier).

I also considered offsetting the DT address (also at preprocessor time), offsetting 0x2000_0000 to a buffer whose address is known at link time, where that buffer represent the whole global memory. That solution would be ok for the nrf5340, but would easily became quite cumbersome if we have different processors using different common memories.

That's why I leaned for a solution which changes the configured addresses at runtime for simulated targets. Note there is no penalty for the embedded targets. Neither in executed code or memory layout (what is const remains const).

isn't it enough to remap buffers defined on dts level (i.e., entire memory passed to ipc service)? I expect you already tried this but I wonder why every pointer needs to be mapped separately.

We are just remapping the pointers set from DT macros. The macros that initialize the config structure are already pre-initializing all those pointers. Unfortunately there is not just 1 pointer set from DT, there is all of those to which the DT address propagates thru several layers of macros.

[doki-nordic]

Maybe make this function more consistent, because right now calling it depends on few conditions. It is automatically called in init function, but if you not calling init then you have to call it directly. If CONFIG_ARCH_POSIX is enabled, you have to call it. Otherwise, you don't.

I suggest calling this function always before using pb. Even before pbuf_init. if CONFIG_ARCH_POSIX it can be empty static inline function. This will make it clearer when to use it and we avoid additional #ifs when calling it.

[doki-nordic]

Also, please update the comment to be doxygen-compatible and add statement when it must be called.

[aescolar]

Thank you very much for the review. I updated the PR content with what we just discussed on the phone.

[pdunaj]

Considering the situation I see we will have to accept this changes. I think it would be best to find a long term solution hiding the simulator quirks into some porting layer. I understand however this cannot be done at this point and we need to go on.
Since Dominik is ok, I don't see a reason to nack it.

[arnopo]

a minor comment else LGTM

[arnopo]

if you don't treat the error returned would be nice to have a comment explaining why