ipc: ipc_service: Add support for multiple instances

The IPC service code is currently assuming that only one IPC instance
does exist and the user can use the IPC service API to interface with
that singleton instance.

This is a huge limitation and this patch is trying to fix this
assumption introducing three major changes to the IPC service API:

- All the IPC instances are now supposed to be instantiated as a struct
  device. A new test is introduced to be used as skeleton for all the
  other backends.

- ipc_service_register_backend() is now removed (because multiple
  backends are now supported at the same time).

- All the other ipc_service_*() functions are now taking a struct device
  pointer as parameter to specify on which instance the user is going to
  act and operate.

In this patch the documentation is also extended to better clarify the
terminology used.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>

include/ipc/ipc_service.h

@@ -4,10 +4,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-#ifndef ZEPHYR_INCLUDE_IPC_SERVICE_IPC_SERVICE_H_
-#define ZEPHYR_INCLUDE_IPC_SERVICE_IPC_SERVICE_H_
+#ifndef ZEPHYR_INCLUDE_IPC_IPC_SERVICE_H_
+#define ZEPHYR_INCLUDE_IPC_IPC_SERVICE_H_
 
 #include <stdio.h>
+#include <device.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -17,6 +18,44 @@ extern "C" {
  * @brief IPC Service API
  * @defgroup ipc_service_api IPC service APIs
  * @{
+ *
+ * Some terminology:
+ *
+ * - INSTANCE: an instance is the external representation of a physical
+ *             communication channel between two domains / CPUs.
+ *
+ *             The actual implementation and internal representation of the
+ *             instance is peculiar to each backend. For example for
+ *             OpenAMP-based backends, an instance is usually represented by a
+ *             shared memory region and a couple of IPM devices for RX/TX
+ *             signalling.
+ *
+ *             It's important to note that an instance per se is not used to
+ *             send data between domains / CPUs. To send and receive data the
+ *             user have to create (register) an endpoint in the instance
+ *             connecting the two domains of interest.
+ *
+ *             It's possible to have zero or multiple endpoints in one single
+ *             instance, each one used to exchange data, possibly with different
+ *             priorities.
+ *
+ *             The creation of the instances is left to the backend (usually at
+ *             init time), while the registration of the endpoints is left to
+ *             the user (usually at run time).
+ *
+ * - ENDPOINT: an endpoint is the entity the user must use to send / receive
+ *             data between two domains (connected by the instance). An
+ *             endpoint is always associated to an instance.
+ *
+ * - BACKEND: the backend must take care of at least two different things:
+ *
+ *            1) creating the instances at init time
+ *            2) creating / registering the endpoints onto an instance at run
+ *               time when requested by the user
+ *
+ *            The API doesn't mandate a way for the backend to create the
+ *            instances but itis strongly recommended to use the DT to retrieve
+ *            the configuration parameters for the instance.
  */
 
 /** @brief Event callback structure.
@@ -49,10 +88,17 @@ struct ipc_service_cb {
 
 /** @brief Endpoint instance.
  *
- *  Content is not important for user of the API.
- *  It is implemented in a specific backend.
+ *  Token is not important for user of the API. It is implemented in a
+ *  specific backend.
  */
