Re: [PATCH v2 1/3] drm/v3d: Associate BOs with every job that accesses them

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incomplete fix: `bin` job lacks implicit synchronization dependencies due to missing BO attachment.
--

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

drm/v3d: Associate BOs with every job that accesses them

This patch modifies the V3D driver to attach buffer objects (BOs) to every
job in a submission chain rather than just the last job. This is intended to
ensure all jobs receive the proper implicit synchronization dependencies
during reservation locking.

> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
> index ee2ac2540ed50..18467e448c91f 100644
> --- a/drivers/gpu/drm/v3d/v3d_submit.c
> +++ b/drivers/gpu/drm/v3d/v3d_submit.c

[ ... ]

> @@ -1085,6 +1081,11 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		goto fail;
>  
> +	ret = v3d_lookup_bos(submit.file_priv, &render->base,
> +			     args->bo_handles, args->bo_handle_count);
> +	if (ret)
> +		goto fail;
> +

[Severity: High]
This is a pre-existing issue, but does the bin job also need to have BOs
attached to pick up the correct implicit dependencies?

The commit message indicates that v3d_lookup_bos() should be called for
each job that references the submission's BOs. However, it looks like
v3d_lookup_bos() is only being called for the render job here.

If a userspace client submits a binner command list, the bin job is created
earlier in v3d_submit_cl_ioctl() via v3d_submit_add_job(). Because
v3d_lookup_bos() is not called for it, bin->base.bo_count remains 0.

Could this cause the hardware to execute the bin job without implicit
synchronization, potentially leading to corrupted data if another context
is still writing to the BOs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-v3d-cpu-job-fixes-v2-0-fec132484fc4@igalia.com?part=1