[nrf fromtree] ipc_service: icmsg: Add "unbound" functionality

doc/services/ipc/ipc_service/backends/ipc_service_icmsg.rst

@@ -83,8 +83,7 @@ connected through the IPC instance:
    memory.
 #. It then sends a signal to the other domain or CPU, informing that the data
    has been written. Sending the signal to the other domain or CPU is repeated
-   with timeout specified by
-   :kconfig:option:`CONFIG_IPC_SERVICE_ICMSG_BOND_NOTIFY_REPEAT_TO_MS` option.
+   with timeout.
 #. When the signal from the other domain or CPU is received, the magic number
    is read from ``rx-region``. If it is correct, the bonding process is finished
    and the backend informs the application by calling

dts/bindings/ipc/zephyr,ipc-icmsg.yaml

@@ -21,6 +21,21 @@ properties:
     required: true
     type: phandle
 
+  unbound:
+    type: string
+    enum:
+      - disable
+      - enable
+      - detect
+    default: disable
+    description: |
+      Select unbound() callback mode. The callback can be enabled or disabled with the
+      "enable" and "disable" option respectively. This functionality requires session
+      number handshake procedure on both sides, so you cannot set "enable" on one side
+      and "disable" on the other. The "detect" mode detects if the remote side
+      supports handshake procedure and adjusts its behavior accordingly. The
+      "detect" mode is possible only if "dcache-alignment" is at least 8 bytes.
+
   dcache-alignment:
     type: int
     description: |

include/zephyr/ipc/icmsg.h

