ipc_service: icmsg: Add "unbound" functionality

In some cases, CPUs that may need to reset or
temporary stop communication. This commit adds "unbound"
functionality that provides a callback to IPC service user
when connection was interrupted for some reason, e.g.
expected or unexpected CPU reset, closing the
endpoint. The "unbound" callback is optional to implement
by endpoints. This commit implements it in the ICMsg
backend.

Signed-off-by: Dominik Kilian <[email protected]>

Author: doki-nordic

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,
 	},
 };
 

subsys/ipc/ipc_service/backends/ipc_icbmsg.c

@@ -1402,11 +1402,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,					\
@@ -1418,6 +1418,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),	\

subsys/ipc/ipc_service/backends/ipc_icmsg.c

@@ -54,33 +54,52 @@ static int backend_init(const struct device *instance)
 	return 0;
 }
 
-#define DEFINE_BACKEND_DEVICE(i)					\
-	static const struct icmsg_config_t backend_config_##i = {	\
-		.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx),		\
-		.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx),		\
-	};								\
-									\
-	PBUF_DEFINE(tx_pb_##i,						\
-			DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)),	\
-			DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)),	\
-			DT_INST_PROP_OR(i, dcache_alignment, 0));	\
-	PBUF_DEFINE(rx_pb_##i,						\
-			DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)),	\
-			DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)),	\
-			DT_INST_PROP_OR(i, dcache_alignment, 0));	\
-									\
-	static struct icmsg_data_t backend_data_##i = {			\
-		.tx_pb = &tx_pb_##i,					\
-		.rx_pb = &rx_pb_##i,					\
-	};								\
-									\
-	DEVICE_DT_INST_DEFINE(i,					\
-			 &backend_init,					\
-			 NULL,						\
-			 &backend_data_##i,				\
-			 &backend_config_##i,				\
-			 POST_KERNEL,					\
-			 CONFIG_IPC_SERVICE_REG_BACKEND_PRIORITY,	\
+#define UNBOUND_MODE(i) CONCAT(ICMSG_UNBOUND_MODE_, DT_INST_STRING_UPPER_TOKEN(i, unbound))
+
+#define DEFINE_BACKEND_DEVICE(i)						\
+	static const struct icmsg_config_t backend_config_##i = {		\
+		.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx),			\
+		.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx),			\
+		.unbound_mode = UNBOUND_MODE(i),				\
+	};									\
+										\
+	PBUF_DEFINE(tx_pb_##i,							\
+			DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)),		\
+			DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)),		\
+			DT_INST_PROP_OR(i, dcache_alignment, 0),		\
+			UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE,		\
+			UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT);		\
+	PBUF_DEFINE(rx_pb_##i,							\
+			DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)),		\
+			DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)),		\
+			DT_INST_PROP_OR(i, dcache_alignment, 0),		\
+			UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE,		\
+			UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT);		\
+										\
+	BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE ||		\
+		IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DISABLED_ALLOWED),	\
+		"Unbound mode \"disabled\" is was forbidden in Kconfig.");	\
+										\
+	BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_ENABLE ||		\
+		IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_ENABLED_ALLOWED),	\
+		"Unbound mode \"enabled\" is was forbidden in Kconfig.");	\
+										\
+	BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DETECT ||		\
+		IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DETECT_ALLOWED),	\
+		"Unbound mode \"detect\" is was forbidden in Kconfig.");	\
+										\
+	static struct icmsg_data_t backend_data_##i = {				\
+		.tx_pb = &tx_pb_##i,						\
+		.rx_pb = &rx_pb_##i,						\
+	};									\
+										\
+	DEVICE_DT_INST_DEFINE(i,						\
+			 &backend_init,						\
+			 NULL,							\
+			 &backend_data_##i,					\
+			 &backend_config_##i,					\
+			 POST_KERNEL,						\
+			 CONFIG_IPC_SERVICE_REG_BACKEND_PRIORITY,		\
 			 &backend_ops);
 
 DT_INST_FOREACH_STATUS_OKAY(DEFINE_BACKEND_DEVICE)