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 RENDER job
could
therefore be dispatched to the hardware and read a BO while another
context
was still writing it, leading to data corruption.
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 | 36 +++++++++++++++++-------------
------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c
b/drivers/gpu/drm/v3d/v3d_submit.c
index ee2ac2540ed5..18467e448c91 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
@@ -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;
+
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 +1098,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 +1356,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;
}
