OpenAMP support.

[iabdalkader]

This PR adds support for OpenAMP in MicroPython, with the goal of standardizing asymmetric multi-core support across different hardware running MicroPython. There are 3 major components to OpenAMP, libmetal, remoteproc and RPMsg. libmetal provides abstraction of the low-level underlying hardware, remoteproc is used for processor Life Cycle Management (LCM) (i.e loading firmware, starting, stopping a core etc..), and RPMsg is a bus infrastructure that enables Inter Processor Communications (IPC) between different cores. These components are implemented in this PR as follows:

• OpenAMP's libmetal submodule is added in this PR (see note about this submodule), and a generic MicroPython system port is implemented, one that uses some sane defaults and common mp_hal functions, and also allows for MicroPython's ports to override defaults and config libmetal.

• OpenAMP's remoteproc component is implemented in extmod/modremoteproc.c module which provides an API to load firmware and control remote processors. Port-specific ops must be implemented for this module (similar to mpbthciport.c or mpnetworkport.c). One example is already done in this PR for stm32.

• OpenAMP's RPMsg component and the rest of the logic is implemented in extmod/modopenamp.c. This module provides anendpoint type (a logical connection on top of RPMsg channel) that can be used to communicate with the remote core.

Example: The following script loads a firmware image that implements a virtual uart over RPMsg channel, built for STM32H7's M4 core, starts it, and communicates with the M4 once the core is ready to communicate. The remote core becomes ready to communicate after the master/host core initializes a shared memory area, and it announces its readiness by creating a new RPMsg channel. At this point, the ns_callback is called, and the host processor creates an endpoint to communicate over that RPMsg channel. Note the firmware is parsed and loaded from an elf file stored on the filesystem, I can share the source somewhere later as an example or for testing.

import openamp
import time

ept = None


def ept_recv_callback(src_addr, data):
    print(f'Received message on endpoint "{data}"')


def ns_bind_callback(src_addr, name):
    global ept
    print(f'New service announcement src address: {src_addr} name: "{name}"')
    # Create a new RPMsg endpoint to communicate with the M4.
    ept = openamp.RPMsg(
        name, src_addr=openamp.RPMSG_ADDR_ANY, dst_addr=src_addr, callback=ept_recv_callback
    )


# Initialize OpenAMP. Name service callback is called when the remote
# processor creates an RPMsg channel, and is ready to communicate.
openamp.init(ns_callback=ns_bind_callback)

# Create a remoteproc object, load its firmware and start it.
# Note you can also start from an entry point (ex 0x081E0000)
#rproc = openamp.RProc(0x081E0000)
rproc = openamp.RProc("vuart.elf")
rproc.start()

while True:
    if ept is not None:
        ept.send("Hello World!", timeout=1000)
    time.sleep_ms(1000)

Output:

registered generic bus
remoteproc_load: check remoteproc status
remoteproc_load: open executable image
remoteproc_load: check loader
remoteproc_load: loading headers
Loading ELF headering
Loading ELF program header.
Loading ELF section header.
remoteproc_load, load header 0x0, 0x1000, next 0x18098, 0x258
Loading ELF section header.
Loading ELF section header complete.
Loading ELF shstrtab.
remoteproc_load, load header 0x18098, 0x258, next 0x1800d, 0x8a
Loading ELF shstrtab.
remoteproc_load, load header 0x1800d, 0x8a, next 0x1800d, 0x0
remoteproc_load: load executable data
segment: 1, total segs 1
load data: da 0x38000000, offset 0x10000, len = 0x7f90, memsize = 0xc138, state 0x20801
load data: da 0xffffffff, offset 0x0, len = 0x0, memsize = 0x0, state 0x40801
remoteproc_load: successfully load firmware
New service announcement src address: 1024 name: "m4-vuart-channel"
Received message on endpoint "Hello from M4!"

The firmware running on the M4 can be found here: https://github.com/iabdalkader/openamp_vuart

Todo:

• [x] Handle file system access in a generic way using mp_vfs
• [x] Support endpoint send/send non-blocking and off-channel.
• [x] Add optional trace buffer resource.
• [x] Support loading/starting firmware from Flash address.
• [x] Support loading/starting firmware from SD-RAM on H7.
• [x] Provide an example for the H7 M4 side (optional).

