Re: [PATCH v3 08/14] drm/v3d: Introduce struct v3d_submit and convert CL/TFU/CSD ioctls

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Earlier jobs in the submission chain (BIN and RENDER) do not wait for implicit BO dependencies, leading to concurrent hardware access.
--

commit c2a10c5b0fe7c3f03c50306d4d031d5c5d2ba3a6
Author: Maíra Canal <mcanal@igalia.com>

drm/v3d: Introduce struct v3d_submit and convert CL/TFU/CSD ioctls

As the V3D driver grew with time, different types of submission were added
and the submission code grew more complex, but the driver stuck with the
same abstractions.

> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
> index d80afbf2c03be..fcaf3a6cfddc6 100644
> --- a/drivers/gpu/drm/v3d/v3d_submit.c
> +++ b/drivers/gpu/drm/v3d/v3d_submit.c
[ ... ]
> @@ -984,99 +1088,71 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
[ ... ]
> -	ret = v3d_lookup_bos(dev, file_priv, last_job,
> -			     args->bo_handles, args->bo_handle_count);
> -	if (ret)
> +	ret = v3d_lookup_bos(dev, file_priv,
> +			     submit.jobs[submit.job_count - 1],
> +			     args->bo_handles, args->bo_handle_count);
> +	if (ret)
>  		goto fail;
>  
> -	ret = v3d_lock_bo_reservations(last_job, &exec);
> -	if (ret)
> -		goto fail;
> +	ret = v3d_lock_bo_reservations(submit.jobs[submit.job_count - 1],
> +				       &submit.exec);
> +	if (ret)
> +		goto fail;

[Severity: High]
This is a pre-existing issue, but since earlier jobs in the submission chain
(like BIN and RENDER) don't have these BOs associated with them, they
receive no implicit fences.

Because v3d_lookup_bos() only associates the BOs with the last job
(submit.jobs[submit.job_count - 1]), the earlier jobs have their bo_count
set to 0. Consequently, they skip adding implicit dependencies when
reservations are processed. Even if a subsequent patch updates the code
to iterate over all jobs to add dependencies, earlier jobs will still be
skipped because their job->bo_count is 0.

Could this cause the earlier jobs to be pushed to the hardware immediately,
allowing them to read BOs while they are still being written by previous
operations from other processes or hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-v3d-sched-misc-fixes-v3-0-d7114bba55a0@igalia.com?part=8