casaroli commented on code in PR #21381:
URL: https://github.com/apache/nuttx/pull/21381#discussion_r4473601334
##########
drivers/rpmsg/rpmsg_virtio.c:
##########
@@ -596,23 +696,24 @@ static int rpmsg_virtio_start(FAR struct
rpmsg_virtio_priv_s *priv)
return ret;
}
- priv->cbrx = priv->rvdev.rvq->callback;
- priv->cbtx = priv->rvdev.svq->callback;
priv->notifytx = priv->rvdev.svq->notify;
- priv->rvdev.rvq->callback = rpmsg_virtio_rx_callback;
- priv->rvdev.svq->callback = rpmsg_virtio_tx_callback;
priv->rvdev.svq->notify = rpmsg_virtio_tx_notify;
priv->rvdev.notify_wait_cb = rpmsg_virtio_notify_wait;
priv->rvdev.rdev.ns_unbind_cb = rpmsg_ns_unbind;
/* Wake up the rx thread to process message */
+ rpmsg_virtio_update_rx(priv, priv->rvdev.rvq);
rpmsg_virtio_wakeup_rx(priv);
/* Broadcast device_created to all registers */
rpmsg_device_created(&priv->rpmsg);
+ /* Open tx buffer return callback */
+
+ virtqueue_enable_cb(priv->rvdev.svq);
Review Comment:
The tx callback stays off during `rpmsg_device_created()` at line 711, and
master's enable around the wait in `rpmsg_virtio_notify_wait()` is gone. So a
tx buffer wait there takes the full 20 ms, and with `RPMSG_VIRTIO_PM` and no
autorelax the wakelock of `rpmsg_virtio_tx_notify()` stays until a later tx
completes.
Fix: call `virtqueue_enable_cb()` before `rpmsg_device_created()`.
##########
drivers/rpmsg/rpmsg_virtio.c:
##########
@@ -252,10 +251,83 @@ static bool rpmsg_virtio_is_recursive(FAR struct
rpmsg_virtio_priv_s *priv)
static void rpmsg_virtio_rx_worker(FAR struct rpmsg_virtio_priv_s *priv)
{
- if (rpmsg_virtio_available_rx(priv))
+ FAR struct virtio_device *vdev = priv->vdev;
+ FAR struct rpmsg_virtio_device *rvdev = vdev->priv;
+ FAR struct rpmsg_device *rdev = &rvdev->rdev;
+ FAR struct rpmsg_endpoint *ept;
+ FAR struct rpmsg_hdr *rp_hdr;
+ bool release = false;
+ bool last = false;
+ uint32_t len;
+ uint16_t idx;
+ int status;
+
+ if (!rpmsg_virtio_available_rx(priv))
{
- priv->cbrx(priv->rvdev.rvq);
+ return;
}
+
+ metal_mutex_acquire(&rdev->lock);
+ while (!last)
+ {
+ /* Process the received data from remote node */
+
+ rp_hdr = rpmsg_virtio_get_rx_buffer(rvdev, &len, &idx, &last);
+ if (!rp_hdr)
+ {
+ break;
+ }
+
+ rp_hdr->reserved = idx;
+
+ /* Get the channel node from the remote device channels list. */
+
+ ept = rpmsg_get_ept_from_addr(rdev, rp_hdr->dst);
Review Comment:
This copy of open-amp's rx callback does not have the length test that patch
0020 added to it, so a message whose `rp_hdr->len` does not fit its buffer now
reaches `ept->cb`; on master it is dropped with no callback. `len` from line
275 is never read.
Fix: `ept = len >= sizeof(*rp_hdr) && rp_hdr->len <= len - sizeof(*rp_hdr) ?
rpmsg_get_ept_from_addr(rdev, rp_hdr->dst) : NULL;`, as in
`rpmsg_virtio_rx_callback()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
