A submission can expand into a chain of jobs (e.g. bin + render + cache
clean), but v3d_lookup_bos() only looked up the user's BO list onto the
*last* job of the submission. Every earlier job was left with
bo_count == 0 and an empty bo[] array.
As a consequence, when implicit synchronization happens in
v3d_submit_lock_reservations(), earlier jobs get no implicit
dependencies at all, as the loop is gated on job->bo_count and earlier
jobs don't have any BO attached to them. With that, the BIN job reads the
same buffers as the RENDER job, yet nothing made it wait for a prior
writer to finish. The BIN job could therefore be dispatched to the
hardware and read a BO while another context was still writing it,
leading to data corruption that was only avoided as the userspace adds
explicit syncobjs.
Fix this by calling v3d_lookup_bos() for each job that references the
submission's BOs, so every job carries its own bo[]/bo_count and picks
up the correct implicit dependencies during reservation locking.
Fixes: dffa9b7a78c4 ("drm/v3d: Add missing implicit synchronization.")
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/v3d/v3d_submit.c | 41 +++++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index ee2ac2540ed5..3d6582dfb1bf 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -68,7 +68,6 @@ v3d_submit_unlock_reservations(struct v3d_submit *submit)
/**
* v3d_lookup_bos() - Sets up job->bo[] with the GEM objects
* referenced by the job.
- * @dev: DRM device
* @file_priv: DRM file for this fd
* @job: V3D job being set up
* @bo_handles: GEM handles
@@ -82,23 +81,19 @@ v3d_submit_unlock_reservations(struct v3d_submit *submit)
* failure, because that will happen at `v3d_job_free()`.
*/
static int
-v3d_lookup_bos(struct v3d_submit *submit, u64 bo_handles, u32 bo_count)
+v3d_lookup_bos(struct drm_file *file_priv, struct v3d_job *job,
+ u64 bo_handles, u32 bo_count)
{
- struct v3d_job *last_job = submit->jobs[submit->job_count - 1];
-
- last_job->bo_count = bo_count;
-
- if (!last_job->bo_count) {
- /* See comment on bo_index for why we have to check
- * this.
- */
- drm_warn(&submit->v3d->drm, "Rendering requires BOs\n");
+ if (!bo_count) {
+ drm_warn(&job->v3d->drm, "Rendering requires BOs\n");
return -EINVAL;
}
- return drm_gem_objects_lookup(submit->file_priv,
+ job->bo_count = bo_count;
+
+ return drm_gem_objects_lookup(file_priv,
(void __user *)(uintptr_t)bo_handles,
- last_job->bo_count, &last_job->bo);
+ job->bo_count, &job->bo);
}
static void
@@ -446,7 +441,8 @@ v3d_setup_csd_jobs_and_bos(struct v3d_submit *submit,
if (IS_ERR(clean_job))
return PTR_ERR(clean_job);
- return v3d_lookup_bos(submit, args->bo_handles, args->bo_handle_count);
+ return v3d_lookup_bos(submit->file_priv, &job->base,
+ args->bo_handles, args->bo_handle_count);
}
static void
@@ -1066,6 +1062,11 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
&se);
if (ret)
goto fail;
+
+ ret = v3d_lookup_bos(submit.file_priv, &bin->base,
+ args->bo_handles, args->bo_handle_count);
+ if (ret)
+ goto fail;
}
render = (struct v3d_render_job *)v3d_submit_add_job(&submit,
V3D_RENDER);
@@ -1085,6 +1086,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;
+
if (args->flags & DRM_V3D_SUBMIT_CL_FLUSH_CACHE) {
clean_job = v3d_submit_add_job(&submit, V3D_CACHE_CLEAN);
if (IS_ERR(clean_job)) {
@@ -1097,10 +1103,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
if (ret)
goto fail;
- ret = v3d_lookup_bos(&submit, args->bo_handles, args->bo_handle_count);
- if (ret)
- goto fail;
-
ret = v3d_submit_lock_reservations(&submit);
if (ret)
goto fail;
@@ -1359,7 +1361,8 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
* the CSD and clean jobs in the case of indirect CSD job.
*/
if (args->bo_handle_count) {
- ret = v3d_lookup_bos(&submit, args->bo_handles,
args->bo_handle_count);
+ ret = v3d_lookup_bos(submit.file_priv, &cpu_job->base,
+ args->bo_handles, args->bo_handle_count);
if (ret)
goto fail;
}
--
2.54.0