@@ -27,14 +27,58 @@ extern "C" {
  */
 
 enum icmsg_state {
+	/** Instance is not initialized yet. In this state: sending will fail, opening allowed.
+	 */
 	ICMSG_STATE_OFF,
-	ICMSG_STATE_BUSY,
-	ICMSG_STATE_READY,
+
+	/** Instance is initializing without session handshake. In this state: sending will fail,
+	 * opening will fail.
+	 */
+	ICMSG_STATE_INITIALIZING_SID_DISABLED,
+
+	/** Instance is initializing with session handshake. It is waiting for remote to acknowledge
+	 * local session id. In this state: sending will fail, opening is allowed (local session id
+	 * will change, so the remote may get unbound() callback).
+	 */
+	ICMSG_STATE_INITIALIZING_SID_ENABLED,
+
+	/** Instance is initializing with detection of session handshake support on remote side.
+	 * It is waiting for remote to acknowledge local session id or to send magic bytes.
+	 * In this state: sending will fail, opening is allowed (local session id
+	 * will change, so the remote may get unbound() callback if it supports it).
+	 */
+	ICMSG_STATE_INITIALIZING_SID_DETECT,
+
+	/** Instance was closed on remote side. The unbound() callback was send on local side.
+	 * In this state: sending will be silently discarded (there may be outdated sends),
+	 * opening is allowed.
+	 */
+	ICMSG_STATE_DISCONNECTED,
+
+	/* Connected states must be at the end. */
+
+	/** Instance is connected without session handshake support. In this state: sending will be
+	 * successful, opening will fail.
+	 */
+	ICMSG_STATE_CONNECTED_SID_DISABLED,
+
+	/** Instance is connected with session handshake support. In this state: sending will be
+	 * successful, opening is allowed (session will change and remote will get unbound()
+	 * callback).
+	 */
+	ICMSG_STATE_CONNECTED_SID_ENABLED,
+};
+
+enum icmsg_unbound_mode {
+	ICMSG_UNBOUND_MODE_DISABLE = ICMSG_STATE_INITIALIZING_SID_DISABLED,
+	ICMSG_UNBOUND_MODE_ENABLE = ICMSG_STATE_INITIALIZING_SID_ENABLED,
+	ICMSG_UNBOUND_MODE_DETECT = ICMSG_STATE_INITIALIZING_SID_DETECT,
 };
 
 struct icmsg_config_t {
 	struct mbox_dt_spec mbox_tx;
 	struct mbox_dt_spec mbox_rx;
+	enum icmsg_unbound_mode unbound_mode;
 };
 
 struct icmsg_data_t {
@@ -52,9 +96,10 @@ struct icmsg_data_t {
 	/* General */
 	const struct icmsg_config_t *cfg;
 #ifdef CONFIG_MULTITHREADING
-	struct k_work_delayable notify_work;
 	struct k_work mbox_work;
 #endif
+	uint16_t remote_sid;
+	uint16_t local_sid;
 	atomic_t state;
 };
 

include/zephyr/ipc/ipc_service.h

@@ -151,6 +151,21 @@ struct ipc_service_cb {
 	 */
 	void (*bound)(void *priv);
 
+	/** @brief The endpoint unbound by the remote.
+	 *
+	 *  This callback is called when the endpoint binding is removed. It may happen on
+	 *  different reasons, e.g. when the remote deregistered the endpoint, connection was
+	 *  lost, or remote CPU got reset.
+	 *
+	 *  You may want to do some cleanup, resetting, e.t.c. and after that if you want to bound
+	 *  again, you can register the endpoint. When the remote becomes available again and it
+	 *  also registers the endpoint, the binding will be reestablished and the `bound()`
+	 *  callback will be called.
+	 *
+	 *  @param[in] priv Private user data.
+	 */
+	void (*unbound)(void *priv);
+
 	/** @brief New packet arrived.
 	 *
 	 *  This callback is called when new data is received.

include/zephyr/ipc/pbuf.h

@@ -47,20 +47,23 @@ extern "C" {
  * The structure contains configuration data.
  */
 struct pbuf_cfg {
-	volatile uint32_t *rd_idx_loc;	/* Address of the variable holding
-					 * index value of the first valid byte
-					 * in data[].
-					 */
-	volatile uint32_t *wr_idx_loc;	/* Address of the variable holding
-					 * index value of the first free byte
-					 * in data[].
-					 */
-	uint32_t dcache_alignment;	/* CPU data cache line size in bytes.
-					 * Used for validation - TODO: To be
-					 * replaced by flags.
-					 */
-	uint32_t len;			/* Length of data[] in bytes. */
-	uint8_t *data_loc;		/* Location of the data[]. */
+	volatile uint32_t *rd_idx_loc;	 /* Address of the variable holding
+					  * index value of the first valid byte
+					  * in data[].
+					  */
+	volatile uint32_t *handshake_loc;/* Address of the variable holding
+					  * handshake information.
+					  */
+	volatile uint32_t *wr_idx_loc;	 /* Address of the variable holding
+					  * index value of the first free byte
+					  * in data[].
+					  */
+	uint32_t dcache_alignment;	 /* CPU data cache line size in bytes.
+					  * Used for validation - TODO: To be
+					  * replaced by flags.
+					  */
+	uint32_t len;			 /* Length of data[] in bytes. */
+	uint8_t *data_loc;		 /* Location of the data[]. */
 };
 
 /**
@@ -111,16 +114,21 @@ struct pbuf {
  * @param mem_addr	Memory address for pbuf.
  * @param size		Size of the memory.
  * @param dcache_align	Data cache alignment.
+ * @param use_handshake	Add handshake word inside shared memory that can be access with
+ *			@ref pbuf_handshake_read and @ref pbuf_handshake_write.
  */
-#define PBUF_CFG_INIT(mem_addr, size, dcache_align)					\
+#define PBUF_CFG_INIT(mem_addr, size, dcache_align, use_handshake)			\
 {											\
 	.rd_idx_loc = (uint32_t *)(mem_addr),						\
-	.wr_idx_loc = (uint32_t *)((uint8_t *)(mem_addr) +				\
-				MAX(dcache_align, _PBUF_IDX_SIZE)),			\
+	.handshake_loc = use_handshake ? (uint32_t *)((uint8_t *)(mem_addr) +		\
+				_PBUF_IDX_SIZE) : NULL,					\
+	.wr_idx_loc = (uint32_t *)((uint8_t *)(mem_addr) + MAX(dcache_align,		\
+				(use_handshake ? 2 : 1) * _PBUF_IDX_SIZE)),		\
 	.data_loc = (uint8_t *)((uint8_t *)(mem_addr) +					\
-				MAX(dcache_align, _PBUF_IDX_SIZE) + _PBUF_IDX_SIZE),	\
-	.len = (uint32_t)((uint32_t)(size) - MAX(dcache_align, _PBUF_IDX_SIZE) -	\
-				_PBUF_IDX_SIZE),					\
+				MAX(dcache_align, (use_handshake ? 2 : 1) *		\
+						  _PBUF_IDX_SIZE) + _PBUF_IDX_SIZE),	\
+	.len = (uint32_t)((uint32_t)(size) - MAX(dcache_align,				\
+			(use_handshake ? 2 : 1) * _PBUF_IDX_SIZE) - _PBUF_IDX_SIZE),	\
 	.dcache_alignment = (dcache_align),						\
 }
 
@@ -140,9 +148,11 @@ struct pbuf {
  * @param name			Name of the pbuf.
  * @param mem_addr		Memory address for pbuf.
  * @param size			Size of the memory.
- * @param dcache_align	Data cache line size.
+ * @param dcache_align		Data cache line size.
+ * @param use_handshake		Add handshake word inside shared memory that can be access with
+ *				@ref pbuf_handshake_read and @ref pbuf_handshake_write.
  */
-#define PBUF_DEFINE(name, mem_addr, size, dcache_align)					\
+#define PBUF_DEFINE(name, mem_addr, size, dcache_align, use_handshake, compatibility)	\
 	BUILD_ASSERT(dcache_align >= 0,							\
 			"Cache line size must be non negative.");			\
 	BUILD_ASSERT((size) > 0 && IS_PTR_ALIGNED_BYTES(size, _PBUF_IDX_SIZE),		\
@@ -151,8 +161,10 @@ struct pbuf {
 			"Misaligned memory.");						\
 	BUILD_ASSERT(size >= (MAX(dcache_align, _PBUF_IDX_SIZE) + _PBUF_IDX_SIZE +	\
 			_PBUF_MIN_DATA_LEN), "Insufficient size.");			\
+	BUILD_ASSERT(!(compatibility) || (dcache_align) >= 8,				\
+		"Data cache alignment must be at least 8 if compatibility is enabled.");\
 	static PBUF_MAYBE_CONST struct pbuf_cfg cfg_##name =				\
-			PBUF_CFG_INIT(mem_addr, size, dcache_align);			\
+			PBUF_CFG_INIT(mem_addr, size, dcache_align, use_handshake);	\
 	static struct pbuf name = {							\
 		.cfg = &cfg_##name,							\
 	}
@@ -223,6 +235,40 @@ int pbuf_write(struct pbuf *pb, const char *buf, uint16_t len);
  */
 int pbuf_read(struct pbuf *pb, char *buf, uint16_t len);
 
+/**
+ * @brief Read handshake word from pbuf.
+ *
+ * The pb must be defined with "PBUF_DEFINE" with "use_handshake" set.
+ *
+ * @param pb		A buffer from which data will be read.
+ * @retval uint32_t	The handshake word value.
+ */
+uint32_t pbuf_handshake_read(struct pbuf *pb);
+
+/**
+ * @brief Write handshake word to pbuf.
+ *
+ * The pb must be defined with "PBUF_DEFINE" with "use_handshake" set.
+ *
+ * @param pb		A buffer to which data will be written.
+ * @param value		A handshake value.
+ */
+void pbuf_handshake_write(struct pbuf *pb, uint32_t value);
+
+/**
+ * @brief Get first buffer from pbuf.
+ *
+ * This function retrieves buffer located at the beginning of queue.
+ * It will be continuous block since it is the first buffer.
+ *
+ * @param pb		A buffer from which data will be read.
+ * @param[out] buf	A pointer to output pointer to the date of the first buffer.
+ * @param[out] len	A pointer to output length the first buffer.
+ * @retval		0 on success.
+ *			-EINVAL when there is no buffer at the beginning of queue.
+ */
+int pbuf_get_initial_buf(struct pbuf *pb, volatile char **buf, uint16_t *len);
+
 /**
  * @}
  */

samples/subsys/ipc/ipc_service/icmsg/src/main.c

@@ -40,6 +40,11 @@ static void ep_bound(void *priv)
 	LOG_INF("Ep bounded");
 }
 
+static void ep_unbound(void *priv)
+{
+	LOG_INF("Ep unbounded");
+}
+
 static void ep_recv(const void *data, size_t len, void *priv)
 {
 #if defined(CONFIG_ASSERT)
@@ -68,6 +73,11 @@ static void ep_recv(const void *data, size_t len, void *priv)
 	}
 }
 
+static void ep_error(const char *message, void *priv)
+{
+	LOG_ERR("ICMsg error: %s", message);
+}
+
 static int send_for_time(struct ipc_ept *ep, const int64_t sending_time_ms)
 {
 	struct data_packet msg = {.data[0] = 'a'};
@@ -123,7 +133,9 @@ static int send_for_time(struct ipc_ept *ep, const int64_t sending_time_ms)
 static struct ipc_ept_cfg ep_cfg = {
 	.cb = {
 		.bound    = ep_bound,
+		.unbound  = ep_unbound,
 		.received = ep_recv,
+		.error    = ep_error,
 	},
 };
 

scripts/ci/check_compliance.py

@@ -1000,6 +1000,8 @@ def check_no_undef_outside_kconfig(self, kconf):
         "FOO_SETTING_2",
         "HEAP_MEM_POOL_ADD_SIZE_", # Used as an option matching prefix
         "HUGETLBFS",          # Linux, in boards/xtensa/intel_adsp_cavs25/doc
+        "IPC_SERVICE_ICMSG_BOND_NOTIFY_REPEAT_TO_MS", # Used in ICMsg tests for intercompatibility
+                                                      # with older versions of the ICMsg.
         "LIBGCC_RTLIB",
         "LLVM_USE_LD",   # Both LLVM_USE_* are in cmake/toolchain/llvm/Kconfig
         "LLVM_USE_LLD",  # which are only included if LLVM is selected but

subsys/ipc/ipc_service/backends/ipc_icbmsg.c

@@ -1408,11 +1408,11 @@ const static struct ipc_service_backend backend_ops = {
 	PBUF_DEFINE(tx_icbmsg_pb_##i,							\
 			GET_MEM_ADDR_INST(i, tx),					\
 			GET_ICMSG_SIZE_INST(i, tx, rx),					\
-			GET_CACHE_ALIGNMENT(i));					\
+			GET_CACHE_ALIGNMENT(i), 0, 0);					\
 	PBUF_DEFINE(rx_icbmsg_pb_##i,							\
 			GET_MEM_ADDR_INST(i, rx),					\
 			GET_ICMSG_SIZE_INST(i, rx, tx),					\
-			GET_CACHE_ALIGNMENT(i));					\
+			GET_CACHE_ALIGNMENT(i), 0, 0);					\
 	static struct backend_data backend_data_##i = {					\
 		.control_data = {							\
 			.tx_pb = &tx_icbmsg_pb_##i,					\
@@ -1424,6 +1424,7 @@ const static struct ipc_service_backend backend_ops = {
 		.control_config = {							\
 			.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx),			\
 			.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx),			\
+			.unbound_mode = ICMSG_UNBOUND_MODE_DISABLE,			\
 		},									\
 		.tx = {									\
 			.blocks_ptr = (uint8_t *)GET_BLOCKS_ADDR_INST(i, tx, rx),	\