-struct ipc_ept;
+struct ipc_ept {
+
+	/** Instance this endpoint belongs to. */
+	const struct device *instance;
+
+	/** Backend-specific token used to identify an endpoint in an instance. */
+	void *token;
+};
 
 /** @brief Endpoint configuration structure. */
 struct ipc_ept_cfg {
@@ -70,20 +116,24 @@ struct ipc_ept_cfg {
 	void *priv;
 };
 
-/** @brief Register IPC endpoint.
+/** @brief Register IPC endpoint onto an instance.
  *
- *  Registers IPC endpoint to enable communication with a remote device.
+ *  Registers IPC endpoint onto an instance to enable communication with a
+ *  remote device.
  *
  *  The same function registers endpoints for both master and slave devices.
  *
+ *  @param instance Instance to register the endpoint onto.
  *  @param ept Endpoint object.
  *  @param cfg Endpoint configuration.
  *
  *  @retval -EIO when no backend is registered.
- *  @retval -EINVAL when pointer to an endpoint or endpoint configuration is invalid.
+ *  @retval -EINVAL when instance or endpoint configuration is invalid.
  *  @retval Other errno codes depending on the implementation of the backend.
  */
-int ipc_service_register_endpoint(struct ipc_ept **ept, const struct ipc_ept_cfg *cfg);
+int ipc_service_register_endpoint(const struct device *instance,
+				  struct ipc_ept *ept,
+				  const struct ipc_ept_cfg *cfg);
 
 /** @brief Send data using given IPC endpoint.
  *
@@ -104,4 +154,4 @@ int ipc_service_send(struct ipc_ept *ept, const void *data, size_t len);
 }
 #endif
 
-#endif /* ZEPHYR_INCLUDE_IPC_SERVICE_IPC_SERVICE_H_ */
+#endif /* ZEPHYR_INCLUDE_IPC_IPC_SERVICE_H_ */

include/ipc/ipc_service_backend.h

@@ -25,42 +25,31 @@ extern "C" {
  *  This structure is used for configuration backend during registration.
  */
 struct ipc_service_backend {
-	/** @brief Name of the IPC backend. */
-	const char *name;
-
 	/** @brief Pointer to the function that will be used to send data to the endpoint.
 	 *
-	 *  @param ept Registered endpoint.
+	 *  @param instance Instance pointer.
+	 *  @param token Backend-specific token.
 	 *  @param data Pointer to the buffer to send.
 	 *  @param len Number of bytes to send.
 	 *
 	 *  @retval Status code.
 	 */
-	int (*send)(struct ipc_ept *ept, const void *data, size_t len);
+	int (*send)(const struct device *instance, void *token,
+		    const void *data, size_t len);
 
 	/** @brief Pointer to the function that will be used to register endpoints.
 	 *
-	 *  @param ept Endpoint object.
+	 *  @param instance Instance to register the endpoint onto.
+	 *  @param token Backend-specific token.
 	 *  @param cfg Endpoint configuration.
 	 *
 	 *  @retval Status code.
 	 */
-	int (*register_endpoint)(struct ipc_ept **ept, const struct ipc_ept_cfg *cfg);
+	int (*register_endpoint)(const struct device *instance,
+				 void **token,
+				 const struct ipc_ept_cfg *cfg);
 };
 
-/** @brief IPC backend registration.
- *
- *  Registration must be done before using IPC Service.
- *
- *  @param backend Configuration of the backend.
- *
- *  @retval -EALREADY The backend is already registered.
- *  @retval -EINVAL The backend configuration is incorrect.
- *  @retval Zero on success.
- *
- */
-int ipc_service_register_backend(const struct ipc_service_backend *backend);
-
 /**
  * @}
  */

subsys/ipc/ipc_service/backends/Kconfig

@@ -11,6 +11,10 @@ config IPC_SERVICE_BACKEND_RPMSG_MI
 	select OPENAMP
 	select IPM
 
+config IPC_SERVICE_BACKEND_ZTEST
+	depends on ZTEST
+	bool "IPC service backend test"
+
 endchoice
 
 if IPC_SERVICE_BACKEND_RPMSG_MI

subsys/ipc/ipc_service/ipc_service.c

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2021 Nordic Semiconductor ASA
+ * Copyright (c) 2021 Carlo Caione <ccaione@baylibre.com>
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -13,46 +14,41 @@
 
 LOG_MODULE_REGISTER(ipc_service, CONFIG_IPC_SERVICE_LOG_LEVEL);
 
-const static struct ipc_service_backend *backend;
-
-int ipc_service_register_backend(const struct ipc_service_backend *bkd)
+int ipc_service_register_endpoint(const struct device *instance,
+				  struct ipc_ept *ept,
+				  const struct ipc_ept_cfg *cfg)
 {
-	if (backend) {
-		return -EALREADY;
-	}
+	const struct ipc_service_backend *backend;
 
-	if (!bkd || !bkd->register_endpoint || !bkd->send) {
+	if (!instance || !ept || !cfg) {
+		LOG_ERR("Invalid instance, endpoint or configuration");
 		return -EINVAL;
 	}
 
-	backend = bkd;
-	LOG_DBG("Registered: %s", backend->name ? backend->name : "");
+	backend = (const struct ipc_service_backend *) instance->api;
 
-	return 0;
-}
-
-int ipc_service_register_endpoint(struct ipc_ept **ept, const struct ipc_ept_cfg *cfg)
-{
-	LOG_DBG("Register endpoint %s", cfg->name ? cfg->name : "");
 	if (!backend || !backend->register_endpoint) {
-		LOG_ERR("Backend not registered");
+		LOG_ERR("Invalid backend configuration");
 		return -EIO;
 	}
 
-	if (!ept || !cfg) {
-		LOG_ERR("Invalid endpoint or configuration");
-		return -EINVAL;
-	}
+	LOG_DBG("Register endpoint %s", cfg->name ? cfg->name : "");
 
-	return backend->register_endpoint(ept, cfg);
+	ept->instance = instance;
+
+	return backend->register_endpoint(instance, &ept->token, cfg);
 }
 
 int ipc_service_send(struct ipc_ept *ept, const void *data, size_t len)
 {
+	const struct ipc_service_backend *backend;
+
+	backend = ept->instance->api;
+
 	if (!backend || !backend->send) {
-		LOG_ERR("Backend not registered");
+		LOG_ERR("Invalid backend configuration");
 		return -EIO;
 	}
 
-	return backend->send(ept, data, len);
+	return backend->send(ept->instance, ept->token, data, len);
 }

tests/subsys/ipc/ipc_service/CMakeLists.txt

@@ -0,0 +1,10 @@
+# Copyright 2021 Google LLC
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.20.0)
+
+find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
+project(ipc_service)
+
+FILE(GLOB app_sources src/*.c)
+target_sources(app PRIVATE ${app_sources})

tests/subsys/ipc/ipc_service/boards/qemu_cortex_a53.overlay

@@ -0,0 +1,17 @@
+/* Copyright 2021 Carlo Caione <ccaione@baylibre.com>
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/ {
+	ipc10: ipc10 {
+		compatible = "ipc-service-backend";
+		offset = <10>;
+		status = "okay";
+	};
+
+	ipc20: ipc20 {
+		compatible = "ipc-service-backend";
+		offset = <20>;
+		status = "okay";
+	};
+};

tests/subsys/ipc/ipc_service/dts/bindings/ipc-service-backend.yaml

@@ -0,0 +1,15 @@
+#
+# Copyright (c) 2021 Carlo Caione <ccaione@baylibre.com>
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+
+description: Backend binding to test the IPC service
+
+compatible: "ipc-service-backend"
+
+properties:
+  offset:
+    type: int
+    required: true
+    description: offset to add

tests/subsys/ipc/ipc_service/prj.conf

@@ -0,0 +1,7 @@
+# Copyright 2021 Carlo Caione <ccaione@baylibre.com>
+# SPDX-License-Identifier: Apache-2.0
+
+CONFIG_ZTEST=y
+CONFIG_MMU=y
+CONFIG_IPC_SERVICE=y
+CONFIG_IPC_SERVICE_BACKEND_ZTEST=y

tests/subsys/ipc/ipc_service/src/backend.c

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2021, Carlo Caione <ccaione@baylibre.com>
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Simple backend that adds an offset (defined into the DT) to whatever it is
+ * passed in input as IPC message.
+ */
+
+#include <ipc/ipc_service_backend.h>
+
+#include <logging/log.h>
+#include <sys/util_macro.h>
+#include <zephyr.h>
+#include <device.h>
+
+#define DT_DRV_COMPAT		ipc_service_backend
+
+struct backend_data_t {
+	const struct ipc_ept_cfg *cfg;
+};
+
+struct backend_config_t {
+	unsigned int offset;
+};
+
+static int send(const struct device *instance, void *token,
+		const void *p_data, size_t len)
+{
+	const struct backend_config_t *config;
+	struct backend_data_t *data;
+	uint8_t *msg;
+
+	config = instance->config;
+	data = instance->data;
+	msg = (uint8_t *) p_data;
+
+	*msg += config->offset;
+
+	data->cfg->cb.received(msg, sizeof(*msg), data->cfg->priv);
+
+	return 0;
+}
+
+static int register_ept(const struct device *instance,
+			void **token,
+			const struct ipc_ept_cfg *cfg)
+{
+	struct backend_data_t *data = instance->data;
+
+	data->cfg = cfg;
+
+	return 0;
+}
+
+const static struct ipc_service_backend backend_ops = {
+	.send = send,
+	.register_endpoint = register_ept,
+};
+
+static int backend_init(const struct device *dev)
+{
+	ARG_UNUSED(dev);
+
+	/* Nothing to do */
+
+	return 0;
+}
+
+#define DEFINE_BACKEND_DEVICE(i)					\
+	static struct backend_config_t backend_config_##i = {		\
+		.offset = DT_INST_PROP(i, offset),			\
+	};								\
+									\
+	static struct backend_data_t backend_data_##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)

tests/subsys/ipc/ipc_service/src/main.c

@@ -0,0 +1,77 @@
+/* Copyright 2021 Carlo Caione, <ccaione@baylibre.com>
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <device.h>
+#include <devicetree.h>
+#include <ztest.h>
+#include <ipc/ipc_service.h>
+
+static void received_cb(const void *data, size_t len, void *priv)
+{
+	uintptr_t expected = (uintptr_t) priv;
+	uint8_t *msg = (uint8_t *) data;
+
+	zassert_equal(*msg, expected, "msg doesn't match the expected value");
+
+	printk("Received: %d, expected: %ld\n", *msg, expected);
+}
+
+static struct ipc_ept_cfg ept_cfg = {
+	.name = "test_ept",
+	.cb = {
+		.received = received_cb,
+	},
+};
+
+void test_ipc_service(void)
+{
+	const struct device *dev_10;
+	const struct device *dev_20;
+	struct ipc_ept ept_10;
+	struct ipc_ept ept_20;
+	uint8_t msg;
+	int ret;
+
+	dev_10 = DEVICE_DT_GET(DT_NODELABEL(ipc10));
+	dev_20 = DEVICE_DT_GET(DT_NODELABEL(ipc20));
+
+	/*
+	 * We send 10 through the ipc10 instance so we expect 20 in the
+	 * receiving callback (10 + 10 == 20)
+	 */
+	msg = 10;
+	printk("Sending %d\n", msg);
+
+	/* Save the expected result in priv */
+	ept_cfg.priv = (void *) 20;
+
+	ret = ipc_service_register_endpoint(dev_10, &ept_10, &ept_cfg);
+	zassert_ok(ret, "ipc_service_register_endpoint() failed", NULL);
+
+	ret = ipc_service_send(&ept_10, &msg, sizeof(msg));
+	zassert_ok(ret, "ipc_service_send() failed", NULL);
+
+	/*
+	 * We send 10 again this time through the ipc20 instance so we expect
+	 * 20 in the receiving callback (10 + 20 == 30)
+	 */
+	msg = 10;
+	printk("Sending %d\n", msg);
+
+	/* Save the expected result in priv */
+	ept_cfg.priv = (void *) 30;
+
+	ret = ipc_service_register_endpoint(dev_20, &ept_20, &ept_cfg);
+	zassert_ok(ret, "ipc_service_register_endpoint() failed", NULL);
+
+	ret = ipc_service_send(&ept_20, &msg, sizeof(msg));
+	zassert_ok(ret, "ipc_service_send() failed", NULL);
+}
+
+void test_main(void)
+{
+	ztest_test_suite(ipc_service,
+			 ztest_unit_test(test_ipc_service));
+	ztest_run_test_suite(ipc_service);
+}

tests/subsys/ipc/ipc_service/testcase.yaml

@@ -0,0 +1,8 @@
+# Copyright 2021 Carlo Caione 2021 <ccaione@baylibre.com>
+# SPDX-License-Identifier: Apache-2.0
+
+tests:
+  ipc.ipc_service:
+    tags: ipc_service
+    harness: ztest
+    platform_allow: qemu_cortex_a53