rptun: support can_recursive check for openamp

Signed-off-by: ligd <liguiding1@xiaomi.com>
This commit is contained in:
ligd 2021-11-23 20:12:55 +08:00 committed by Xiang Xiao
parent 5b696559d9
commit f5ddbb317f
3 changed files with 384 additions and 20 deletions

View File

@ -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)

View File

@ -0,0 +1,296 @@
From d14f87ea5ff1371318acaf4539116b72986bae2c Mon Sep 17 00:00:00 2001
From: ligd <liguiding1@xiaomi.com>
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 <liguiding1@xiaomi.com>
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

View File

@ -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))