[PATCH 0/6] Miscellaneous fixes for the Host1x driver

[PATCH 0/6] Miscellaneous fixes for the Host1x driver

[Mikko Perttunen]

This series has a number of small miscellaneous fixes to the host1x 
driver.

Patches 1 to 4 fix various logic issues that are unlikely to happen
but technically possible.

Patch 5 fixes a return type from unsigned int to int -- no functional
difference.

Patch 6 adds makes syncpoint value arithmetic explicitly wrapping, 
mostly to help static/dynamic analysis tools.

---
Mikko Perttunen (6):
      gpu: host1x: Wait for timeout worker completion on channel free
      gpu: host1x: Avoid double device_add when clients already present
      gpu: host1x: Fix offset calculation in trace_write_gather
      gpu: host1x: Avoid stack over-read in debug output helpers
      gpu: host1x: Change pin_job() return type to int
      gpu: host1x: Annotate intentional syncpoint wrap-around

 drivers/gpu/host1x/bus.c           |  2 +-
 drivers/gpu/host1x/cdma.c          |  3 ++-
 drivers/gpu/host1x/debug.c         |  4 ++--
 drivers/gpu/host1x/hw/cdma_hw.c    |  2 +-
 drivers/gpu/host1x/hw/channel_hw.c | 15 +++++++++------
 drivers/gpu/host1x/intr.c          |  5 +++--
 drivers/gpu/host1x/job.c           |  2 +-
 drivers/gpu/host1x/syncpt.c        |  7 ++++++-
 drivers/gpu/host1x/syncpt.h        |  3 ++-
 9 files changed, 27 insertions(+), 16 deletions(-)
---
base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
change-id: 20260608-b4-host1x-small-fixes-a-081cfea2c073

[PATCH 1/6] gpu: host1x: Wait for timeout worker completion on channel free

[Mikko Perttunen]

cdma_timeout_destroy() used cancel_delayed_work() to cancel pending
timeout work when destroying the CDMA. Usually this is fine, but
there is a narrow race condition where the timeout handler has started
execution but has not taken cdma->lock; the channel is freed causing
cdma_stop to take cdma->lock and flush the channel; host1x_cdma_deinit
then proceeds with deinitializing cdma while the handler is waiting to
take cdma->lock.

Therefore change cdma_timeout_destroy to use cancel_delayed_work_sync
instead to ensure any pending timeout work completes before proceeding.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/host1x/hw/cdma_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/hw/cdma_hw.c b/drivers/gpu/host1x/hw/cdma_hw.c
index 3f3f0018eee0..ab714d221120 100644
--- a/drivers/gpu/host1x/hw/cdma_hw.c
+++ b/drivers/gpu/host1x/hw/cdma_hw.c
@@ -355,7 +355,7 @@ static int cdma_timeout_init(struct host1x_cdma *cdma)
 static void cdma_timeout_destroy(struct host1x_cdma *cdma)
 {
 	if (cdma->timeout.initialized)
-		cancel_delayed_work(&cdma->timeout.wq);
+		cancel_delayed_work_sync(&cdma->timeout.wq);
 
 	cdma->timeout.initialized = false;
 }

-- 
2.53.0

[PATCH 2/6] gpu: host1x: Avoid double device_add when clients already present

[Mikko Perttunen]

host1x_device_add looks through the idle clients list to populate
subdevs, and any matches entries are moved from the subdevs list
to the active list. If all subdevs are populated, device_add will
be called on the device. The secondary "subdevs list empty" check
will then incorrectly again call device_add.

However, this would require a convoluted scenario since clients don't
typically end up on the idle clients list.

Fix by checking whether the device was already added before adding
again.

