ipc: icmsg: Add mutual exclussion access to shmem

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>
Willmish · Apr 29, 2023 · 169d8fa · 169d8fa
1 parent 52e203f
commit 169d8fa
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 10 deletions.
diff --git a/include/zephyr/ipc/icmsg.h b/include/zephyr/ipc/icmsg.h
@@ -46,6 +46,9 @@ struct icmsg_data_t {
 	struct spsc_pbuf *tx_ib;
 	struct spsc_pbuf *rx_ib;
 	atomic_t send_buffer_reserved;
+#ifdef CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC
+	struct k_mutex send;
+#endif
 
 	/* Callbacks for an endpoint. */
 	const struct ipc_service_cb *cb;

diff --git a/subsys/ipc/ipc_service/lib/Kconfig.icmsg b/subsys/ipc/ipc_service/lib/Kconfig.icmsg
@@ -18,6 +18,25 @@ config IPC_SERVICE_ICMSG_NOCOPY_RX
 	  Enable nocopy feature for receiving path of the icmsg library that
 	  might be used by backends based on icmsg.
 
+config IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC
+	bool "Synchronize access to shared memory"
+	default y
+	help
+	  Provide synchronization access to shared memory at a library level.
+	  This option is enabled by default to allow to use sending API from
+	  multiple contexts. Mutex is used to guard access to the memory.
+	  This option can be safely disabled if an application ensures data
+	  are sent from single context.
+
+config IPC_SERVICE_ICMSG_SHMEM_ACCESS_TO_MS
+	int "Mutex lock timeout in milliseconds"
+	depends on IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC
+	range 1 5
+	default 1
+	help
+	  Maximum time to wait, in milliseconds, for access to send data with
+	  backends basing on icmsg library. This time should be relatively low.
+
 # The Icmsg library in its simplicity requires the system workqueue to execute
 # at a cooperative priority.
 config SYSTEM_WORKQUEUE_PRIORITY

diff --git a/subsys/ipc/ipc_service/lib/icmsg.c b/subsys/ipc/ipc_service/lib/icmsg.c
@@ -11,8 +11,9 @@
 #include <zephyr/sys/atomic.h>
 #include <zephyr/sys/spsc_pbuf.h>
 
-#define RX_BUF_SIZE	CONFIG_IPC_SERVICE_ICMSG_CB_BUF_SIZE
-#define BOND_NOTIFY_REPEAT_TO K_MSEC(CONFIG_IPC_SERVICE_ICMSG_BOND_NOTIFY_REPEAT_TO_MS)
+#define RX_BUF_SIZE		CONFIG_IPC_SERVICE_ICMSG_CB_BUF_SIZE
+#define BOND_NOTIFY_REPEAT_TO	K_MSEC(CONFIG_IPC_SERVICE_ICMSG_BOND_NOTIFY_REPEAT_TO_MS)
+#define SHMEM_ACCESS_TO		K_MSEC(CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_TO_MS)
 
 #define RX_BUFFER_RELEASED	0
 #define RX_BUFFER_HELD		1
@@ -77,23 +78,37 @@ static bool is_send_buffer_reserved(struct icmsg_data_t *dev_data)
 
 static int reserve_send_buffer_if_unused(struct icmsg_data_t *dev_data)
 {
-	bool was_unused;
+#ifdef CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC
+	int ret = k_mutex_lock(&dev_data->send, SHMEM_ACCESS_TO);
 
-	was_unused = atomic_cas(&dev_data->send_buffer_reserved,
+	if (ret < 0) {
+		return ret;
+	}
+#endif
+
+	bool was_unused = atomic_cas(&dev_data->send_buffer_reserved,
 				  SEND_BUFFER_UNUSED, SEND_BUFFER_RESERVED);
 
 	return was_unused ? 0 : -EALREADY;
 }
 
 static int release_send_buffer(struct icmsg_data_t *dev_data)
 {
-	bool was_reserved;
+	bool was_reserved = atomic_cas(&dev_data->send_buffer_reserved,
+					SEND_BUFFER_RESERVED, SEND_BUFFER_UNUSED);
 
-	was_reserved = atomic_cas(&dev_data->send_buffer_reserved,
-				  SEND_BUFFER_RESERVED, SEND_BUFFER_UNUSED);
-	return was_reserved ? 0 : -EALREADY;
+	if (!was_reserved) {
+		return -EALREADY;
+	}
+
+#ifdef CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC
+	return k_mutex_unlock(&dev_data->send);
+#else
+	return 0;
+#endif
 }
 
+
 static bool is_rx_buffer_free(struct icmsg_data_t *dev_data)
 {
 #ifdef CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX
@@ -222,6 +237,10 @@ int icmsg_open(const struct icmsg_config_t *conf,
 	dev_data->ctx = ctx;
 	dev_data->cfg = conf;
 
+#ifdef CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC
+	k_mutex_init(&dev_data->send);
+#endif
+
 	dev_data->tx_ib = spsc_pbuf_init((void *)conf->tx_shm_addr,
 					 conf->tx_shm_size,
 					 SPSC_PBUF_CACHE);
@@ -286,7 +305,7 @@ int icmsg_send(const struct icmsg_config_t *conf,
 	}
 
 	ret = reserve_send_buffer_if_unused(dev_data);
-	if (ret) {
+	if (ret < 0) {
 		return -ENOBUFS;
 	}
 
@@ -332,7 +351,7 @@ int icmsg_get_tx_buffer(const struct icmsg_config_t *conf,
 	}
 
 	ret = reserve_send_buffer_if_unused(dev_data);
-	if (ret) {
+	if (ret < 0) {
 		return -ENOBUFS;
 	}