Add support for Zephyr's POSIX architecture

[aescolar]

Zephyr's "POSIX architecture" is a test architecture which allows building the embedded SW with the Zephyr OS and HW models as a Linux executable.

When building for this target the Zephyr build
system will set the PROJECT_PROCESSOR as "posix".

We need to provide appropriate replacements
for the processor HAL for this "processor".
The "metal_cpu_yield()" call, which in other
architectures is handled as doing nothing, and therefore busy waiting, needs replacing with a call out to
Z_SPIN_DELAY(1)

[arnopo]

Hi @aescolar

the libmetal folder is aligned with the code in https://github.com/OpenAMP/libmetal. Therefore, we would like to avoid adding patches on top of itn as it would require rebases.

That said, Any reason to not address this in https://github.com/OpenAMP/libmetal/blob/main/lib/system/zephyr?
and perhaps in https://github.com/OpenAMP/libmetal/edit/main/cmake/options.cmake?

[aescolar]

Any reason to not address this in https://github.com/OpenAMP/libmetal/blob/main/lib/system/zephyr?

I'm certainly open to that. What would you recommend?
To add a lib/system/zephyr/cpu.h and have it conditionally included if("${CMAKE_SYSTEM_PROCESSOR}" STREQUAL "zephyr_native")?
And in either lib/CMakeLists.txt or lib/processor/CMakeLists.txt have the processor directory excluded in that case? (and similarly have lib/cpu.h excluded)

[aescolar]

@arnopo would something like this roughly what you had in mind? (if not please just tell how you'd like it)
If it is, please tell, and I will update the corresponding PR in the openamp/libmetal repo

[aescolar]

@arnopo
Or maybe as a variation: aescolar@d666f16

[arnopo]

I took more time to analyze solutions. there are different options...
First I have some questions:

Do you need some other interface than the metal_cpu_yield ?
Is k_busy_wait could be replaced by k_sleep()
Here is one option that seem simple to implement with current design of the libmetal:
->Create libmetal/lib/processor/posix processor
This folder would contains a cpu.h implementing following functionI take more time to analyse solution. there are different options...
First I have some questions:

• Do you need some other interface than the metal_cpu_yield ?
• Is k_busy_wait could be replaced by k_sleep()

Here is one option that seem simple to implement with current design of the libmetal:
->Create libmetal/lib/processor/posix processor
This folder would contains a cpu.h implementing following function

static inline void metal_cpu_yield(void)
{
	/*
	 *  Native test environment let 1 microsecond pass
	 *  to allow other threads to run
	 */
	metal_sleep_usec(1); /* I assume you can use the sleep method */
};

With such implementation, you have just to create a machine that will rely on Zephyr metal_sleep_usec() system implementation available here:
https://github.com/OpenAMP/libmetal/blob/main/lib/system/zephyr/sleep.h#L25

[aescolar]

@arnopo Thank you very much for looking into it, I really appreciate it :)

Do you need some other interface than the metal_cpu_yield ?

It seems the only trouble are the "processor" headers. And for these, only providing metal_cpu_yield() seems necessary.
processor/<..>/atomic.h would not be strictly necessary.

Is k_busy_wait could be replaced by k_sleep()

It would be possible, though not too nice. Zephyr's k_sleep() has a resolution of the system tick period (10ms by default), so calling metal_sleep_usec(1), will sleep between 10ms and 20ms by default (K_USEC() ceils the input value to a multiple of the system tick period).
So if we use this call, we would have a quite different behavior when building for this target.

Due to that, it would be nicer to call either zephyr's Z_SPIN_DELAY(1) or k_busy_wait(1).

If you want, we can have this special processor metal_cpu_yield() call into a __metal_cpu_yield(), and require the system header to define it if they support this target.

Create libmetal/lib/processor/posix processor

If you think that is the best way to go, I can certainly do that. Though if you feel like it would be better to not have a weird name in the processor folder like "posix", but would prefer something a bit more descriptive like "hosted" I can also do that.
Note that setting the processor to "posix" is something the zephyr build system does, and is a quite arbitrary choice. So we can have that be renamed to whatever you'd want before calling into the libmetal cmake.

If you want we can have a chat in discord (aescolar) or Teams.

I updated this PR with something you may like.

[arnopo]

I made some remarks here but please push on libmetal github for next reviews

[aescolar]

Discussion moved to OpenAMP/libmetal#260

[aescolar]

As libmetal is now frozen until the end of the month, it would be very nice to make an exception and allow this patch into the zephyr module. Without this fix, it is not possible to build libmetal, and therefore openAMP for the native targets, including the simulated nrf5340.
And without that, we cannot test in CI all configurations which depend on openAMP, which is in turn all configuration which use the split BT and 15.4 stacks.
Once this patch is in, it will be followed right away by the corresponding Zephyr patch to enable this functionality (zephyrproject-rtos/zephyr#63803)
Note that after this we will be running in CI (not just building) quite a lot of configurations using openAMP which should provide nice runtime coverage in Zephyr's CI both for libmetal and OpenAMP (apart from all the code which now we will be able to test, which is this split configuration of the BT and 802.15.4 stacks, and a number of sample and test applications).
This change is quite isolated, so I hope it will be acceptable to take it in ahead of its upstream.

[arnopo]

I'm Ok with that, @carlocaione any concerns?
On my side I will push to have more reviews on OpenAMP to be able to merge it ASAP in the lib

[carlocaione]

No particular concern

[aescolar]

@arnopo @carlocaione should this be merged?

[carlocaione]

@aescolar with the openamp PR we rushed a bit but this time I'd like to do things properly. Can you open a zephyr PR with a manifest change pointing to this PR? After the Zephyr PR is greed, I will merge this.

[aescolar]

@carlocaione This is the PR that tests with this module update: zephyrproject-rtos/zephyr#63803
CI is green :)

[carlocaione]

@carlocaione This is the PR that tests with this module update: zephyrproject-rtos/zephyr#63803 CI is green :)

oooh ok, I missed that.

[aescolar]

@carlocaione Thanks! :)