diff --git a/dts/bindings/ipc/zephyr,ipc-icmsg.yaml b/dts/bindings/ipc/zephyr,ipc-icmsg.yaml index b67c9072980fc0e..7bc9dc7f06e491d 100644 --- a/dts/bindings/ipc/zephyr,ipc-icmsg.yaml +++ b/dts/bindings/ipc/zephyr,ipc-icmsg.yaml @@ -21,6 +21,10 @@ properties: required: true type: phandle + cache-line-sz: + description: Size of data cache line. Should be same as for remote. + type: int + mboxes: description: phandle to the MBOX controller (TX and RX are required) required: true diff --git a/include/zephyr/ipc/icmsg.h b/include/zephyr/ipc/icmsg.h index 099c6265acd5272..5bd03b791d221d8 100644 --- a/include/zephyr/ipc/icmsg.h +++ b/include/zephyr/ipc/icmsg.h @@ -12,8 +12,8 @@ #include #include #include +#include #include -#include #ifdef __cplusplus extern "C" { @@ -33,19 +33,16 @@ enum icmsg_state { }; struct icmsg_config_t { - uintptr_t tx_shm_addr; - uintptr_t rx_shm_addr; - size_t tx_shm_size; - size_t rx_shm_size; struct mbox_channel mbox_tx; struct mbox_channel mbox_rx; + struct pbuf_cfg tx_pb; + struct pbuf_cfg rx_pb; }; struct icmsg_data_t { /* Tx/Rx buffers. */ - struct spsc_pbuf *tx_ib; - struct spsc_pbuf *rx_ib; - atomic_t tx_buffer_state; + struct pbuf *tx_pb; + struct pbuf *rx_pb; #ifdef CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC struct k_mutex tx_lock; #endif @@ -59,12 +56,6 @@ struct icmsg_data_t { struct k_work_delayable notify_work; struct k_work mbox_work; atomic_t state; - /* No-copy */ -#ifdef CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX - atomic_t rx_buffer_state; - const void *rx_buffer; - uint16_t rx_len; -#endif }; /** @brief Open an icmsg instance diff --git a/subsys/ipc/ipc_service/backends/ipc_icmsg.c b/subsys/ipc/ipc_service/backends/ipc_icmsg.c index eca84f3275c34da..89ed78f16e185b8 100644 --- a/subsys/ipc/ipc_service/backends/ipc_icmsg.c +++ b/subsys/ipc/ipc_service/backends/ipc_icmsg.c @@ -56,17 +56,29 @@ static int backend_init(const struct device *instance) #define DEFINE_BACKEND_DEVICE(i) \ static const struct icmsg_config_t backend_config_##i = { \ - .tx_shm_size = DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \ - .tx_shm_addr = DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \ - .rx_shm_size = DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \ - .rx_shm_addr = DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \ .mbox_tx = MBOX_DT_CHANNEL_GET(DT_DRV_INST(i), tx), \ .mbox_rx = MBOX_DT_CHANNEL_GET(DT_DRV_INST(i), rx), \ + .tx_pb = PBUF_CFG_INIT( \ + DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \ + DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \ + DT_INST_PROP_OR(i, cache_line_sz, 0)), \ + .rx_pb = PBUF_CFG_INIT( \ + DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \ + DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \ + DT_INST_PROP_OR(i, cache_line_sz, 0)), \ }; \ \ - BUILD_ASSERT(DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)) > \ - sizeof(struct spsc_pbuf)); \ - static struct icmsg_data_t backend_data_##i; \ + static struct pbuf tx_pb_##i = { \ + .cfg = &backend_config_##i.tx_pb, \ + }; \ + static struct pbuf rx_pb_##i = { \ + .cfg = &backend_config_##i.rx_pb, \ + }; \ + \ + 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, \ diff --git a/subsys/ipc/ipc_service/backends/ipc_icmsg_me_follower.c b/subsys/ipc/ipc_service/backends/ipc_icmsg_me_follower.c index 8b0944c840f9b8c..ba4525043ec5779 100644 --- a/subsys/ipc/ipc_service/backends/ipc_icmsg_me_follower.c +++ b/subsys/ipc/ipc_service/backends/ipc_icmsg_me_follower.c @@ -277,16 +277,34 @@ static int backend_init(const struct device *instance) } #define DEFINE_BACKEND_DEVICE(i) \ - static const struct icmsg_config_t backend_config_##i = \ - { \ - .tx_shm_size = DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \ - .tx_shm_addr = DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \ - .rx_shm_size = DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \ - .rx_shm_addr = DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \ + static const struct icmsg_config_t backend_config_##i = { \ .mbox_tx = MBOX_DT_CHANNEL_GET(DT_DRV_INST(i), tx), \ .mbox_rx = MBOX_DT_CHANNEL_GET(DT_DRV_INST(i), rx), \ + .tx_pb = PBUF_CFG_INIT( \ + DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \ + DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \ + DT_INST_PROP_OR(i, cache_line_sz, 0)), \ + .rx_pb = PBUF_CFG_INIT( \ + DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \ + DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \ + DT_INST_PROP_OR(i, cache_line_sz, 0)), \ + }; \ + \ + static struct pbuf tx_pb_##i = { \ + .cfg = &backend_config_##i.tx_pb, \ + }; \ + static struct pbuf rx_pb_##i = { \ + .cfg = &backend_config_##i.rx_pb, \ + }; \ + \ + static struct backend_data_t backend_data_##i = { \ + .icmsg_me_data = { \ + .icmsg_data = { \ + .tx_pb = &tx_pb_##i, \ + .rx_pb = &rx_pb_##i, \ + } \ + } \ }; \ - static struct backend_data_t backend_data_##i; \ \ DEVICE_DT_INST_DEFINE(i, \ &backend_init, \ diff --git a/subsys/ipc/ipc_service/backends/ipc_icmsg_me_initiator.c b/subsys/ipc/ipc_service/backends/ipc_icmsg_me_initiator.c index 2699d3b6db66feb..da6996f801e1d69 100644 --- a/subsys/ipc/ipc_service/backends/ipc_icmsg_me_initiator.c +++ b/subsys/ipc/ipc_service/backends/ipc_icmsg_me_initiator.c @@ -183,16 +183,34 @@ static int backend_init(const struct device *instance) } #define DEFINE_BACKEND_DEVICE(i) \ - static const struct icmsg_config_t backend_config_##i = \ - { \ - .tx_shm_size = DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \ - .tx_shm_addr = DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \ - .rx_shm_size = DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \ - .rx_shm_addr = DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \ + static const struct icmsg_config_t backend_config_##i = { \ .mbox_tx = MBOX_DT_CHANNEL_GET(DT_DRV_INST(i), tx), \ .mbox_rx = MBOX_DT_CHANNEL_GET(DT_DRV_INST(i), rx), \ + .tx_pb = PBUF_CFG_INIT( \ + DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \ + DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \ + DT_INST_PROP_OR(i, cache_line_sz, 0)), \ + .rx_pb = PBUF_CFG_INIT( \ + DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \ + DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \ + DT_INST_PROP_OR(i, cache_line_sz, 0)), \ + }; \ + \ + static struct pbuf tx_pb_##i = { \ + .cfg = &backend_config_##i.tx_pb, \ + }; \ + static struct pbuf rx_pb_##i = { \ + .cfg = &backend_config_##i.rx_pb, \ + }; \ + \ + static struct backend_data_t backend_data_##i = { \ + .icmsg_me_data = { \ + .icmsg_data = { \ + .tx_pb = &tx_pb_##i, \ + .rx_pb = &rx_pb_##i, \ + } \ + } \ }; \ - static struct backend_data_t backend_data_##i; \ \ DEVICE_DT_INST_DEFINE(i, \ &backend_init, \ diff --git a/subsys/ipc/ipc_service/lib/Kconfig b/subsys/ipc/ipc_service/lib/Kconfig index 6bd57ab15aa67a3..0fbc82ddc13f14e 100644 --- a/subsys/ipc/ipc_service/lib/Kconfig +++ b/subsys/ipc/ipc_service/lib/Kconfig @@ -21,8 +21,7 @@ config IPC_SERVICE_STATIC_VRINGS_MEM_ALIGNMENT menuconfig IPC_SERVICE_ICMSG bool "icmsg IPC library" - select SPSC_PBUF - select SPSC_PBUF_USE_CACHE + select PBUF help Icmsg library diff --git a/subsys/ipc/ipc_service/lib/icmsg.c b/subsys/ipc/ipc_service/lib/icmsg.c index 9d238f193514e74..5d93686eed94b78 100644 --- a/subsys/ipc/ipc_service/lib/icmsg.c +++ b/subsys/ipc/ipc_service/lib/icmsg.c @@ -9,22 +9,12 @@ #include #include #include -#include +#include #include #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) -enum rx_buffer_state { - RX_BUFFER_STATE_RELEASED, - RX_BUFFER_STATE_RELEASING, - RX_BUFFER_STATE_HELD -}; - -enum tx_buffer_state { - TX_BUFFER_STATE_UNUSED, - TX_BUFFER_STATE_RESERVED -}; static const uint8_t magic[] = {0x45, 0x6d, 0x31, 0x6c, 0x31, 0x4b, 0x30, 0x72, 0x6e, 0x33, 0x6c, 0x69, 0x34}; @@ -82,12 +72,6 @@ static bool is_endpoint_ready(struct icmsg_data_t *dev_data) return atomic_get(&dev_data->state) == ICMSG_STATE_READY; } -static bool is_tx_buffer_reserved(struct icmsg_data_t *dev_data) -{ - return atomic_get(&dev_data->tx_buffer_state) == - TX_BUFFER_STATE_RESERVED; -} - static int reserve_tx_buffer_if_unused(struct icmsg_data_t *dev_data) { #ifdef CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC @@ -97,52 +81,20 @@ static int reserve_tx_buffer_if_unused(struct icmsg_data_t *dev_data) return ret; } #endif - - bool was_unused = atomic_cas(&dev_data->tx_buffer_state, - TX_BUFFER_STATE_UNUSED, TX_BUFFER_STATE_RESERVED); - - return was_unused ? 0 : -EALREADY; + return 0; } static int release_tx_buffer(struct icmsg_data_t *dev_data) { - bool was_reserved = atomic_cas(&dev_data->tx_buffer_state, - TX_BUFFER_STATE_RESERVED, TX_BUFFER_STATE_UNUSED); - - if (!was_reserved) { - return -EALREADY; - } - #ifdef CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_SYNC return k_mutex_unlock(&dev_data->tx_lock); -#else - return 0; -#endif -} - -static bool is_rx_buffer_free(struct icmsg_data_t *dev_data) -{ -#ifdef CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX - return atomic_get(&dev_data->rx_buffer_state) == RX_BUFFER_STATE_RELEASED; -#else - return true; -#endif -} - -static bool is_rx_buffer_held(struct icmsg_data_t *dev_data) -{ -#ifdef CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX - return atomic_get(&dev_data->rx_buffer_state) == RX_BUFFER_STATE_HELD; -#else - return false; #endif + return 0; } -static bool is_rx_data_available(struct icmsg_data_t *dev_data) +static uint32_t data_available(struct icmsg_data_t *dev_data) { - int len = spsc_pbuf_read(dev_data->rx_ib, NULL, 0); - - return len > 0; + return pbuf_read(dev_data->rx_pb, NULL, 0); } static void submit_mbox_work(struct icmsg_data_t *dev_data) @@ -157,20 +109,13 @@ static void submit_mbox_work(struct icmsg_data_t *dev_data) static void submit_work_if_buffer_free(struct icmsg_data_t *dev_data) { - if (!is_rx_buffer_free(dev_data)) { - return; - } - submit_mbox_work(dev_data); } static void submit_work_if_buffer_free_and_data_available( struct icmsg_data_t *dev_data) { - if (!is_rx_buffer_free(dev_data)) { - return; - } - if (!is_rx_data_available(dev_data)) { + if (!data_available(dev_data)) { return; } @@ -179,47 +124,31 @@ static void submit_work_if_buffer_free_and_data_available( static void mbox_callback_process(struct k_work *item) { - char *rx_buffer; struct icmsg_data_t *dev_data = CONTAINER_OF(item, struct icmsg_data_t, mbox_work); atomic_t state = atomic_get(&dev_data->state); - uint16_t len = spsc_pbuf_claim(dev_data->rx_ib, &rx_buffer); + uint32_t len = data_available(dev_data); if (len == 0) { /* Unlikely, no data in buffer. */ return; } + uint8_t rx_buffer[len]; + + len = pbuf_read(dev_data->rx_pb, rx_buffer, len); + if (state == ICMSG_STATE_READY) { if (dev_data->cb->received) { -#if CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX - dev_data->rx_buffer = rx_buffer; - dev_data->rx_len = len; -#endif - dev_data->cb->received(rx_buffer, len, dev_data->ctx); - - /* Release Rx buffer here only in case when user did not request - * to hold it. - */ - if (!is_rx_buffer_held(dev_data)) { - spsc_pbuf_free(dev_data->rx_ib, len); - -#if CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX - dev_data->rx_buffer = NULL; - dev_data->rx_len = 0; -#endif - } } } else { __ASSERT_NO_MSG(state == ICMSG_STATE_BUSY); bool endpoint_invalid = (len != sizeof(magic) || memcmp(magic, rx_buffer, len)); - spsc_pbuf_free(dev_data->rx_ib, len); - if (endpoint_invalid) { __ASSERT_NO_MSG(false); return; @@ -262,8 +191,6 @@ int icmsg_open(const struct icmsg_config_t *conf, struct icmsg_data_t *dev_data, const struct ipc_service_cb *cb, void *ctx) { - __ASSERT_NO_MSG(conf->tx_shm_size > sizeof(struct spsc_pbuf)); - if (!atomic_cas(&dev_data->state, ICMSG_STATE_OFF, ICMSG_STATE_BUSY)) { /* Already opened. */ return -EALREADY; @@ -277,12 +204,18 @@ int icmsg_open(const struct icmsg_config_t *conf, k_mutex_init(&dev_data->tx_lock); #endif - dev_data->tx_ib = spsc_pbuf_init((void *)conf->tx_shm_addr, - conf->tx_shm_size, - SPSC_PBUF_CACHE); - dev_data->rx_ib = (void *)conf->rx_shm_addr; + int ret = pbuf_init(dev_data->tx_pb); - int ret = spsc_pbuf_write(dev_data->tx_ib, magic, sizeof(magic)); + if (ret < 0) { + __ASSERT(false, "Incorrect configuration"); + return ret; + } + + /* Initialize local copies of rx_pb. */ + dev_data->rx_pb->data.wr_idx = 0; + dev_data->rx_pb->data.rd_idx = 0; + + ret = pbuf_write(dev_data->tx_pb, magic, sizeof(magic)); if (ret < 0) { __ASSERT_NO_MSG(false); @@ -345,7 +278,7 @@ int icmsg_send(const struct icmsg_config_t *conf, return -ENOBUFS; } - write_ret = spsc_pbuf_write(dev_data->tx_ib, msg, len); + write_ret = pbuf_write(dev_data->tx_pb, msg, len); release_ret = release_tx_buffer(dev_data); __ASSERT_NO_MSG(!release_ret); @@ -366,169 +299,6 @@ int icmsg_send(const struct icmsg_config_t *conf, return sent_bytes; } -int icmsg_get_tx_buffer(const struct icmsg_config_t *conf, - struct icmsg_data_t *dev_data, - void **data, size_t *size) -{ - int ret; - int release_ret; - uint16_t requested_size; - int allocated_len; - char *allocated_buf; - - if (*size == 0) { - /* Requested allocation of maximal size. - * Try to allocate maximal buffer size from spsc, - * potentially after wrapping marker. - */ - requested_size = SPSC_PBUF_MAX_LEN - 1; - } else { - requested_size = *size; - } - - ret = reserve_tx_buffer_if_unused(dev_data); - if (ret < 0) { - return -ENOBUFS; - } - - ret = spsc_pbuf_alloc(dev_data->tx_ib, requested_size, &allocated_buf); - if (ret < 0) { - release_ret = release_tx_buffer(dev_data); - __ASSERT_NO_MSG(!release_ret); - return ret; - } - allocated_len = ret; - - if (*size == 0) { - /* Requested allocation of maximal size. - * Pass the buffer that was allocated. - */ - *size = allocated_len; - *data = allocated_buf; - return 0; - } - - if (*size == allocated_len) { - /* Allocated buffer is of requested size. */ - *data = allocated_buf; - return 0; - } - - /* Allocated smaller buffer than requested. - * Silently stop using the allocated buffer what is allowed by SPSC API - */ - release_tx_buffer(dev_data); - *size = allocated_len; - return -ENOMEM; -} - -int icmsg_drop_tx_buffer(const struct icmsg_config_t *conf, - struct icmsg_data_t *dev_data, - const void *data) -{ - /* Silently stop using the allocated buffer what is allowed by SPSC API - */ - return release_tx_buffer(dev_data); -} - -int icmsg_send_nocopy(const struct icmsg_config_t *conf, - struct icmsg_data_t *dev_data, - const void *msg, size_t len) -{ - int ret; - int sent_bytes; - - if (!is_endpoint_ready(dev_data)) { - return -EBUSY; - } - - /* Empty message is not allowed */ - if (len == 0) { - return -ENODATA; - } - - if (!is_tx_buffer_reserved(dev_data)) { - return -ENXIO; - } - - spsc_pbuf_commit(dev_data->tx_ib, len); - sent_bytes = len; - - ret = release_tx_buffer(dev_data); - __ASSERT_NO_MSG(!ret); - - __ASSERT_NO_MSG(conf->mbox_tx.dev != NULL); - - ret = mbox_send(&conf->mbox_tx, NULL); - if (ret) { - return ret; - } - - return sent_bytes; -} - -#ifdef CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX -int icmsg_hold_rx_buffer(const struct icmsg_config_t *conf, - struct icmsg_data_t *dev_data, - const void *data) -{ - bool was_released; - - if (!is_endpoint_ready(dev_data)) { - return -EBUSY; - } - - if (data != dev_data->rx_buffer) { - return -EINVAL; - } - - was_released = atomic_cas(&dev_data->rx_buffer_state, - RX_BUFFER_STATE_RELEASED, RX_BUFFER_STATE_HELD); - if (!was_released) { - return -EALREADY; - } - - return 0; -} - -int icmsg_release_rx_buffer(const struct icmsg_config_t *conf, - struct icmsg_data_t *dev_data, - const void *data) -{ - bool was_held; - - if (!is_endpoint_ready(dev_data)) { - return -EBUSY; - } - - if (data != dev_data->rx_buffer) { - return -EINVAL; - } - - /* Do not schedule new packet processing until buffer will be released. - * Protect buffer against being freed multiple times. - */ - was_held = atomic_cas(&dev_data->rx_buffer_state, - RX_BUFFER_STATE_HELD, RX_BUFFER_STATE_RELEASING); - if (!was_held) { - return -EALREADY; - } - - spsc_pbuf_free(dev_data->rx_ib, dev_data->rx_len); - -#if CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX - dev_data->rx_buffer = NULL; - dev_data->rx_len = 0; -#endif - - atomic_set(&dev_data->rx_buffer_state, RX_BUFFER_STATE_RELEASED); - - submit_work_if_buffer_free_and_data_available(dev_data); - - return 0; -} -#endif /* CONFIG_IPC_SERVICE_ICMSG_NOCOPY_RX */ - #if IS_ENABLED(CONFIG_IPC_SERVICE_BACKEND_ICMSG_WQ_ENABLE) static int work_q_init(void)