• Note about libmetal it's basically impossible to add its upstream as a submodule, since it only supports cmake, and it hard-codes configure_file variables into headers, and it's basically expected to be built into a static library and installed somewhere. So for now I've used my own fork, with minor changes to make it more "MicroPython-friendly". This library hardly changes, I think it would make sense to maintain our own fork, I can transfer ownership of the libmetal repo to MicroPython later if you want, I have no need to maintain it.

[github-actions[bot]]

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:   +32 +0.004% standard
      stm32:   +24 +0.006% PYBV10
     mimxrt:   +24 +0.007% TEENSY40
        rp2:   +16 +0.005% RPI_PICO
       samd:   +24 +0.009% ADAFRUIT_ITSYBITSY_M4_EXPRESS

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.39%. Comparing base (7f5d8c4) to head (df2ff0c).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #12961   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21078    21079    +1     
=======================================
+ Hits        20739    20740    +1     
  Misses        339      339           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[iabdalkader]

@dpgeorge

It would be really nice if we can update the CMSIS headers to support zero and copy tables, this way I can remove the function that initializes the shared resource table (openamp_rsc_table_init) .

https://github.com/ARM-software/CMSIS_5/blob/a75f01746df18bb5b929dfb8dc6c9407fac3a0f3/CMSIS/Core/Include/cmsis_gcc.h#L137

[dpgeorge]

It would be really nice if we can update the CMSIS headers to support zero and copy tables

We can. But not until after the upcoming release, which will be in the next couple of weeks.

[iabdalkader]

It would be really nice if we can update the CMSIS headers to support zero and copy tables

We can. But not until after the upcoming release, which will be in the next couple of weeks.

I'm still waiting on the feedback, but in the meantime, I was wondering if now would be a good time to update CMSIS to support copy/zero tables ? It would help with simplifying some of the code here, but might also be required for other future ports.

[iabdalkader]

Hi @jimmo

Thanks for the thorough review! I'll get around to addressing all of the review comments next week, when I get back from vacation.

This would mean moving the implementation of e.g. ns_bind_callback into C, with the expectation that a standard one-size-fit-all way of matching up endpoints would for most use cases.

I think we can have both, I need to double check to be sure, but we can either create the channel in the announcement callback, or have it created beforehand. In the latter case, it will remain unconnected until the remote opens the channel, then open-amp will match it. In the C callback I can check if the Python callback is None, then assume the channel is already created, note I may not even have to do that as it may not call the callback in that case. Anyway, if this is the case, I assume there's no problem in keeping the Python ns_bind_callback option, right ?

