* [PATCH] drm/amdgpu: Protect the validate list with a mutex [not found] <a0ae6ae0-35ec-7da9-0671-42bdf126460b@amd.com> @ 2022-06-23 4:25 ` Luben Tuikov 2022-06-23 4:39 ` [PATCH v2] " Luben Tuikov 0 siblings, 1 reply; 3+ messages in thread From: Luben Tuikov @ 2022-06-23 4:25 UTC (permalink / raw) To: amd-gfx Cc: Alex Deucher, Andrey Grodzovsky, Luben Tuikov, Christian König, Vitaly Prosyak Protect the parser's validate list with a mutex in order to avoid buffer object corruption as recorded in the link below. Cc: Christian König <christian.koenig@amd.com> Cc: Alex Deucher <Alexander.Deucher@amd.com> Cc: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> Cc: Vitaly Prosyak <Vitaly.Prosyak@amd.com> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2048 Signed-off-by: Luben Tuikov <luben.tuikov@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 28 +++++++++++++++++++++----- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h | 4 ++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 36ac1f1d11e6b4..e593e30a8545f9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -498,6 +498,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, struct amdgpu_bo *oa; int r; + mutex_init(&p->mutex_validated); INIT_LIST_HEAD(&p->validated); /* p->bo_list could already be assigned if AMDGPU_CHUNK_ID_BO_HANDLES is present */ @@ -521,13 +522,15 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, amdgpu_bo_list_for_each_entry(e, p->bo_list) e->tv.num_shared = 2; - amdgpu_bo_list_get_list(p->bo_list, &p->validated); - INIT_LIST_HEAD(&duplicates); + + mutex_lock(&p->mutex_validated); + amdgpu_bo_list_get_list(p->bo_list, &p->validated); amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd); if (p->uf_entry.tv.bo && !ttm_to_amdgpu_bo(p->uf_entry.tv.bo)->parent) list_add(&p->uf_entry.tv.head, &p->validated); + mutex_unlock(&p->mutex_validated); /* Get userptr backing pages. If pages are updated after registered * in amdgpu_gem_userptr_ioctl(), amdgpu_cs_list_validate() will do @@ -563,8 +566,11 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, e->user_invalidated = userpage_invalidated; } + mutex_lock(&p->mutex_validated); r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true, &duplicates); + mutex_unlock(&p->mutex_validated); + if (unlikely(r != 0)) { if (r != -ERESTARTSYS) DRM_ERROR("ttm_eu_reserve_buffers failed.\n"); @@ -607,11 +613,15 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, goto error_validate; } + mutex_lock(&p->mutex_validated); r = amdgpu_cs_list_validate(p, &duplicates); - if (r) + if (r) { + mutex_unlock(&p->mutex_validated); goto error_validate; + } r = amdgpu_cs_list_validate(p, &p->validated); + mutex_unlock(&p->mutex_validated); if (r) goto error_validate; @@ -648,7 +658,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, dma_fence_chain_free(e->chain); e->chain = NULL; } + mutex_lock(&p->mutex_validated); ttm_eu_backoff_reservation(&p->ticket, &p->validated); + mutex_unlock(&p->mutex_validated); } out_free_user_pages: @@ -672,6 +684,7 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) struct amdgpu_bo_list_entry *e; int r; + mutex_lock(&p->mutex_validated); list_for_each_entry(e, &p->validated, tv.head) { struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo); struct dma_resv *resv = bo->tbo.base.resv; @@ -682,9 +695,10 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, sync_mode, &fpriv->vm); if (r) - return r; + break; } - return 0; + mutex_unlock(&p->mutex_validated); + return r ?: 0; } /** @@ -709,8 +723,10 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, e->chain = NULL; } + mutex_lock(&parser->mutex_validated); ttm_eu_backoff_reservation(&parser->ticket, &parser->validated); + mutex_unlock(&parser->mutex_validated); } for (i = 0; i < parser->num_post_deps; i++) { @@ -1307,7 +1323,9 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, e->chain = NULL; } + mutex_lock(&p->mutex_validated); ttm_eu_fence_buffer_objects(&p->ticket, &p->validated, p->fence); + mutex_unlock(&p->mutex_validated); mutex_unlock(&p->adev->notifier_lock); return 0; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h index 30ecc4917f811d..284d1c03d65d0b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h @@ -59,6 +59,10 @@ struct amdgpu_cs_parser { struct amdgpu_bo_list *bo_list; struct amdgpu_mn *mn; struct amdgpu_bo_list_entry vm_pd; + + /* Protect access to list "valided" below. + */ + struct mutex mutex_validated; struct list_head validated; struct dma_fence *fence; uint64_t bytes_moved_threshold; base-commit: ab7e60938be74e21c723223e7eb96cac7b441e5e -- 2.36.1.74.g277cf0bc36 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2] drm/amdgpu: Protect the validate list with a mutex 2022-06-23 4:25 ` [PATCH] drm/amdgpu: Protect the validate list with a mutex Luben Tuikov @ 2022-06-23 4:39 ` Luben Tuikov 2022-06-23 6:31 ` Christian König 0 siblings, 1 reply; 3+ messages in thread From: Luben Tuikov @ 2022-06-23 4:39 UTC (permalink / raw) To: amd-gfx Cc: Alex Deucher, Andrey Grodzovsky, Luben Tuikov, Christian König, Vitaly Prosyak Protect the parser's validate list with a mutex in order to avoid buffer object corruption as recorded in the link below. Cc: Christian König <christian.koenig@amd.com> Cc: Alex Deucher <Alexander.Deucher@amd.com> Cc: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> Cc: Vitaly Prosyak <Vitaly.Prosyak@amd.com> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2048 Signed-off-by: Luben Tuikov <luben.tuikov@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 30 ++++++++++++++++++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h | 4 ++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 36ac1f1d11e6b4..0be0bf17c05420 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -498,6 +498,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, struct amdgpu_bo *oa; int r; + mutex_init(&p->mutex_validated); INIT_LIST_HEAD(&p->validated); /* p->bo_list could already be assigned if AMDGPU_CHUNK_ID_BO_HANDLES is present */ @@ -521,13 +522,15 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, amdgpu_bo_list_for_each_entry(e, p->bo_list) e->tv.num_shared = 2; - amdgpu_bo_list_get_list(p->bo_list, &p->validated); - INIT_LIST_HEAD(&duplicates); + + mutex_lock(&p->mutex_validated); + amdgpu_bo_list_get_list(p->bo_list, &p->validated); amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd); if (p->uf_entry.tv.bo && !ttm_to_amdgpu_bo(p->uf_entry.tv.bo)->parent) list_add(&p->uf_entry.tv.head, &p->validated); + mutex_unlock(&p->mutex_validated); /* Get userptr backing pages. If pages are updated after registered * in amdgpu_gem_userptr_ioctl(), amdgpu_cs_list_validate() will do @@ -563,8 +566,11 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, e->user_invalidated = userpage_invalidated; } + mutex_lock(&p->mutex_validated); r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true, &duplicates); + mutex_unlock(&p->mutex_validated); + if (unlikely(r != 0)) { if (r != -ERESTARTSYS) DRM_ERROR("ttm_eu_reserve_buffers failed.\n"); @@ -607,11 +613,15 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, goto error_validate; } + mutex_lock(&p->mutex_validated); r = amdgpu_cs_list_validate(p, &duplicates); - if (r) + if (r) { + mutex_unlock(&p->mutex_validated); goto error_validate; + } r = amdgpu_cs_list_validate(p, &p->validated); + mutex_unlock(&p->mutex_validated); if (r) goto error_validate; @@ -648,7 +658,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, dma_fence_chain_free(e->chain); e->chain = NULL; } + mutex_lock(&p->mutex_validated); ttm_eu_backoff_reservation(&p->ticket, &p->validated); + mutex_unlock(&p->mutex_validated); } out_free_user_pages: @@ -670,8 +682,9 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) { struct amdgpu_fpriv *fpriv = p->filp->driver_priv; struct amdgpu_bo_list_entry *e; - int r; + int r = 0; + mutex_lock(&p->mutex_validated); list_for_each_entry(e, &p->validated, tv.head) { struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo); struct dma_resv *resv = bo->tbo.base.resv; @@ -682,9 +695,10 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, sync_mode, &fpriv->vm); if (r) - return r; + break; } - return 0; + mutex_unlock(&p->mutex_validated); + return r; } /** @@ -709,8 +723,10 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, e->chain = NULL; } + mutex_lock(&parser->mutex_validated); ttm_eu_backoff_reservation(&parser->ticket, &parser->validated); + mutex_unlock(&parser->mutex_validated); } for (i = 0; i < parser->num_post_deps; i++) { @@ -1307,7 +1323,9 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, e->chain = NULL; } + mutex_lock(&p->mutex_validated); ttm_eu_fence_buffer_objects(&p->ticket, &p->validated, p->fence); + mutex_unlock(&p->mutex_validated); mutex_unlock(&p->adev->notifier_lock); return 0; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h index 30ecc4917f811d..284d1c03d65d0b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h @@ -59,6 +59,10 @@ struct amdgpu_cs_parser { struct amdgpu_bo_list *bo_list; struct amdgpu_mn *mn; struct amdgpu_bo_list_entry vm_pd; + + /* Protect access to list "valided" below. + */ + struct mutex mutex_validated; struct list_head validated; struct dma_fence *fence; uint64_t bytes_moved_threshold; base-commit: ab7e60938be74e21c723223e7eb96cac7b441e5e -- 2.36.1.74.g277cf0bc36 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/amdgpu: Protect the validate list with a mutex 2022-06-23 4:39 ` [PATCH v2] " Luben Tuikov @ 2022-06-23 6:31 ` Christian König 0 siblings, 0 replies; 3+ messages in thread From: Christian König @ 2022-06-23 6:31 UTC (permalink / raw) To: Luben Tuikov, amd-gfx; +Cc: Alex Deucher, Andrey Grodzovsky, Vitaly Prosyak The mutex must be added to the bo_list structure, not the parser structure. The parser is only a temporary structure we allocate for the current thread. Regards, Christian. Am 23.06.22 um 06:39 schrieb Luben Tuikov: > Protect the parser's validate list with a mutex in order to avoid buffer > object corruption as recorded in the link below. > > Cc: Christian König <christian.koenig@amd.com> > Cc: Alex Deucher <Alexander.Deucher@amd.com> > Cc: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > Cc: Vitaly Prosyak <Vitaly.Prosyak@amd.com> > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2048 > Signed-off-by: Luben Tuikov <luben.tuikov@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 30 ++++++++++++++++++++------ > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h | 4 ++++ > 2 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 36ac1f1d11e6b4..0be0bf17c05420 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -498,6 +498,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, > struct amdgpu_bo *oa; > int r; > > + mutex_init(&p->mutex_validated); > INIT_LIST_HEAD(&p->validated); > > /* p->bo_list could already be assigned if AMDGPU_CHUNK_ID_BO_HANDLES is present */ > @@ -521,13 +522,15 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, > amdgpu_bo_list_for_each_entry(e, p->bo_list) > e->tv.num_shared = 2; > > - amdgpu_bo_list_get_list(p->bo_list, &p->validated); > - > INIT_LIST_HEAD(&duplicates); > + > + mutex_lock(&p->mutex_validated); > + amdgpu_bo_list_get_list(p->bo_list, &p->validated); > amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd); > > if (p->uf_entry.tv.bo && !ttm_to_amdgpu_bo(p->uf_entry.tv.bo)->parent) > list_add(&p->uf_entry.tv.head, &p->validated); > + mutex_unlock(&p->mutex_validated); > > /* Get userptr backing pages. If pages are updated after registered > * in amdgpu_gem_userptr_ioctl(), amdgpu_cs_list_validate() will do > @@ -563,8 +566,11 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, > e->user_invalidated = userpage_invalidated; > } > > + mutex_lock(&p->mutex_validated); > r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true, > &duplicates); > + mutex_unlock(&p->mutex_validated); > + > if (unlikely(r != 0)) { > if (r != -ERESTARTSYS) > DRM_ERROR("ttm_eu_reserve_buffers failed.\n"); > @@ -607,11 +613,15 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, > goto error_validate; > } > > + mutex_lock(&p->mutex_validated); > r = amdgpu_cs_list_validate(p, &duplicates); > - if (r) > + if (r) { > + mutex_unlock(&p->mutex_validated); > goto error_validate; > + } > > r = amdgpu_cs_list_validate(p, &p->validated); > + mutex_unlock(&p->mutex_validated); > if (r) > goto error_validate; > > @@ -648,7 +658,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, > dma_fence_chain_free(e->chain); > e->chain = NULL; > } > + mutex_lock(&p->mutex_validated); > ttm_eu_backoff_reservation(&p->ticket, &p->validated); > + mutex_unlock(&p->mutex_validated); > } > > out_free_user_pages: > @@ -670,8 +682,9 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) > { > struct amdgpu_fpriv *fpriv = p->filp->driver_priv; > struct amdgpu_bo_list_entry *e; > - int r; > + int r = 0; > > + mutex_lock(&p->mutex_validated); > list_for_each_entry(e, &p->validated, tv.head) { > struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo); > struct dma_resv *resv = bo->tbo.base.resv; > @@ -682,9 +695,10 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) > r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, sync_mode, > &fpriv->vm); > if (r) > - return r; > + break; > } > - return 0; > + mutex_unlock(&p->mutex_validated); > + return r; > } > > /** > @@ -709,8 +723,10 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, > e->chain = NULL; > } > > + mutex_lock(&parser->mutex_validated); > ttm_eu_backoff_reservation(&parser->ticket, > &parser->validated); > + mutex_unlock(&parser->mutex_validated); > } > > for (i = 0; i < parser->num_post_deps; i++) { > @@ -1307,7 +1323,9 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, > e->chain = NULL; > } > > + mutex_lock(&p->mutex_validated); > ttm_eu_fence_buffer_objects(&p->ticket, &p->validated, p->fence); > + mutex_unlock(&p->mutex_validated); > mutex_unlock(&p->adev->notifier_lock); > > return 0; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h > index 30ecc4917f811d..284d1c03d65d0b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h > @@ -59,6 +59,10 @@ struct amdgpu_cs_parser { > struct amdgpu_bo_list *bo_list; > struct amdgpu_mn *mn; > struct amdgpu_bo_list_entry vm_pd; > + > + /* Protect access to list "valided" below. > + */ > + struct mutex mutex_validated; > struct list_head validated; > struct dma_fence *fence; > uint64_t bytes_moved_threshold; > > base-commit: ab7e60938be74e21c723223e7eb96cac7b441e5e ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-23 6:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <a0ae6ae0-35ec-7da9-0671-42bdf126460b@amd.com>
2022-06-23 4:25 ` [PATCH] drm/amdgpu: Protect the validate list with a mutex Luben Tuikov
2022-06-23 4:39 ` [PATCH v2] " Luben Tuikov
2022-06-23 6:31 ` Christian König
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.