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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Direct `<` comparison of wrap-around syncpoint values leads to incorrect evaluation of job completion status during timeout recovery.
--

commit 00ebce8e032b66ba3bfbcd7a17200301254bdb05
Author: Mikko Perttunen <mperttunen@nvidia.com>

gpu: host1x: Annotate intentional syncpoint wrap-around

Host1x syncpoints are 32-bit counters that roll over by design. This commit
uses wrapping_* functions whenever arithmetic is done on syncpoint values to
make this explicit in the code.

> diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> index ba2e572567c03..f6d3db2c8c395 100644
> --- a/drivers/gpu/host1x/cdma.c
> +++ b/drivers/gpu/host1x/cdma.c

[ ... ]

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

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem earlier in
this same function regarding syncpt_val and job->syncpt_end wrap-around.

In host1x_cdma_update_sync_queue(), the loop over the sync_queue uses a
direct comparison to check if a job is uncompleted:

	list_for_each_entry(job, &cdma->sync_queue, list) {
		if (syncpt_val < job->syncpt_end) {

			if (!list_is_last(&job->list, &cdma->sync_queue))
				next_job = list_next_entry(job, list);

			goto syncpt_incr;
		}

If syncpt_val is near the maximum value and job->syncpt_end has wrapped
around to a small value, the condition evaluates to false. This makes the
recovery logic incorrectly assume the job has completed, skipping the CPU
increments and resuming the hardware channel without bypassing the failed
job.

Should that comparison also be updated to handle wrap-around conditions
safely?

>  		dev_dbg(dev, "%s: CPU incr (%d)\n", __func__, syncpt_incrs);
>  
>  		host1x_job_dump(dev, job);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-b4-host1x-small-fixes-a-v1-0-7c1131c0b3ad@nvidia.com?part=6