We note that Zephyr chooses the master/slave role statically. Eventually it would be nice to make this dynamic (and that doesn't seem too difficult) but for now it seems reasonable to make it static at compile time.

I've only added support for Master/Host mode, there's no support for a remote role, not even statically, because to target the remote cores, you'd first have to support building MicroPython for the M4 core of the STM32H7 or the RT1170 for example, so I left this as an exercise for the reader :)

To that end, Zephyr has done a very good job of providing a high-level API

I haven't looked at the Zephyr implementation, but the general idea here might be the opposite. I wanted to provide a sort of "low-level" Open-AMP Python API, on top of which you can build more complex Python libraries, not something that's high-level. For example I use the current API in a MSGPack RPC module that supports sync/async remote function calls, it's unpublished yet, but here's an example usage, you can see how it wraps the API details:

    rpc = msgpackrpc.MsgPackRPC()

    # Register objects or functions to be called by the remote processor.
    rpc.bind(function_to_called_by_remote)

    # Start the remote processor and wait for it to be ready to communicate.
    rpc.start(firmware=0x08180000, timeout=1000, num_channels=2)

    # Sync call
    print(rpc.call("add", 1, 1))

    # Async call
    future = rpc.call_async("add", 1, 2)
    print(future.join())

That said, I understand that there might be simple use cases in which a send/recv data in a loop is all that's needed, but I think we can still have that without sacrificing the flexibility of the current API.

But really, what we'd love to see is that the Endpoint implement the stream API (exactly like a UDP socket). This would make this directly usable from asyncio. The stream would become writable once connected.

Note that there's no rpmsg_recv function you basically get the buffer and its size in the recv_callback and then you're expected to consume it immediately. That said, I think we can still have both here too. If the callback is None, buffer the message in a FIFO and return it in a recv call or stream read (ex. "streaming mode"), but is this something that has to be implemented right away or can it be a later improvement ?

Or better, using asyncio.

It's important to say that I intentionally didn't want this to have a hard-dependency on asyncio because then everything we write that uses it has to be asyncio code, but having an asyncio option is fine though, as long as we don't have to use asyncio to use open-amp.

[iabdalkader]

@jimmo PR updated, I think I've fixed all of the minor naming, files etc.. issues in the review, if I missed something let me know.

• Re libmetal, I forked the upstream repo and put all changes required to support MicroPython build in a branch based on the latest stable tag. Note that in the upstream repo the files are in libmetal/lib, but the same files include from metal/*. It's done this way because you're supposed to build the library which will then generate a metal directory, not use it directly, so I also had to git mv lib metal for this to work. BTW we can try to push a fix for this upstream, they seem reasonable, if you have a suggestion maybe open an issue upstream.

• Re endpoint management, I've added support for both endpoint use cases; you can either create the channel in the announcement callback (if one is provided), or register the endpoint locally and it will be bound when the remote announces the services. So this example works:

def ept_recv_callback(src_addr, data):
    print("Received message on endpoint", data)

openamp.init()
ept = openamp.RPMsg("vuart-channel", callback=ept_recv_callback)
rproc = openamp.RProc("vuart.elf")
rproc.start()

while True:
    if ept.is_ready():
        ept.send("HelloWorld from M7")

Or you can provide a new service announcement callback i.e:

def ns_bind_callback(src_addr, name):
    global ept
    print(f'New service announcement src address: {src_addr} name: "{name}"')
    # Create a new RPMsg endpoint to communicate with the M4.
    ept = openamp.RPMsg(
        name, src_addr=openamp.RPMSG_ADDR_ANY, dst_addr=src_addr, callback=ept_recv_callback
    )

openamp.init(ns_callback=ns_bind_callback)
....

• Re elf loader, and loading .bin I provided the following options to disable the elf loader, rproc store, and store ops. Note they're enabled by default, disabling them saves about 7KBs:

MICROPY_PY_OPENAMP_RPROC_STORE_ENABLE
MICROPY_PY_OPENAMP_RPROC_ELFLD_ENABLE

As for the loading .bin, we can use rproc rproc_mmap provided by ports to map the memory (this will ensure the address is valid, writable etc..), then load the binary to memory and flush the cache in modopenamp.c, but I didn't implement this in this PR, I can send in a follow-up PR.

[jimmo]

Thanks for the update @iabdalkader -- I will reply with more comments, but wanted to specifically address the libmetal submodule first.

Here's an attempt at a very basic implementation of their pre-processor that allows us to use the upstream submodule. It also makes it much clearer what the minimal diff is to the "micropython" system (compared to the "generic" system that it's based on) as only three files change.

https://github.com/micropython/micropython/commit/52eef33d3e9dee981e590234ea77fcf5e14a87b9

Further to what @projectgus was saying about the directory/file naming of metal_port.c -- additionally the port-specific stuff should match the existing convention and use "mpXYZport.c". So I've changed it to "ports/stm32/mpmetalport.h". Perhaps this should be mpopenampport_libmetal.{h,c} to match what Angus was suggesting. (This sort of matches what we already do e.g. unix/mpbtstackport_h4.c).

[iabdalkader]

additionally the port-specific stuff should match the existing convention and use "mpXYZport.c".

I was following how mbedtls is ported (i.e. mbedtls/mbedtls_config.h, mbedtls/mbedtls_port.c) but I'm okay with mpmetalport.h, I don't think it should be any longer, will rename them.

I will add your changes to libmetal, thanks!

[iabdalkader]

@jimmo

Here's an attempt at a very basic implementation of their pre-processor that allows us to use the upstream submodule. It also makes it much clearer what the minimal diff is to the "micropython" system (compared to the "generic" system that it's based on) as only three files change.

52eef33

You missed a few files, for example https://github.com/OpenAMP/libmetal/compare/main...iabdalkader:libmetal:v2023.10.0-micropython#diff-1def8da686dc921f56f400212059bf90033737f5d7928085c7bd98a5a278c188

I'm going to add them back, did you remove them for a reason ?

[jimmo]

You missed a few files, for example OpenAMP/[email protected]:libmetal:v2023.10.0-micropython#diff-1def8da686dc921f56f400212059bf90033737f5d7928085c7bd98a5a278c188

I'm going to add them back, did you remove them for a reason ?

@iabdalkader There's no diff from the generic version there.

The steps I took were:

• Start with the version in https://github.com/iabdalkader/libmetal/tree/v2023.10.0-micropython
• Undo the @PLATFORM...@ change
• Diff each file in plaform/micropython with platform/generic
• Only keep the modified files. (With the exception of log.h which could be done directly in mpmetalport.h)

[iabdalkader]

Oh okay, so it falls back to the generic version.

[iabdalkader]

@jimmo

Only keep the modified files. (With the exception of log.h which could be done directly in mpmetalport.h)

It doesn't build with that change, because headers include log.h before the mpmetalport.h undefs it. I reverted the logging and confirmed that it is working again.

[iabdalkader]

I've pushed the branch again to reword the outdated commit messages, and add a config file include for open-amp. If needed the config file will allow ports/boards to override openamp/rproc options, provide a different resource table etc.. It should be defined in mk files following mbedtls and others:

MICROPY_PY_OPENAMP_CONFIG_FILE = '"$(BOARD_DIR)/openamp_config_board.h"'

I'm very happy with the current state of this feature, but let me know if there's something else to be fixed while we're at it. Note I don't know how/where to add the coverage test for stream seek function.

[jimmo]

Re endpoint management, I've added support for both endpoint use cases; you can either create the channel in the announcement callback (if one is provided), or register the endpoint locally and it will be bound when the remote announces the services.

Thanks for this, looks good!

(For my own curiosity, I'm still not entirely clear why the first way is needed though, i.e. what scenario does that support that wasn't possible the second way?)

[iabdalkader]

(For my own curiosity, I'm still not entirely clear why the first way is needed though, i.e. what scenario does that support that wasn't possible the second way?)

Although probably unlikely, but a scenario which is impossible without ns_callback if you don't know the exact names, or numbers, of the endpoints the remote will announce beforehand, or if the endpoint name is random (for some reason). In this case you must create endpoints on the fly. But more commonly you may just not care about the exact name of endpoint that will be created by the remote firmware, or if it might change you don't want it to be hard-coded in the Python script (which might be a frozen module).

ports/unix/coverage.c is where we add tests for the C API, there are already some stream examples there.

I'm still not sure how to add it.

[dpgeorge]

I pushed a commit to this branch which refactors the stream seek code, and now the coverage passes because the new code is tested by existing tests.

[dpgeorge]

@iabdalkader can you please add some initial documentation for the new openamp module introduced here?

[iabdalkader]

@iabdalkader can you please add some initial documentation for the new openamp module introduced here?

I can try, but the API is not finalized yet. We need to decide what to do with init() function it's been discussed in one of the review comments. Also, now that RPMsg was renamed to Endpoint I think RProc maybe should also be renamed to Remote or RemoteProc @jimmo any thoughts on this ?

[jimmo]

I think RProc maybe should also be renamed to Remote or RemoteProc @jimmo any thoughts on this ?

@iabdalkader Yes this sounds good. Thanks

[iabdalkader]

I pushed a commit to this branch which refactors the stream seek code, and now the coverage passes because the new code is tested by existing tests.

@dpgeorge Thanks! But for some reason there are more failures now. Note I rebased on master and squashed the last commit.

[dpgeorge]

I can try, but the API is not finalized yet.

I think it's still worth starting on the docs, at least to have a skeleton there with the current API as it stands. That will give a good summary/overview of the new modules/functions/classes/methods.

[iabdalkader]

@dpgeorge Docs added and all CI issues fixed.

[iabdalkader]

@dpgeorge Since you went through API docs, is it safe to assume that the API will be stable after those changes ? Because then I can merge (and send) other PRs that use this API, while waiting for this PR to be merged.

[iabdalkader]

Rebased to fix conflicts and enabled OpenAMP for all H747 boards.

Is there anything else that needs to be done here ?

[dpgeorge]

Since you went through API docs, is it safe to assume that the API will be stable after those changes ?

Yes, the API looks stable now.

[iabdalkader]

Pushed a minor fix to detect if the CM4 is configured for auto-boot, via option bytes, and handle that case gracefully. Note in this case the CM4 can't be started, and options bytes must be disabled first (using ST tools or Arduino IDE or something else).

[dpgeorge]

Note: I'm working on this PR and will merge it shortly.

[iabdalkader]

I reviewed the commits again, everything seems to be in order.

[dpgeorge]

Merged!!

Thanks @iabdalkader for implementing this, I think it will be a great feature for multicore SoCs, of which there are more and more these days.