From 0d352a3a8a1f26168d09f7073e61bb4b328e3bb9 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Mon, 16 Sep 2019 09:11:25 +0200 Subject: drm/v3d: don't leak bin job if v3d_job_init fails. If the initialization of the job fails we need to kfree() it before returning. Signed-off-by: Iago Toral Quiroga Signed-off-by: Eric Anholt Link: https://patchwork.freedesktop.org/patch/msgid/20190916071125.5255-1-itoral@igalia.com Fixes: a783a09ee76d ("drm/v3d: Refactor job management.") Reviewed-by: Eric Anholt --- drivers/gpu/drm/v3d/v3d_gem.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpu/drm/v3d/v3d_gem.c') diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 5d80507b539b..fb32cda18ffe 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -563,6 +563,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, ret = v3d_job_init(v3d, file_priv, &bin->base, v3d_job_free, args->in_sync_bcl); if (ret) { + kfree(bin); v3d_job_put(&render->base); return ret; } -- cgit From 455d56ce809fcc540dc029a05db074855269dc33 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Thu, 19 Sep 2019 09:10:16 +0200 Subject: drm/v3d: clean caches at the end of render jobs on request from user space Extends the user space ioctl for CL submissions so it can include a request to flush the cache once the CL execution has completed. Fixes memory write violation messages reported by the kernel in workloads involving shader memory writes (SSBOs, shader images, scratch, etc) which sometimes also lead to GPU resets during Piglit and CTS workloads. v2: if v3d_job_init() fails we need to kfree() the job instead of v3d_job_put() it (Eric Anholt). v3 (Eric Anholt): - Drop _FLAG suffix from the new flag name. - Add a new param so userspace can tell whether cache flushing is implemented in the kernel. Signed-off-by: Iago Toral Quiroga Reviewed-by: Eric Anholt Signed-off-by: Eric Anholt Link: https://patchwork.freedesktop.org/patch/msgid/20190919071016.4578-1-itoral@igalia.com --- drivers/gpu/drm/v3d/v3d_drv.c | 3 +++ drivers/gpu/drm/v3d/v3d_gem.c | 54 ++++++++++++++++++++++++++++++++++++------- include/uapi/drm/v3d_drm.h | 8 ++++--- 3 files changed, 54 insertions(+), 11 deletions(-) (limited to 'drivers/gpu/drm/v3d/v3d_gem.c') diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index 3506ae2723ae..e94bf75368be 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -126,6 +126,9 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data, case DRM_V3D_PARAM_SUPPORTS_CSD: args->value = v3d_has_csd(v3d); return 0; + case DRM_V3D_PARAM_SUPPORTS_CACHE_FLUSH: + args->value = 1; + return 0; default: DRM_DEBUG("Unknown parameter %d\n", args->param); return -EINVAL; diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index fb32cda18ffe..4c4b59ae2c81 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -530,13 +530,16 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, struct drm_v3d_submit_cl *args = data; struct v3d_bin_job *bin = NULL; struct v3d_render_job *render; + struct v3d_job *clean_job = NULL; + struct v3d_job *last_job; struct ww_acquire_ctx acquire_ctx; int ret = 0; trace_v3d_submit_cl_ioctl(&v3d->drm, args->rcl_start, args->rcl_end); - if (args->pad != 0) { - DRM_INFO("pad must be zero: %d\n", args->pad); + if (args->flags != 0 && + args->flags != DRM_V3D_SUBMIT_CL_FLUSH_CACHE) { + DRM_INFO("invalid flags: %d\n", args->flags); return -EINVAL; } @@ -576,12 +579,31 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, bin->render = render; } - ret = v3d_lookup_bos(dev, file_priv, &render->base, + if (args->flags & DRM_V3D_SUBMIT_CL_FLUSH_CACHE) { + clean_job = kcalloc(1, sizeof(*clean_job), GFP_KERNEL); + if (!clean_job) { + ret = -ENOMEM; + goto fail; + } + + ret = v3d_job_init(v3d, file_priv, clean_job, v3d_job_free, 0); + if (ret) { + kfree(clean_job); + clean_job = NULL; + goto fail; + } + + last_job = clean_job; + } else { + last_job = &render->base; + } + + ret = v3d_lookup_bos(dev, file_priv, last_job, args->bo_handles, args->bo_handle_count); if (ret) goto fail; - ret = v3d_lock_bo_reservations(&render->base, &acquire_ctx); + ret = v3d_lock_bo_reservations(last_job, &acquire_ctx); if (ret) goto fail; @@ -600,28 +622,44 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, ret = v3d_push_job(v3d_priv, &render->base, V3D_RENDER); if (ret) goto fail_unreserve; + + if (clean_job) { + struct dma_fence *render_fence = + dma_fence_get(render->base.done_fence); + ret = drm_gem_fence_array_add(&clean_job->deps, render_fence); + if (ret) + goto fail_unreserve; + ret = v3d_push_job(v3d_priv, clean_job, V3D_CACHE_CLEAN); + if (ret) + goto fail_unreserve; + } + mutex_unlock(&v3d->sched_lock); v3d_attach_fences_and_unlock_reservation(file_priv, - &render->base, + last_job, &acquire_ctx, args->out_sync, - render->base.done_fence); + last_job->done_fence); if (bin) v3d_job_put(&bin->base); v3d_job_put(&render->base); + if (clean_job) + v3d_job_put(clean_job); return 0; fail_unreserve: mutex_unlock(&v3d->sched_lock); - drm_gem_unlock_reservations(render->base.bo, - render->base.bo_count, &acquire_ctx); + drm_gem_unlock_reservations(last_job->bo, + last_job->bo_count, &acquire_ctx); fail: if (bin) v3d_job_put(&bin->base); v3d_job_put(&render->base); + if (clean_job) + v3d_job_put(clean_job); return ret; } diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h index 58fbe48c91e9..1ce746e228d9 100644 --- a/include/uapi/drm/v3d_drm.h +++ b/include/uapi/drm/v3d_drm.h @@ -48,6 +48,8 @@ extern "C" { #define DRM_IOCTL_V3D_SUBMIT_TFU DRM_IOW(DRM_COMMAND_BASE + DRM_V3D_SUBMIT_TFU, struct drm_v3d_submit_tfu) #define DRM_IOCTL_V3D_SUBMIT_CSD DRM_IOW(DRM_COMMAND_BASE + DRM_V3D_SUBMIT_CSD, struct drm_v3d_submit_csd) +#define DRM_V3D_SUBMIT_CL_FLUSH_CACHE 0x01 + /** * struct drm_v3d_submit_cl - ioctl argument for submitting commands to the 3D * engine. @@ -61,7 +63,7 @@ extern "C" { * flushed by the time the render done IRQ happens, which is the * trigger for out_sync. Any dirtying of cachelines by the job (only * possible using TMU writes) must be flushed by the caller using the - * CL's cache flush commands. + * DRM_V3D_SUBMIT_CL_FLUSH_CACHE_FLAG flag. */ struct drm_v3d_submit_cl { /* Pointer to the binner command list. @@ -124,8 +126,7 @@ struct drm_v3d_submit_cl { /* Number of BO handles passed in (size is that times 4). */ __u32 bo_handle_count; - /* Pad, must be zero-filled. */ - __u32 pad; + __u32 flags; }; /** @@ -193,6 +194,7 @@ enum drm_v3d_param { DRM_V3D_PARAM_V3D_CORE0_IDENT2, DRM_V3D_PARAM_SUPPORTS_TFU, DRM_V3D_PARAM_SUPPORTS_CSD, + DRM_V3D_PARAM_SUPPORTS_CACHE_FLUSH, }; struct drm_v3d_get_param { -- cgit From 29cd13cfd7624726d9e6becbae9aa419ef35af7f Mon Sep 17 00:00:00 2001 From: Navid Emamdoost Date: Mon, 21 Oct 2019 13:52:49 -0500 Subject: drm/v3d: Fix memory leak in v3d_submit_cl_ioctl In the impelementation of v3d_submit_cl_ioctl() there are two memory leaks. One is when allocation for bin fails, and the other is when bin initialization fails. If kcalloc fails to allocate memory for bin then render->base should be put. Also, if v3d_job_init() fails to initialize bin->base then allocated memory for bin should be released. Fixes: a783a09ee76d ("drm/v3d: Refactor job management.") Signed-off-by: Navid Emamdoost Reviewed-by: Eric Anholt Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20191021185250.26130-1-navid.emamdoost@gmail.com --- drivers/gpu/drm/v3d/v3d_gem.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/v3d/v3d_gem.c') diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 5d80507b539b..19c092d75266 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -557,13 +557,16 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, if (args->bcl_start != args->bcl_end) { bin = kcalloc(1, sizeof(*bin), GFP_KERNEL); - if (!bin) + if (!bin) { + v3d_job_put(&render->base); return -ENOMEM; + } ret = v3d_job_init(v3d, file_priv, &bin->base, v3d_job_free, args->in_sync_bcl); if (ret) { v3d_job_put(&render->base); + kfree(bin); return ret; } -- cgit