Fixes: fab823d82ee5 ("gpu: host1x: Allow loading tegra-drm without enabled engines")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/host1x/bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index f814eb4941c0..fe4af98e8fc6 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -508,7 +508,7 @@ static int host1x_device_add(struct host1x *host1x,
 	 * Add device even if there are no subdevs to ensure syncpoint functionality
 	 * is available regardless of whether any engine subdevices are present
 	 */
-	if (list_empty(&device->subdevs)) {
+	if (list_empty(&device->subdevs) && !device->registered) {
 		err = device_add(&device->dev);
 		if (err < 0)
 			dev_err(&device->dev, "failed to add device: %d\n", err);

-- 
2.53.0

[PATCH 3/6] gpu: host1x: Fix offset calculation in trace_write_gather

[Mikko Perttunen]

When a gather longer than 2*TRACE_MAX_LENGTH (256) words is traced
through host1x_cdma_push_gather, the reported BO offset drifts from
the third iteration onward.

Fix the calculation by properly calculating the value on each loop
rather than accumulating.

In reality, gathers tend to be pretty short so this is unlikely to
ever have been observed.

Fixes: b40d02bf96e0 ("gpu: host1x: Use struct host1x_bo pointers in traces")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/host1x/hw/channel_hw.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
index 2df6a16d484e..9dda73199889 100644
--- a/drivers/gpu/host1x/hw/channel_hw.c
+++ b/drivers/gpu/host1x/hw/channel_hw.c
@@ -36,10 +36,9 @@ static void trace_write_gather(struct host1x_cdma *cdma, struct host1x_bo *bo,
 		for (i = 0; i < words; i += TRACE_MAX_LENGTH) {
 			u32 num_words = min(words - i, TRACE_MAX_LENGTH);
 
-			offset += i * sizeof(u32);
-
 			trace_host1x_cdma_push_gather(dev_name(dev), bo,
-						      num_words, offset,
+						      num_words,
+						      offset + i * sizeof(u32),
 						      mem);
 		}
 

-- 
2.53.0

[PATCH 4/6] gpu: host1x: Avoid stack over-read in debug output helpers

[Mikko Perttunen]

host1x_debug_output() and host1x_debug_cont() used vsnprintf(), which
returns the length the formatted string would have reached with an
unbounded buffer. That return value was passed straight to o->fn as
the number of bytes to emit.

This could cause a read past end of the output buffer if a call to
host1x_debug_* produced a string longer than 256 bytes. This only
affected the debugfs files as the printk debug sink ignores the
number of bytes. In practice, this is very unlikely to occur.

Fix by switching to vscnprintf(), which returns the number of bytes
actually written.

Fixes: 6236451d83a7 ("gpu: host1x: Add debug support")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/host1x/debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/host1x/debug.c b/drivers/gpu/host1x/debug.c
index 6433c00d5d7e..b828f773fc06 100644
--- a/drivers/gpu/host1x/debug.c
+++ b/drivers/gpu/host1x/debug.c
@@ -31,7 +31,7 @@ void host1x_debug_output(struct output *o, const char *fmt, ...)
 	int len;
 
 	va_start(args, fmt);
-	len = vsnprintf(o->buf, sizeof(o->buf), fmt, args);
+	len = vscnprintf(o->buf, sizeof(o->buf), fmt, args);
 	va_end(args);
 
 	o->fn(o->ctx, o->buf, len, false);
@@ -43,7 +43,7 @@ void host1x_debug_cont(struct output *o, const char *fmt, ...)
 	int len;
 
 	va_start(args, fmt);
-	len = vsnprintf(o->buf, sizeof(o->buf), fmt, args);
+	len = vscnprintf(o->buf, sizeof(o->buf), fmt, args);
 	va_end(args);
 
 	o->fn(o->ctx, o->buf, len, true);

-- 
2.53.0

[PATCH 5/6] gpu: host1x: Change pin_job() return type to int

[Mikko Perttunen]

pin_job() returns negative errno values on error paths (-EINVAL,
-ENOMEM, PTR_ERR() of mapping) but was declared as unsigned int.
The caller would immediately cast back to int, so there was no
functional issue, but it still warrants fixing.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/host1x/job.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 3ed49e1fd933..165979319599 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -138,7 +138,7 @@ void host1x_job_add_wait(struct host1x_job *job, u32 id, u32 thresh,
 }
 EXPORT_SYMBOL(host1x_job_add_wait);
 
-static unsigned int pin_job(struct host1x *host, struct host1x_job *job)
+static int pin_job(struct host1x *host, struct host1x_job *job)
 {
 	unsigned long mask = HOST1X_RELOC_READ | HOST1X_RELOC_WRITE;
 	struct host1x_client *client = job->client;

-- 
2.53.0

[PATCH 6/6] gpu: host1x: Annotate intentional syncpoint wrap-around

[Mikko Perttunen]

Host1x syncpoints are 32-bit counters that roll over by design.
To make that explicit in the code, use wrapping_* functions whenever
arithmetic is done on syncpoint values.

Atomic operations cannot be updated but a comment is added.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/host1x/cdma.c          |  3 ++-
 drivers/gpu/host1x/hw/channel_hw.c | 10 +++++++---
 drivers/gpu/host1x/intr.c          |  5 +++--
 drivers/gpu/host1x/syncpt.c        |  7 ++++++-
 drivers/gpu/host1x/syncpt.h        |  3 ++-
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
index ba2e572567c0..f6d3db2c8c39 100644
--- a/drivers/gpu/host1x/cdma.c
+++ b/drivers/gpu/host1x/cdma.c
@@ -13,6 +13,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/kfifo.h>
+#include <linux/overflow.h>
 #include <linux/slab.h>
 #include <trace/events/host1x.h>
 
@@ -419,7 +420,7 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma *cdma,
 		/* won't need a timeout when replayed */
 		job->timeout = 0;
 
-		syncpt_incrs = job->syncpt_end - syncpt_val;
+		syncpt_incrs = wrapping_sub(u32, job->syncpt_end, syncpt_val);
 		dev_dbg(dev, "%s: CPU incr (%d)\n", __func__, syncpt_incrs);
 
 		host1x_job_dump(dev, job);
diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
index 9dda73199889..a8251ec0810c 100644
--- a/drivers/gpu/host1x/hw/channel_hw.c
+++ b/drivers/gpu/host1x/hw/channel_hw.c
@@ -7,6 +7,7 @@
 
 #include <linux/host1x.h>
 #include <linux/iommu.h>
+#include <linux/overflow.h>
 #include <linux/slab.h>
 
 #include <trace/events/host1x.h>
@@ -120,7 +121,8 @@ static void submit_gathers(struct host1x_job *job, struct host1x_job_cmd *cmds,
 
 		if (cmd->is_wait) {
 			if (cmd->wait.relative)
-				threshold = job_syncpt_base + cmd->wait.threshold;
+				threshold = wrapping_add(u32, job_syncpt_base,
+							 cmd->wait.threshold);
 			else
 				threshold = cmd->wait.threshold;
 
@@ -259,7 +261,8 @@ static void channel_program_cdma(struct host1x_job *job)
 
 	/* Submit work. */
 	job->syncpt_end = host1x_syncpt_incr_max(sp, job->syncpt_incrs);
-	submit_gathers(job, job->cmds + i, job->num_cmds - i, job->syncpt_end - job->syncpt_incrs);
+	submit_gathers(job, job->cmds + i, job->num_cmds - i,
+		       wrapping_sub(u32, job->syncpt_end, job->syncpt_incrs));
 
 	/* Before releasing MLOCK, ensure engine is idle again. */
 	fence = host1x_syncpt_incr_max(sp, 1);
@@ -297,7 +300,8 @@ static void channel_program_cdma(struct host1x_job *job)
 
 	job->syncpt_end = host1x_syncpt_incr_max(sp, job->syncpt_incrs);
 
-	submit_gathers(job, job->cmds, job->num_cmds, job->syncpt_end - job->syncpt_incrs);
+	submit_gathers(job, job->cmds, job->num_cmds,
+		       wrapping_sub(u32, job->syncpt_end, job->syncpt_incrs));
 #endif
 }
 
diff --git a/drivers/gpu/host1x/intr.c b/drivers/gpu/host1x/intr.c
index f77a678949e9..f9fd8a471e60 100644
--- a/drivers/gpu/host1x/intr.c
+++ b/drivers/gpu/host1x/intr.c
@@ -7,6 +7,7 @@
 
 #include <linux/clk.h>
 #include <linux/interrupt.h>
+#include <linux/overflow.h>
 #include "dev.h"
 #include "fence.h"
 #include "intr.h"
@@ -17,7 +18,7 @@ static void host1x_intr_add_fence_to_list(struct host1x_fence_list *list,
 	struct host1x_syncpt_fence *fence_in_list;
 
 	list_for_each_entry_reverse(fence_in_list, &list->list, list) {
-		if ((s32)(fence_in_list->threshold - fence->threshold) <= 0) {
+		if ((s32)wrapping_sub(u32, fence_in_list->threshold, fence->threshold) <= 0) {
 			/* Fence in list is before us, we can insert here */
 			list_add(&fence->list, &fence_in_list->list);
 			return;
@@ -83,7 +84,7 @@ void host1x_intr_handle_interrupt(struct host1x *host, unsigned int id)
 	spin_lock(&sp->fences.lock);
 
 	list_for_each_entry_safe(fence, tmp, &sp->fences.list, list) {
-		if (((value - fence->threshold) & 0x80000000U) != 0U) {
+		if ((wrapping_sub(u32, value, fence->threshold) & 0x80000000U) != 0U) {
 			/* Fence is not yet expired, we are done */
 			break;
 		}
diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
index acc7d82e0585..9ac4f0c80728 100644
--- a/drivers/gpu/host1x/syncpt.c
+++ b/drivers/gpu/host1x/syncpt.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/dma-fence.h>
+#include <linux/overflow.h>
 #include <linux/slab.h>
 
 #include <trace/events/host1x.h>
@@ -126,6 +127,10 @@ EXPORT_SYMBOL(host1x_syncpt_id);
  */
 u32 host1x_syncpt_incr_max(struct host1x_syncpt *sp, u32 incrs)
 {
+	/*
+	 * Syncpoint values are intended to be modulo 2^32, so overflow
+	 * here is intended.
+	 */
 	return (u32)atomic_add_return(incrs, &sp->max_val);
 }
 EXPORT_SYMBOL(host1x_syncpt_incr_max);
@@ -274,7 +279,7 @@ bool host1x_syncpt_is_expired(struct host1x_syncpt *sp, u32 thresh)
 
 	current_val = (u32)atomic_read(&sp->min_val);
 
-	return ((current_val - thresh) & 0x80000000U) == 0U;
+	return (wrapping_sub(u32, current_val, thresh) & 0x80000000U) == 0U;
 }
 
 int host1x_syncpt_init(struct host1x *host)
diff --git a/drivers/gpu/host1x/syncpt.h b/drivers/gpu/host1x/syncpt.h
index 4c3f3b2f0e9c..9eff42efc445 100644
--- a/drivers/gpu/host1x/syncpt.h
+++ b/drivers/gpu/host1x/syncpt.h
@@ -12,6 +12,7 @@
 #include <linux/host1x.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
+#include <linux/overflow.h>
 #include <linux/sched.h>
 
 #include "fence.h"
@@ -77,7 +78,7 @@ static inline bool host1x_syncpt_check_max(struct host1x_syncpt *sp, u32 real)
 	if (sp->client_managed)
 		return true;
 	max = host1x_syncpt_read_max(sp);
-	return (s32)(max - real) >= 0;
+	return (s32)wrapping_sub(u32, max, real) >= 0;
 }
 
 /* Return true if sync point is client managed. */

-- 
2.53.0

Re: [PATCH 0/6] Miscellaneous fixes for the Host1x driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

On Tue, Jun 09, 2026 at 05:09:16PM +0900, Mikko Perttunen wrote:
> This series has a number of small miscellaneous fixes to the host1x 
> driver.
> 
> Patches 1 to 4 fix various logic issues that are unlikely to happen
> but technically possible.
> 
> Patch 5 fixes a return type from unsigned int to int -- no functional
> difference.
> 
> Patch 6 adds makes syncpoint value arithmetic explicitly wrapping, 
> mostly to help static/dynamic analysis tools.
> 
> ---
> Mikko Perttunen (6):
>       gpu: host1x: Wait for timeout worker completion on channel free
>       gpu: host1x: Avoid double device_add when clients already present
>       gpu: host1x: Fix offset calculation in trace_write_gather
>       gpu: host1x: Avoid stack over-read in debug output helpers
>       gpu: host1x: Change pin_job() return type to int
>       gpu: host1x: Annotate intentional syncpoint wrap-around
> 
>  drivers/gpu/host1x/bus.c           |  2 +-
>  drivers/gpu/host1x/cdma.c          |  3 ++-
>  drivers/gpu/host1x/debug.c         |  4 ++--
>  drivers/gpu/host1x/hw/cdma_hw.c    |  2 +-
>  drivers/gpu/host1x/hw/channel_hw.c | 15 +++++++++------
>  drivers/gpu/host1x/intr.c          |  5 +++--
>  drivers/gpu/host1x/job.c           |  2 +-
>  drivers/gpu/host1x/syncpt.c        |  7 ++++++-
>  drivers/gpu/host1x/syncpt.h        |  3 ++-
>  9 files changed, 27 insertions(+), 16 deletions(-)
> ---
> base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
> change-id: 20260608-b4-host1x-small-fixes-a-081cfea2c073

Applied to drm-misc-next, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]