diff --git a/drivers/rptun/rptun.c b/drivers/rptun/rptun.c index 0be24b5ab4..0110c96e8b 100644 --- a/drivers/rptun/rptun.c +++ b/drivers/rptun/rptun.c @@ -49,6 +49,7 @@ #endif #define RPTUNIOC_NONE 0 +#define NO_HOLDER (pid_t)-1 /**************************************************************************** * Private Types @@ -63,6 +64,7 @@ struct rptun_priv_s struct metal_list bind; struct metal_list node; sem_t sem; + int tid; unsigned long cmd; }; @@ -107,6 +109,7 @@ rptun_get_mem(FAR struct remoteproc *rproc, metal_phys_addr_t da, FAR void *va, size_t size, FAR struct remoteproc_mem *buf); +static int rptun_can_recursive(FAR struct remoteproc *rproc); static void rptun_ns_bind(FAR struct rpmsg_device *rdev, FAR const char *name, uint32_t dest); @@ -138,13 +141,14 @@ static metal_phys_addr_t rptun_da_to_pa(FAR struct rptun_dev_s *dev, static struct remoteproc_ops g_rptun_ops = { - .init = rptun_init, - .remove = rptun_remove, - .config = rptun_config, - .start = rptun_start, - .stop = rptun_stop, - .notify = rptun_notify, - .get_mem = rptun_get_mem, + .init = rptun_init, + .remove = rptun_remove, + .config = rptun_config, + .start = rptun_start, + .stop = rptun_stop, + .notify = rptun_notify, + .get_mem = rptun_get_mem, + .can_recursive = rptun_can_recursive, }; static const struct file_operations g_rptun_devops = @@ -162,7 +166,9 @@ static struct image_store_ops g_rptun_storeops = }; #endif -static sem_t g_rptun_sem = SEM_INITIALIZER(1); +static sem_t g_rptunlock = SEM_INITIALIZER(1); +static pid_t g_holder = NO_HOLDER; +static unsigned int g_count = 0; static METAL_DECLARE_LIST(g_rptun_cb); static METAL_DECLARE_LIST(g_rptun_priv); @@ -171,11 +177,65 @@ static METAL_DECLARE_LIST(g_rptun_priv); * Private Functions ****************************************************************************/ +static int rptun_lock(void) +{ + pid_t me = getpid(); + int ret = OK; + + /* Does this thread already hold the semaphore? */ + + if (g_holder == me) + { + /* Yes.. just increment the reference count */ + + g_count++; + } + else + { + /* No.. take the semaphore (perhaps waiting) */ + + ret = nxsem_wait_uninterruptible(&g_rptunlock); + if (ret >= 0) + { + /* Now this thread holds the semaphore */ + + g_holder = me; + g_count = 1; + } + } + + return ret; +} + +static void rptun_unlock(void) +{ + DEBUGASSERT(g_holder == getpid() && g_count > 0); + + /* If the count would go to zero, then release the semaphore */ + + if (g_count == 1) + { + /* We no longer hold the semaphore */ + + g_holder = NO_HOLDER; + g_count = 0; + nxsem_post(&g_rptunlock); + } + else + { + /* We still hold the semaphore. Just decrement the count */ + + g_count--; + } +} + static int rptun_thread(int argc, FAR char *argv[]) { FAR struct rptun_priv_s *priv; priv = (FAR struct rptun_priv_s *)((uintptr_t)strtoul(argv[2], NULL, 0)); + priv->tid = gettid(); + remoteproc_init(&priv->rproc, &g_rptun_ops, priv); while (1) @@ -321,6 +381,13 @@ rptun_get_mem(FAR struct remoteproc *rproc, return buf; } +static int rptun_can_recursive(FAR struct remoteproc *rproc) +{ + FAR struct rptun_priv_s *priv = rproc->priv; + + return gettid() == priv->tid; +} + static void *rptun_get_priv_by_rdev(FAR struct rpmsg_device *rdev) { struct rpmsg_virtio_device *rvdev; @@ -360,7 +427,7 @@ static void rptun_ns_bind(FAR struct rpmsg_device *rdev, bind->dest = dest; strncpy(bind->name, name, RPMSG_NAME_SIZE); - nxsem_wait(&g_rptun_sem); + rptun_lock(); metal_list_add_tail(&priv->bind, &bind->node); @@ -373,7 +440,7 @@ static void rptun_ns_bind(FAR struct rpmsg_device *rdev, } } - nxsem_post(&g_rptun_sem); + rptun_unlock(); } } @@ -383,7 +450,7 @@ static void rptun_ns_unbind(FAR struct rpmsg_device *rdev, FAR struct rptun_priv_s *priv = rptun_get_priv_by_rdev(rdev); FAR struct metal_list *node; - nxsem_wait(&g_rptun_sem); + rptun_lock(); metal_list_for_each(&priv->bind, node) { @@ -399,7 +466,7 @@ static void rptun_ns_unbind(FAR struct rpmsg_device *rdev, } } - nxsem_post(&g_rptun_sem); + rptun_unlock(); } static int rptun_dev_start(FAR struct remoteproc *rproc) @@ -529,7 +596,7 @@ static int rptun_dev_start(FAR struct remoteproc *rproc) return ret; } - nxsem_wait(&g_rptun_sem); + rptun_lock(); /* Add priv to list */ @@ -546,7 +613,7 @@ static int rptun_dev_start(FAR struct remoteproc *rproc) } } - nxsem_post(&g_rptun_sem); + rptun_unlock(); /* Register callback to mbox for receiving remote message */ @@ -565,7 +632,7 @@ static int rptun_dev_stop(FAR struct remoteproc *rproc) RPTUN_UNREGISTER_CALLBACK(priv->dev); - nxsem_wait(&g_rptun_sem); + rptun_lock(); /* Remove priv from list */ @@ -582,7 +649,7 @@ static int rptun_dev_stop(FAR struct remoteproc *rproc) } } - nxsem_post(&g_rptun_sem); + rptun_unlock(); /* Remote proc stop and shutdown */ @@ -765,7 +832,7 @@ int rpmsg_register_callback(FAR void *priv_, cb->device_destroy = device_destroy; cb->ns_bind = ns_bind; - nxsem_wait(&g_rptun_sem); + rptun_lock(); metal_list_add_tail(&g_rptun_cb, &cb->node); @@ -791,7 +858,7 @@ int rpmsg_register_callback(FAR void *priv_, } } - nxsem_post(&g_rptun_sem); + rptun_unlock(); return 0; } @@ -804,7 +871,7 @@ void rpmsg_unregister_callback(FAR void *priv_, FAR struct metal_list *node; FAR struct metal_list *pnode; - nxsem_wait(&g_rptun_sem); + rptun_lock(); metal_list_for_each(&g_rptun_cb, node) { @@ -835,7 +902,7 @@ void rpmsg_unregister_callback(FAR void *priv_, } } - nxsem_post(&g_rptun_sem); + rptun_unlock(); } int rptun_initialize(FAR struct rptun_dev_s *dev) diff --git a/openamp/0006-openamp-fix-scenario-case.patch b/openamp/0006-openamp-fix-scenario-case.patch new file mode 100644 index 0000000000..a9fa95f1bd --- /dev/null +++ b/openamp/0006-openamp-fix-scenario-case.patch @@ -0,0 +1,296 @@ +From d14f87ea5ff1371318acaf4539116b72986bae2c Mon Sep 17 00:00:00 2001 +From: ligd +Date: Thu, 18 Nov 2021 20:54:45 +0800 +Subject: [PATCH] openamp: fix scenario case + +description: +There are two CPUs use IPC. +Each cpu has one thread to handle RX callback. +When meet scenario case, both CPU0 & CPU1 can't get tx buffer. +IPC can't do communication any more. + +rootcase: +CPU0 CPU1 +TX thread0 send msg --> +RX thread get ack <-- RX thread get msg, send ack + <-- TX thread0 send msg +RX thread get msg, send ack --> RX thread get ack + +TX thread1 send msg --> +RX thread get ack <-- RX thread get msg, send ack + <-- TX thread1 send msg +RX thread get msg, send ack --> RX thread get ack + +TX thread2 send msg --> +TX thread3 send msg --> +.... + <-- TX thread2 send msg + <-- TX thread3 send msg + ... + +when CPU0 & CPU1 msg send too quickly, then CPU0 RX thread can't +get tx buffer(ack), wait CPU1 RX thread return buffer. But at this time, +CPU1 RX thread also can't get tx buffer(ack), wait CPU0 RX thread +return buffer. So, you will find two cpu locked. + +last call stack: + +CPU0 RX thread: CPU1 RX thread: +wait tx buffer wait tx buffer +can't handle rx buffer can't handle rx buffer + +Resolve: +add recursive call rpmsg_virtio_rx_callback() when get_tx_buffer() failed + +Signed-off-by: ligd +Change-Id: I60512c50327f180a0aba891e3ec847b211e172db +--- + lib/include/openamp/remoteproc.h | 10 +++ + lib/include/openamp/rpmsg_virtio.h | 6 ++ + lib/include/openamp/virtio.h | 1 + + lib/remoteproc/remoteproc_virtio.c | 12 +++ + lib/rpmsg/rpmsg_virtio.c | 134 +++++++++++++++-------------- + 5 files changed, 97 insertions(+), 66 deletions(-) + +diff --git a/lib/include/openamp/remoteproc.h open-amp/lib/include/openamp/remoteproc.h +index a71327b..222079e 100644 +--- a/lib/include/openamp/remoteproc.h ++++ open-amp/lib/include/openamp/remoteproc.h +@@ -449,6 +449,16 @@ struct remoteproc_ops { + metal_phys_addr_t da, + void *va, size_t size, + struct remoteproc_mem *buf); ++ /** ++ * can_recursive ++ * ++ * Check current vdev can do recursive rx_callabck or not ++ * ++ * @rproc - pointer to remoteproc instance ++ * ++ * return true if can do recursive rx_callabck. ++ */ ++ int (*can_recursive)(struct remoteproc *rproc); + }; + + /* Remoteproc error codes */ +diff --git a/lib/include/openamp/rpmsg_virtio.h open-amp/lib/include/openamp/rpmsg_virtio.h +index e552b96..fa556a2 100644 +--- a/lib/include/openamp/rpmsg_virtio.h ++++ open-amp/lib/include/openamp/rpmsg_virtio.h +@@ -114,6 +114,12 @@ rpmsg_virtio_create_virtqueues(struct rpmsg_virtio_device *rvdev, + callbacks); + } + ++static inline int rpmsg_virtio_can_recursive(struct rpmsg_virtio_device *rvdev) ++{ ++ return rvdev->vdev->func->can_recursive ? ++ rvdev->vdev->func->can_recursive(rvdev->vdev) : 0; ++} ++ + /** + * rpmsg_virtio_get_buffer_size - get rpmsg virtio buffer size + * +diff --git a/lib/include/openamp/virtio.h open-amp/lib/include/openamp/virtio.h +index 55c8ea5..ce808cc 100644 +--- a/lib/include/openamp/virtio.h ++++ open-amp/lib/include/openamp/virtio.h +@@ -137,6 +137,7 @@ struct virtio_dispatch { + void *src, int length); + void (*reset_device)(struct virtio_device *dev); + void (*notify)(struct virtqueue *vq); ++ int (*can_recursive)(struct virtio_device *dev); + }; + + int virtio_create_virtqueues(struct virtio_device *vdev, unsigned int flags, +diff --git a/lib/remoteproc/remoteproc_virtio.c open-amp/lib/remoteproc/remoteproc_virtio.c +index ed9f33c..937f294 100644 +--- a/lib/remoteproc/remoteproc_virtio.c ++++ open-amp/lib/remoteproc/remoteproc_virtio.c +@@ -30,6 +30,17 @@ static void rproc_virtio_virtqueue_notify(struct virtqueue *vq) + rpvdev->notify(rpvdev->priv, vring_info->notifyid); + } + ++static int rproc_virtio_can_recursive(struct virtio_device *vdev) ++{ ++ struct remoteproc_virtio *rpvdev; ++ struct remoteproc *rproc; ++ ++ rpvdev = metal_container_of(vdev, struct remoteproc_virtio, vdev); ++ rproc = rpvdev->priv; ++ ++ return rproc->ops->can_recursive ? rproc->ops->can_recursive(rproc) : 0; ++} ++ + static unsigned char rproc_virtio_get_status(struct virtio_device *vdev) + { + struct remoteproc_virtio *rpvdev; +@@ -179,6 +190,7 @@ static const struct virtio_dispatch remoteproc_virtio_dispatch_funcs = { + .get_features = rproc_virtio_get_features, + .read_config = rproc_virtio_read_config, + .notify = rproc_virtio_virtqueue_notify, ++ .can_recursive = rproc_virtio_can_recursive, + #ifndef VIRTIO_SLAVE_ONLY + /* + * We suppose here that the vdev is in a shared memory so that can +diff --git a/lib/rpmsg/rpmsg_virtio.c open-amp/lib/rpmsg/rpmsg_virtio.c +index 2687320..a407be9 100644 +--- a/lib/rpmsg/rpmsg_virtio.c ++++ open-amp/lib/rpmsg/rpmsg_virtio.c +@@ -286,6 +286,72 @@ static void rpmsg_virtio_release_rx_buffer(struct rpmsg_device *rdev, + metal_mutex_release(&rdev->lock); + } + ++/** ++ * rpmsg_virtio_rx_callback ++ * ++ * Rx callback function. ++ * ++ * @param vq - pointer to virtqueue on which messages is received ++ * ++ */ ++static void rpmsg_virtio_rx_callback(struct virtqueue *vq) ++{ ++ struct virtio_device *vdev = vq->vq_dev; ++ struct rpmsg_virtio_device *rvdev = vdev->priv; ++ struct rpmsg_device *rdev = &rvdev->rdev; ++ struct rpmsg_endpoint *ept; ++ struct rpmsg_hdr *rp_hdr; ++ uint32_t len; ++ uint16_t idx; ++ int status; ++ ++ metal_mutex_acquire(&rdev->lock); ++ ++ /* Process the received data from remote node */ ++ rp_hdr = rpmsg_virtio_get_rx_buffer(rvdev, &len, &idx); ++ ++ metal_mutex_release(&rdev->lock); ++ ++ while (rp_hdr) { ++ rp_hdr->reserved = idx; ++ ++ /* Get the channel node from the remote device channels list. */ ++ metal_mutex_acquire(&rdev->lock); ++ ept = rpmsg_get_ept_from_addr(rdev, rp_hdr->dst); ++ metal_mutex_release(&rdev->lock); ++ ++ if (ept) { ++ if (ept->dest_addr == RPMSG_ADDR_ANY) { ++ /* ++ * First message received from the remote side, ++ * update channel destination address ++ */ ++ ept->dest_addr = rp_hdr->src; ++ } ++ status = ept->cb(ept, RPMSG_LOCATE_DATA(rp_hdr), ++ rp_hdr->len, rp_hdr->src, ept->priv); ++ ++ RPMSG_ASSERT(status >= 0, ++ "unexpected callback status\r\n"); ++ } ++ ++ metal_mutex_acquire(&rdev->lock); ++ ++ /* Check whether callback wants to hold buffer */ ++ if (!(rp_hdr->reserved & RPMSG_BUF_HELD)) { ++ /* No, return used buffers. */ ++ rpmsg_virtio_return_buffer(rvdev, rp_hdr, len, idx); ++ } ++ ++ rp_hdr = rpmsg_virtio_get_rx_buffer(rvdev, &len, &idx); ++ if (!rp_hdr) { ++ /* tell peer we return some rx buffer */ ++ virtqueue_kick(rvdev->rvq); ++ } ++ metal_mutex_release(&rdev->lock); ++ } ++} ++ + static void *rpmsg_virtio_get_tx_payload_buffer(struct rpmsg_device *rdev, + uint32_t *len, int wait) + { +@@ -315,6 +381,8 @@ static void *rpmsg_virtio_get_tx_payload_buffer(struct rpmsg_device *rdev, + metal_mutex_release(&rdev->lock); + if (rp_hdr || !tick_count) + break; ++ if (rpmsg_virtio_can_recursive(rvdev)) ++ rpmsg_virtio_rx_callback(rvdev->rvq); + metal_sleep_usec(RPMSG_TICKS_PER_INTERVAL); + tick_count--; + } +@@ -440,72 +508,6 @@ static void rpmsg_virtio_tx_callback(struct virtqueue *vq) + (void)vq; + } + +-/** +- * rpmsg_virtio_rx_callback +- * +- * Rx callback function. +- * +- * @param vq - pointer to virtqueue on which messages is received +- * +- */ +-static void rpmsg_virtio_rx_callback(struct virtqueue *vq) +-{ +- struct virtio_device *vdev = vq->vq_dev; +- struct rpmsg_virtio_device *rvdev = vdev->priv; +- struct rpmsg_device *rdev = &rvdev->rdev; +- struct rpmsg_endpoint *ept; +- struct rpmsg_hdr *rp_hdr; +- uint32_t len; +- uint16_t idx; +- int status; +- +- metal_mutex_acquire(&rdev->lock); +- +- /* Process the received data from remote node */ +- rp_hdr = rpmsg_virtio_get_rx_buffer(rvdev, &len, &idx); +- +- metal_mutex_release(&rdev->lock); +- +- while (rp_hdr) { +- rp_hdr->reserved = idx; +- +- /* Get the channel node from the remote device channels list. */ +- metal_mutex_acquire(&rdev->lock); +- ept = rpmsg_get_ept_from_addr(rdev, rp_hdr->dst); +- metal_mutex_release(&rdev->lock); +- +- if (ept) { +- if (ept->dest_addr == RPMSG_ADDR_ANY) { +- /* +- * First message received from the remote side, +- * update channel destination address +- */ +- ept->dest_addr = rp_hdr->src; +- } +- status = ept->cb(ept, RPMSG_LOCATE_DATA(rp_hdr), +- rp_hdr->len, rp_hdr->src, ept->priv); +- +- RPMSG_ASSERT(status >= 0, +- "unexpected callback status\r\n"); +- } +- +- metal_mutex_acquire(&rdev->lock); +- +- /* Check whether callback wants to hold buffer */ +- if (!(rp_hdr->reserved & RPMSG_BUF_HELD)) { +- /* No, return used buffers. */ +- rpmsg_virtio_return_buffer(rvdev, rp_hdr, len, idx); +- } +- +- rp_hdr = rpmsg_virtio_get_rx_buffer(rvdev, &len, &idx); +- if (!rp_hdr) { +- /* tell peer we return some rx buffer */ +- virtqueue_kick(rvdev->rvq); +- } +- metal_mutex_release(&rdev->lock); +- } +-} +- + /** + * rpmsg_virtio_ns_callback + * +-- +2.25.1 + diff --git a/openamp/open-amp.defs b/openamp/open-amp.defs index 558bff680e..cf0dd8d28f 100644 --- a/openamp/open-amp.defs +++ b/openamp/open-amp.defs @@ -38,6 +38,7 @@ open-amp.zip: $(Q) patch -p0 < 0003-rpmsg-wait-endpoint-ready-in-rpmsg_send-and-rpmsg_se.patch $(Q) patch -p0 < 0004-openamp-add-ns_unbind_notify-support.patch $(Q) patch -p0 < 0005-rpmsg-notify-the-user-when-the-remote-address-is-rec.patch + $(Q) patch -p0 < 0006-openamp-fix-scenario-case.patch .openamp_headers: open-amp.zip $(eval headers := $(wildcard open-amp/lib/include/openamp/*.h))