This commit fixes the issue where a serialization
error was reported after properly sending a data with 'icbmsg' backend.
The icbmsg send function's return code is set to
the sent data's len as in other backends.
The related docs were fixed and updated.
Signed-off-by: Piotr Koziar <piotr.koziar@nordicsemi.no>
This change works around the issue with the semaphore timeout during
the Bluetooth HCI driver initialization when the bt_enable function
is called in the context of the System Workqueue thread. This issue
only affects platform that use the IPC service and its ICBMsg backend
(e.g. the nRF54H20 DK target).
The bt_enable function, when called in the System Workqueue context,
results in a deadlock, as the waiting semaphore of the Bluetooth HCI
driver times out:
bt_hci_driver: Endpoint binding failed with -11
During the Bluetooth HCI driver open operation in the context of the
bt_enable function, the driver code waits using the semaphore for the
endpoint binding process of the IPC service module to finalize. The
issue occurs when the waiting occurs in the System Workqueue context.
The ICBMsg backend from the IPC service schedules a system work during
the endpoint registration, in which it finalizes the binding operation
- also in the System Workqueue context. As the Bluetooth HCI driver
with its wait operation keeps the System Workqueue context busy, the
endpoint binding cannot be completed by the ICBMsg backend before the
HCI driver semaphore timeout.
Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>
This patch adds names to the threads created by `mbox_init` via
`k_work_queue_start`. The name of the thread is the same as the name in
the device instance.
The main reason for this has to do with how `mcumgr` reports [thread
information](https://docs.zephyrproject.org/latest/services/device_mgmt/smp_groups/smp_group_0.html#task-statistics-command).
Specifically, data about threads is sent in a CBOR encoded map where the
map keys are the thread names (at least in the default configuration). If
there's more than one IPC channel defined (one example of this being
`samples/subsys/logging/multidomain`), both threads would be created
with an empty name and `mcumgr` will only return the data associated
with the thread created by the last call to `mbox_init`.
Signed-off-by: Bogdan Marinescu <bogdan.marinescu@gmail.com>
If compiling with optimizations (-O2/-Ofast) gcc emits a
maybe-uninitialized warning for `size`.
As far as I can tell, `size` will always be initialized in the cases
where it is used and only left uninitialized in error cases.
Reproduced on main with the multi_endpoint sample on
nrf5340dk/nrf5340/cpuapp.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
This fixes warnings when building with icbmsg
backend for IPC service by defining
_POSIX_C_SOURCE 200809L in source file.
Signed-off-by: Kamil Gawor <Kamil.Gawor@nordicsemi.no>
Update all usages of the MBOX API to the latest API changes (to be
squashed for bisectability).
Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
RPMsg currently sends four dummy bytes over the IPM when sending a
notification/interrupt to another CPU. However, most IPM drivers
support sending messages with no data transfer, and for some IPM
drivers, like the STM32 HSEM IPM driver, data transfer is not
possible at all, thus breaking compatibility. To fix this, send an
empty message with size 0 instead of a dummy message if the STM32
HSEM IPM driver is active.
Signed-off-by: Celina Sophie Kalus <hello@celinakalus.de>
The work item will attempt to dereference pointers that have been nulled
by the backend.
To avoid that, wait until all items currently on the queue have been
processed.
The symptom is a busfault on ARM, and is "fixed" by adding a
`k_msleep(1)` right before `ipc_service_deregister_endpoint()`. This
will in effect do the same thing as this patch, and allow the scheduler
to run the work item on the ipc workqueue.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
When using multiple instances of IPC static vring, each instance
should register its shared memory with a unique name.
Instead of a predefined default name, the name of the ipc instance
is reused for the metal device name.
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
The icmsg and icmsg_me backends has limitations in context of
concurrent access to single instance. Some limitations are:
* sending by more thread at the same time may cause -EBUSY
* allocating TX buffer will cause errors in other threads that want
to allocate before first thread sent the message,
* during no-copy receive, when RX buffer is on hold receiving is
totally blocked.
This backend resolves those limitations by adding dynamically allocated
buffers on shared memory next to ICmsg circular buffer. The data is
passed using those buffers, so concurrency is not a problem. The ICmsg
is used only to pass short 2-byte messages containing references to
those buffers. The backend also supports multiple endpoint.
The ipc/icmsg_me sample was modified to support this backend.
Signed-off-by: Dominik Kilian <Dominik.Kilian@nordicsemi.no>
Allow magic number to be longer than the sizeof(magic).
This will allow to support future versions of the icmsg
with backwards compatibility.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
Replace spsc_pbuf with pbuf implementation dedicated to
be used by IcMsg based backends.
The pbuf is written on top of simple read/write semantics
with minimal footprint and code complexity
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
This adds implementation of secure packed buffer.
Secure packed buffer is added with intention to be used
in icmsg backed backends.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
Remove nocopy feature from icmsg_me backend.
The backend is not meant to be used to send big
data chunks thus no-copy feature is removed.
If one wants to use no-copy it is recommended
to use icmsg_me only for control messages while
allocator will shall be written seperately.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
To enable runninng and testing this backend on the POSIX
architecture, we need to allow defining the shared memory
buffer as a compiler provided symbol, instead of a hard
address provided by the DTS.
For this case we refer to a symbol a POSIX arch implementation
(typically the board) needs to provide.
It is the responsability of that implementation to ensure
that symbol exists and points to a memory buffer of the
DT defined size.
Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
The ICMSG backend now has a dedicated workqueue to process incoming IPC
messages. The system workqueue is no longer utilized for that purpose.
Testing shows that in certain scenarios substituting a RPMsg backend
with ICMsg results in deadlocks.
The deadlocks were a symptom of running a synchronous RPC protocol from
the context of the system workqueue and transpired as follows:
1. The RPC protocol sends a request over the ICMsg backend on the system
workqueue thread.
2. The RPC protocol puts the thread to sleep until response is received.
This puts the system workqueue thread to sleep.
3. The response to the request arrives over ICMsg backend.
4. The backend signals a work item to the system workqueue.
5. The system workqueue is unable to process the response due to being
previously pended on the RPC request.
The deadlock was initially observed with the nrf-802154 driver in
conjuntion with the IPv6 stack.
To prevent this condition from occurring, the approach was selected to
give ICMsg a dedicated workqueue thread.
Added a Kconfig option that enables the dedicated workqueue by default.
The config can be disabled, if the user wants to preserve RAM capacity
and is certain that the deadlock condition is not encountered.
Signed-off-by: Rafał Kuźnia <rafal.kuznia@nordicsemi.no>
currently when no enough memory is found a (1 << (-1)) value is returned
instead of 0.
Fix this by returning 0 and simplify log2 computation
Signed-off-by: Riadh Ghaddab <rghaddab@baylibre.com>
This patchset is doing three things:
1. It is fixing the bogus algorithm to find the optimal number of
descriptors for a given memory size.
2. It is changing values for VDEV_STATUS_SIZE and
IPC_SERVICE_STATIC_VRINGS_ALIGNMENT to better align to a usual cache
line size.
3. RX/TX VRINGs are now correctly aligned to MEM_ALIGNMENT (and cache
line alignment).
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
The "virtio_" prefix is used by the open-amp library API.
Rename local functions using "ipc_virtio_" prefix to avoid
function redefinition.
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
This remanes send_buffer to tx_buffer to be
consistent with Rx buffer naming and with
mbox naming.
Signed-off-by: Kamil Gawor <Kamil.Gawor@nordicsemi.no>
This improves packet reception and fix an issue
where packets bigger than internal Rx buffer
were silently dropped. In current solution
local data coping for reception is not needed.
User gets direct pointer to shared memory which
allow to efficient receive as much data as was sent.
Signed-off-by: Kamil Gawor <Kamil.Gawor@nordicsemi.no>
If the icmsg backend is sending the data from many
contexts, the send buffer must be accessed only by one
context at a time. Ensure that by adding mutex when
sending.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
Clearing of shared memory by one side of the communication
is no longer required after
commit 0620cb1fe1
("ipc: ipc_service: icmsg: Increase reliability of bonding")
was merged.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
The init infrastructure, found in `init.h`, is currently used by:
- `SYS_INIT`: to call functions before `main`
- `DEVICE_*`: to initialize devices
They are all sorted according to an initialization level + a priority.
`SYS_INIT` calls are really orthogonal to devices, however, the required
function signature requires a `const struct device *dev` as a first
argument. The only reason for that is because the same init machinery is
used by devices, so we have something like:
```c
struct init_entry {
int (*init)(const struct device *dev);
/* only set by DEVICE_*, otherwise NULL */
const struct device *dev;
}
```
As a result, we end up with such weird/ugly pattern:
```c
static int my_init(const struct device *dev)
{
/* always NULL! add ARG_UNUSED to avoid compiler warning */
ARG_UNUSED(dev);
...
}
```
This is really a result of poor internals isolation. This patch proposes
a to make init entries more flexible so that they can accept sytem
initialization calls like this:
```c
static int my_init(void)
{
...
}
```
This is achieved using a union:
```c
union init_function {
/* for SYS_INIT, used when init_entry.dev == NULL */
int (*sys)(void);
/* for DEVICE*, used when init_entry.dev != NULL */
int (*dev)(const struct device *dev);
};
struct init_entry {
/* stores init function (either for SYS_INIT or DEVICE*)
union init_function init_fn;
/* stores device pointer for DEVICE*, NULL for SYS_INIT. Allows
* to know which union entry to call.
*/
const struct device *dev;
}
```
This solution **does not increase ROM usage**, and allows to offer clean
public APIs for both SYS_INIT and DEVICE*. Note that however, init
machinery keeps a coupling with devices.
**NOTE**: This is a breaking change! All `SYS_INIT` functions will need
to be converted to the new signature. See the script offered in the
following commit.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
init: convert SYS_INIT functions to the new signature
Conversion scripted using scripts/utils/migrate_sys_init.py.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
manifest: update projects for SYS_INIT changes
Update modules with updated SYS_INIT calls:
- hal_ti
- lvgl
- sof
- TraceRecorderSource
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
tests: devicetree: devices: adjust test
Adjust test according to the recently introduced SYS_INIT
infrastructure.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
tests: kernel: threads: adjust SYS_INIT call
Adjust to the new signature: int (*init_fn)(void);
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
This commit removes the possible race condition when notifying
the bonding request.
When the race condition appears there is possibility that
one of the cores would never send notification for bonding.
This change makes it sure that the notification would be sent
at least once.
Signed-off-by: Radoslaw Koppel <radoslaw.koppel@nordicsemi.no>
This change fixes the deregister functionality for
icmsg backend. There are two changes:
Memory access layer was not initialized properly
when the endpoint was registered again. This lead to
memory fault when attepmt to write to the buffer was
done.
Mbox driver was initialized before memory access layer
was initialized. This could lead to race condition
where immediately after the mbox driver was initialized,
the receiving work is scheduled due to pending interrupt.
In that case an attempt to read from garbage address will
occur.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
The multi-endpoint backend of the ipc_service subsystem contains two
roles with separated implementations: initiator and follower. There
was many code duplications for both roles. This patch introduces a new
IPC library: icmsg_me containing common code extracted from both roles
and encapsulating access to common data fields.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
Add nocopy feature to icmsg-me initiator and follower roles
in their receive path. This feature is optional, configured
with Kconfig.
Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
Add a nocopy feature for sending message through the icmsg IPC library.
Also eliminate data races if multiple threads are using the
same ipc instance and are trying to send a message simultaneously.
Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
This patch adds an optional nocopy feature for RX message to the icmsg
IPC library. The nocopy feature can be enabled using project
configuration.
If this feature is not used by icmsg users it is recommended to disable it
to reduce memory usage and improve run-time performance.
Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
On system startup icmsg headers space must be cleared before
cores start handshake procedure. The simplest way to enforce
it is to clear memory by the core responsible by enabling
the remote core, before the remote core is enabled and before
the handshake is started.
This patch ensures that nRF53 APP core clears both TX and RX
memory for icmsg before it starts NET core.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
This commit enables the initiator or the follower
role for the ICMSG multi-endpoint backend
depending on dts by default.
It is needed when BT_RPMSG transport for HCI is used.
Signed-off-by: Kamil Gawor <Kamil.Gawor@nordicsemi.no>
Separate ipc_service libraries Kconfig entries from ipc_service backend
entries. IPC_SERVICE_BACKEND_ICMSG_BOND_NOTIFY_REPEAT_TO_MS is renamed
to drop BACKEND part, because this entry applies to a library, not to
one backend. Icmsg related entries are grouped now in a menuconfig for
cleaner presentation in configuration editors.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
This commit changes the way bonding between endpoints is processed.
There is no blind attempt to read the buffer without mbox notification.
On second side the notification is repeated multiple times until valid
bonding is detected.
Signed-off-by: Radoslaw Koppel <radoslaw.koppel@nordicsemi.no>
The icmsg backend for ipc_service has a limitation of supporting only
on endpoint. This limitation is acceptable for many IPC instances.
However, some require to use multiple endpoints sharing a single
instance. To preserve the simple and the most efficient single-instance
backend, a separated backend is introduced implementing a wrapper
around icmsg core which adds multiple endpoints support.
There are two multi-endpoint ipc_service icmsg backends: one in the
initiator role, and the other one in the follower role. In a IPC
configuration one end of communication must be in the follower role
while the other one is in the initiator. The initiator initiates
an endpoint discovery handshake to establish enpoint identifiers for
requested endpoint names. The follower responds to requests sent by
the initiator.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
