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?
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 <[email protected]>
Cc: Tanmay Shah <[email protected]>
Cc: Mathieu Poirier <[email protected]>
---
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");
