Re: [v3 PATCH] remoteproc: xlnx: Use high-prio workqueue instead of system wq

[Stefan Roese]

Hi Tanmay,

On 12/10/25 03:51, Zhongqiu Han wrote:

On 12/5/2025 8:06 PM, Stefan Roese wrote:

Hi Tanmay,

On 12/4/25 17:45, Tanmay Shah wrote:

Hello,

Thank You for your patch. Please find my comments below.

On 12/4/25 4:40 AM, Stefan Roese wrote:

Testing on our ZynqMP platform has shown, that some R5 messages might
get dropped under high CPU load. This patch creates a new high-prio

Here, I would like to understand what it means by "R5 messages might get dropped"

Even under high CPU load, the messages from R5 are stored in the virtqueues. If Linux doesn't read it, then it is not really lost/ dropped.

Could you please explain your use case in detail and how the testing is conducted?

Our use-case is, that we send ~4k messages per second from the R5 to
Linux - sometimes even a bit more. Normally these messages are received
okay and no messages are dropped. Sometimes, under "high CPU load"
scenarios it happens, that the R5 has to drop messages, as there is no
free space in the RPMsg buffer, which is 256 entries AFAIU. Resulting
from the Linux driver not emptying the RX queue.

Could you please elaborate on these virtqueues a bit? Especially why no
messages drop should happen because of these virtqueues?

AFAIK, as a transport layer based on virtqueue, rpmsg is reliable once a
message has been successfully enqueued. The observed "drop" here appears
to be on the R5 side, where the application discards messages when no
entry buffer is available.

Correct.

In the long run, while improving the Linux side is recommended,

Yes, please.

it could
also be helpful for the R5 side to implement strategies such as an
application-level buffer and retry mechanisms.

We already did this. We've added an additional buffer mechanism to the
R5, which improved this "message drop situation" a bit. Still it did not
fix it for all our high message rate situations - still resulting in
frame drops on the R5 side (the R5 is a bit resource restricted).

Improving the responsiveness on the Linux side seems to be the best way
for us to deal with this problem.

Thanks,
Stefan

Thanks,
Stefan

Thanks,
Tanmay

workqueue which is now used instead of the default system workqueue.
With this change we don't experience these message drops any more.

Signed-off-by: Stefan Roese <stefan.ro...@mailbox.org>
Cc: Tanmay Shah <tanmay.s...@amd.com>
Cc: Mathieu Poirier <mathieu.poir...@linaro.org>
---
v3:

- Call cancel_work_sync() before freeing ipi (suggested by Zhongqiu Han)

v2:

- Also call destroy_workqueue() in zynqmp_r5_cluster_exit() (suggested by Zhongqiu Han)

- Correct call seq to avoid UAF (suggested by Zhongqiu Han)

  drivers/remoteproc/xlnx_r5_remoteproc.c | 23 ++++++++++++++++++++++-
  1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/ remoteproc/xlnx_r5_remoteproc.c

index feca6de68da28..308328b0b489f 100644
--- a/drivers/remoteproc/xlnx_r5_remoteproc.c
+++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
@@ -16,6 +16,7 @@
  #include <linux/of_reserved_mem.h>
  #include <linux/platform_device.h>
  #include <linux/remoteproc.h>
+#include <linux/workqueue.h>
  #include "remoteproc_internal.h"
@@ -116,6 +117,7 @@ struct zynqmp_r5_cluster {
      enum  zynqmp_r5_cluster_mode mode;
      int core_count;
      struct zynqmp_r5_core **r5_cores;
+    struct workqueue_struct *workqueue;
  };
  /**

@@ -174,10 +176,18 @@ static void handle_event_notified(struct work_struct *work)

  static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg)
  {
      struct zynqmp_ipi_message *ipi_msg, *buf_msg;
+    struct zynqmp_r5_cluster *cluster;
      struct mbox_info *ipi;
+    struct device *dev;
      size_t len;
      ipi = container_of(cl, struct mbox_info, mbox_cl);
+    dev = ipi->r5_core->dev;
+    cluster = dev_get_drvdata(dev->parent);
+    if (!cluster) {
+        dev_err(dev->parent, "Invalid driver data\n");
+        return;
+    }
      /* copy data from ipi buffer to r5_core */
      ipi_msg = (struct zynqmp_ipi_message *)msg;

@@ -195,7 +205,7 @@ static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg)

      if (mbox_send_message(ipi->rx_chan, NULL) < 0)
          dev_err(cl->dev, "ack failed to mbox rx_chan\n");
-    schedule_work(&ipi->mbox_work);
+    queue_work(cluster->workqueue, &ipi->mbox_work);
  }
  /**
@@ -1154,6 +1164,7 @@ static void zynqmp_r5_cluster_exit(void *data)
      for (i = 0; i < cluster->core_count; i++) {
          r5_core = cluster->r5_cores[i];
+        cancel_work_sync(&r5_core->ipi->mbox_work);
          zynqmp_r5_free_mbox(r5_core->ipi);
          of_reserved_mem_device_release(r5_core->dev);
          put_device(r5_core->dev);
@@ -1162,6 +1173,7 @@ static void zynqmp_r5_cluster_exit(void *data)
      }
      kfree(cluster->r5_cores);
+    destroy_workqueue(cluster->workqueue);
      kfree(cluster);
      platform_set_drvdata(pdev, NULL);
  }

@@ -1194,11 +1206,20 @@ static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)

          return ret;
      }
+    cluster->workqueue = alloc_workqueue(dev_name(dev),
+                         WQ_UNBOUND | WQ_HIGHPRI, 0);
+    if (!cluster->workqueue) {
+        dev_err_probe(dev, -ENOMEM, "cannot create workqueue\n");
+        kfree(cluster);
+        return -ENOMEM;
+    }
+
      /* wire in so each core can be cleaned up at driver remove */
      platform_set_drvdata(pdev, cluster);
      ret = zynqmp_r5_cluster_init(cluster);
      if (ret) {
+        destroy_workqueue(cluster->workqueue);
          kfree(cluster);
          platform_set_drvdata(pdev, NULL);

          dev_err_probe(dev, ret, "Invalid r5f subsystem device tree\n");