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