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

[Stefan Roese]

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?

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");