From 0e51a772e2014db55b969c06814e8fe01d167ba2 Mon Sep 17 00:00:00 2001 From: Christian König Date: Wed, 18 May 2016 14:19:32 +0200 Subject: drm/amdgpu: properly abstract scheduler timeout handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The driver shouldn't mess with the scheduler internals. Signed-off-by: Christian König Reviewed-by: Monk.Liu Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 9bc8f1d99733..72694d7f11f5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -838,8 +838,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, p->job = NULL; r = amd_sched_job_init(&job->base, &ring->sched, - entity, amdgpu_job_timeout_func, - amdgpu_job_free_func, + entity, amdgpu_job_free_func, p->filp, &fence); if (r) { amdgpu_job_free(job); -- cgit v1.2.3 From c5f74f7802775b9ccdb0a4fd90e0c7d0b03da9fa Mon Sep 17 00:00:00 2001 From: Christian König Date: Thu, 19 May 2016 09:54:15 +0200 Subject: drm/amdgpu: fix and cleanup job destruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the job reference counting and just properly destroy it from a work item which blocks on any potential running timeout handler. Signed-off-by: Christian König Reviewed-by: Monk.Liu Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 18 +++-------- drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 46 +++++++++++---------------- drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 25 +++------------ 5 files changed, 30 insertions(+), 63 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index a5d1cfbb1a87..ea64c6569704 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -755,7 +755,6 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size, struct amdgpu_job **job); void amdgpu_job_free(struct amdgpu_job *job); -void amdgpu_job_free_func(struct kref *refcount); int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring, struct amd_sched_entity *entity, void *owner, struct fence **f); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 72694d7f11f5..523da20e6ea0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -838,8 +838,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, p->job = NULL; r = amd_sched_job_init(&job->base, &ring->sched, - entity, amdgpu_job_free_func, - p->filp, &fence); + entity, p->filp, &fence); if (r) { amdgpu_job_free(job); return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index 32132f2e236d..34cd971a9afa 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -28,12 +28,6 @@ #include "amdgpu.h" #include "amdgpu_trace.h" -static void amdgpu_job_free_handler(struct work_struct *ws) -{ - struct amdgpu_job *job = container_of(ws, struct amdgpu_job, base.work_free_job); - amd_sched_job_put(&job->base); -} - static void amdgpu_job_timedout(struct amd_sched_job *s_job) { struct amdgpu_job *job = container_of(s_job, struct amdgpu_job, base); @@ -42,8 +36,6 @@ static void amdgpu_job_timedout(struct amd_sched_job *s_job) job->base.sched->name, atomic_read(&job->ring->fence_drv.last_seq), job->ring->fence_drv.sync_seq); - - amd_sched_job_put(&job->base); } int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs, @@ -64,7 +56,6 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs, (*job)->vm = vm; (*job)->ibs = (void *)&(*job)[1]; (*job)->num_ibs = num_ibs; - INIT_WORK(&(*job)->base.work_free_job, amdgpu_job_free_handler); amdgpu_sync_create(&(*job)->sync); @@ -103,9 +94,10 @@ static void amdgpu_job_free_resources(struct amdgpu_job *job) amdgpu_sync_free(&job->sync); } -void amdgpu_job_free_func(struct kref *refcount) +void amdgpu_job_free_cb(struct amd_sched_job *s_job) { - struct amdgpu_job *job = container_of(refcount, struct amdgpu_job, base.refcount); + struct amdgpu_job *job = container_of(s_job, struct amdgpu_job, base); + kfree(job); } @@ -126,8 +118,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring, if (!f) return -EINVAL; - r = amd_sched_job_init(&job->base, &ring->sched, - entity, amdgpu_job_free_func, owner, &fence); + r = amd_sched_job_init(&job->base, &ring->sched, entity, owner, &fence); if (r) return r; @@ -198,4 +189,5 @@ const struct amd_sched_backend_ops amdgpu_sched_ops = { .dependency = amdgpu_job_dependency, .run_job = amdgpu_job_run, .timedout_job = amdgpu_job_timedout, + .free_job = amdgpu_job_free_cb }; diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c index cb56d9065e43..2425172e612e 100644 --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c @@ -319,19 +319,13 @@ static bool amd_sched_entity_in(struct amd_sched_job *sched_job) return added; } -static void amd_sched_free_job(struct fence *f, struct fence_cb *cb) { - struct amd_sched_job *job = container_of(cb, struct amd_sched_job, - cb_free_job); - - schedule_work(&job->work_free_job); -} - /* job_finish is called after hw fence signaled, and * the job had already been deleted from ring_mirror_list */ -static void amd_sched_job_finish(struct amd_sched_job *s_job) +static void amd_sched_job_finish(struct work_struct *work) { - struct amd_sched_job *next; + struct amd_sched_job *s_job = container_of(work, struct amd_sched_job, + finish_work); struct amd_gpu_scheduler *sched = s_job->sched; unsigned long flags; @@ -339,19 +333,26 @@ static void amd_sched_job_finish(struct amd_sched_job *s_job) spin_lock_irqsave(&sched->job_list_lock, flags); list_del_init(&s_job->node); if (sched->timeout != MAX_SCHEDULE_TIMEOUT) { - if (cancel_delayed_work(&s_job->work_tdr)) - amd_sched_job_put(s_job); + struct amd_sched_job *next; + + cancel_delayed_work_sync(&s_job->work_tdr); /* queue TDR for next job */ next = list_first_entry_or_null(&sched->ring_mirror_list, struct amd_sched_job, node); - if (next) { - amd_sched_job_get(next); + if (next) schedule_delayed_work(&next->work_tdr, sched->timeout); - } } spin_unlock_irqrestore(&sched->job_list_lock, flags); + sched->ops->free_job(s_job); +} + +static void amd_sched_job_finish_cb(struct fence *f, struct fence_cb *cb) +{ + struct amd_sched_job *job = container_of(cb, struct amd_sched_job, + finish_cb); + schedule_work(&job->finish_work); } static void amd_sched_job_begin(struct amd_sched_job *s_job) @@ -364,10 +365,7 @@ static void amd_sched_job_begin(struct amd_sched_job *s_job) if (sched->timeout != MAX_SCHEDULE_TIMEOUT && list_first_entry_or_null(&sched->ring_mirror_list, struct amd_sched_job, node) == s_job) - { - amd_sched_job_get(s_job); schedule_delayed_work(&s_job->work_tdr, sched->timeout); - } spin_unlock_irqrestore(&sched->job_list_lock, flags); } @@ -390,9 +388,9 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job) { struct amd_sched_entity *entity = sched_job->s_entity; - fence_add_callback(&sched_job->s_fence->base, - &sched_job->cb_free_job, amd_sched_free_job); trace_amd_sched_job(sched_job); + fence_add_callback(&sched_job->s_fence->base, &sched_job->finish_cb, + amd_sched_job_finish_cb); wait_event(entity->sched->job_scheduled, amd_sched_entity_in(sched_job)); } @@ -401,20 +399,17 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job) int amd_sched_job_init(struct amd_sched_job *job, struct amd_gpu_scheduler *sched, struct amd_sched_entity *entity, - void (*free_cb)(struct kref *refcount), void *owner, struct fence **fence) { - INIT_LIST_HEAD(&job->node); - kref_init(&job->refcount); job->sched = sched; job->s_entity = entity; job->s_fence = amd_sched_fence_create(entity, owner); if (!job->s_fence) return -ENOMEM; - job->s_fence->s_job = job; + INIT_WORK(&job->finish_work, amd_sched_job_finish); + INIT_LIST_HEAD(&job->node); INIT_DELAYED_WORK(&job->work_tdr, amd_sched_job_timedout); - job->free_callback = free_cb; if (fence) *fence = &job->s_fence->base; @@ -468,9 +463,6 @@ static void amd_sched_process_job(struct fence *f, struct fence_cb *cb) struct amd_gpu_scheduler *sched = s_fence->sched; atomic_dec(&sched->hw_rq_count); - - amd_sched_job_finish(s_fence->s_job); - amd_sched_fence_signal(s_fence); trace_amd_sched_process_job(s_fence); diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h index f0de46ce7afe..e63034ec7fa4 100644 --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h @@ -74,19 +74,16 @@ struct amd_sched_fence { struct amd_gpu_scheduler *sched; spinlock_t lock; void *owner; - struct amd_sched_job *s_job; }; struct amd_sched_job { - struct kref refcount; struct amd_gpu_scheduler *sched; struct amd_sched_entity *s_entity; struct amd_sched_fence *s_fence; - struct fence_cb cb_free_job; - struct work_struct work_free_job; - struct list_head node; - struct delayed_work work_tdr; - void (*free_callback)(struct kref *refcount); + struct fence_cb finish_cb; + struct work_struct finish_work; + struct list_head node; + struct delayed_work work_tdr; }; extern const struct fence_ops amd_sched_fence_ops; @@ -109,6 +106,7 @@ struct amd_sched_backend_ops { struct fence *(*dependency)(struct amd_sched_job *sched_job); struct fence *(*run_job)(struct amd_sched_job *sched_job); void (*timedout_job)(struct amd_sched_job *sched_job); + void (*free_job)(struct amd_sched_job *sched_job); }; enum amd_sched_priority { @@ -154,18 +152,5 @@ void amd_sched_fence_signal(struct amd_sched_fence *fence); int amd_sched_job_init(struct amd_sched_job *job, struct amd_gpu_scheduler *sched, struct amd_sched_entity *entity, - void (*free_cb)(struct kref* refcount), void *owner, struct fence **fence); -static inline void amd_sched_job_get(struct amd_sched_job *job) -{ - if (job) - kref_get(&job->refcount); -} - -static inline void amd_sched_job_put(struct amd_sched_job *job) -{ - if (job) - kref_put(&job->refcount, job->free_callback); -} - #endif -- cgit v1.2.3 From 281d144ddd4f6b3944c32ab0f346b03f0de0e6b8 Mon Sep 17 00:00:00 2001 From: Christian König Date: Wed, 15 Jun 2016 13:44:04 +0200 Subject: drm/amdgpu: save the PD addr before scheduling the job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we pipeline evictions the page directory could already be moving somewhere else when grab_id is called. Reviewed-by: Alex Deucher Signed-off-by: Christian König Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++---- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 523da20e6ea0..de171ccf2b9c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -660,6 +660,8 @@ static int amdgpu_cs_ib_vm_chunk(struct amdgpu_device *adev, } } + p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->page_directory); + r = amdgpu_bo_vm_update_pte(p, vm); if (!r) amdgpu_cs_sync_rings(p); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 18daa2d64d57..adb87789cc34 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -177,7 +177,6 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring, struct amdgpu_sync *sync, struct fence *fence, unsigned *vm_id, uint64_t *vm_pd_addr) { - uint64_t pd_addr = amdgpu_bo_gpu_offset(vm->page_directory); struct amdgpu_device *adev = ring->adev; struct fence *updates = sync->last_vm_update; struct amdgpu_vm_id *id, *idle; @@ -250,7 +249,7 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring, if (atomic64_read(&id->owner) != vm->client_id) continue; - if (pd_addr != id->pd_gpu_addr) + if (*vm_pd_addr != id->pd_gpu_addr) continue; if (!same_ring && @@ -298,14 +297,13 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring, fence_put(id->flushed_updates); id->flushed_updates = fence_get(updates); - id->pd_gpu_addr = pd_addr; + id->pd_gpu_addr = *vm_pd_addr; list_move_tail(&id->list, &adev->vm_manager.ids_lru); atomic64_set(&id->owner, vm->client_id); vm->ids[ring->idx] = id; *vm_id = id - adev->vm_manager.ids; - *vm_pd_addr = pd_addr; trace_amdgpu_vm_grab_id(vm, ring->idx, *vm_id, *vm_pd_addr); error: -- cgit v1.2.3 From 5a712a8727200fc617072bed2ccd8ba8e1b50408 Mon Sep 17 00:00:00 2001 From: Christian König Date: Tue, 21 Jun 2016 16:28:15 +0200 Subject: drm/amdgpu: validate VM PTs only on eviction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need to validate them again if the eviction counter didn't changed. Reviewed-by: Alex Deucher Signed-off-by: Christian König Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 ++++- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 13 ++++++++++++- 4 files changed, 20 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index f4d269931dbe..80d9932d86fb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -864,6 +864,7 @@ struct amdgpu_vm { struct amdgpu_bo *page_directory; unsigned max_pde_used; struct fence *page_directory_fence; + uint64_t last_eviction_counter; /* array of page tables, one for each page directory entry */ struct amdgpu_vm_pt *page_tables; @@ -932,7 +933,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm); void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm, struct list_head *validated, struct amdgpu_bo_list_entry *entry); -void amdgpu_vm_get_pt_bos(struct amdgpu_vm *vm, struct list_head *duplicates); +void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, + struct list_head *duplicates); void amdgpu_vm_move_pt_bos_in_lru(struct amdgpu_device *adev, struct amdgpu_vm *vm); int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index de171ccf2b9c..cf97a87f76a6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -459,7 +459,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, list_splice(&need_pages, &p->validated); } - amdgpu_vm_get_pt_bos(&fpriv->vm, &duplicates); + amdgpu_vm_get_pt_bos(p->adev, &fpriv->vm, &duplicates); p->bytes_moved_threshold = amdgpu_cs_get_threshold_for_moves(p->adev); p->bytes_moved = 0; @@ -472,6 +472,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, if (r) goto error_validate; + fpriv->vm.last_eviction_counter = + atomic64_read(&p->adev->num_evictions); + if (p->bo_list) { struct amdgpu_bo *gds = p->bo_list->gds_obj; struct amdgpu_bo *gws = p->bo_list->gws_obj; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 8fab6486064f..88fbed2389c0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -503,7 +503,7 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev, if (r) goto error_print; - amdgpu_vm_get_pt_bos(bo_va->vm, &duplicates); + amdgpu_vm_get_pt_bos(adev, bo_va->vm, &duplicates); list_for_each_entry(entry, &list, head) { domain = amdgpu_mem_type_to_domain(entry->bo->mem.mem_type); /* if anything is swapped out don't swap it in here, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 8f04857fbe1b..ea10b7af33df 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -115,16 +115,26 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm, /** * amdgpu_vm_get_bos - add the vm BOs to a duplicates list * + * @adev: amdgpu device pointer * @vm: vm providing the BOs * @duplicates: head of duplicates list * * Add the page directory to the BO duplicates list * for command submission. */ -void amdgpu_vm_get_pt_bos(struct amdgpu_vm *vm, struct list_head *duplicates) +void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, + struct list_head *duplicates) { + uint64_t num_evictions; unsigned i; + /* We only need to validate the page tables + * if they aren't already valid. + */ + num_evictions = atomic64_read(&adev->num_evictions); + if (num_evictions == vm->last_eviction_counter) + return; + /* add the vm page table to the list */ for (i = 0; i <= vm->max_pde_used; ++i) { struct amdgpu_bo_list_entry *entry = &vm->page_tables[i].entry; @@ -1534,6 +1544,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm) amdgpu_bo_unreserve(vm->page_directory); if (r) goto error_free_page_directory; + vm->last_eviction_counter = atomic64_read(&adev->num_evictions); return 0; -- cgit v1.2.3 From 9a79588c42bf89cff48fdc6db4aab4aeb9d862b6 Mon Sep 17 00:00:00 2001 From: Christian König Date: Wed, 22 Jun 2016 14:25:55 +0200 Subject: drm/amdgpu: don't update page tables for VM emulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's just overhead to do so and allocating a VMID when we don't need one is actually a bit dangerous. Signed-off-by: Christian König Reviewed-by: Alex Deucher Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index cf97a87f76a6..95b248159947 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -656,20 +656,21 @@ static int amdgpu_cs_ib_vm_chunk(struct amdgpu_device *adev, /* Only for UVD/VCE VM emulation */ if (ring->funcs->parse_cs) { + p->job->vm = NULL; for (i = 0; i < p->job->num_ibs; i++) { r = amdgpu_ring_parse_cs(ring, p, i); if (r) return r; } - } - - p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->page_directory); + } else { + p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->page_directory); - r = amdgpu_bo_vm_update_pte(p, vm); - if (!r) - amdgpu_cs_sync_rings(p); + r = amdgpu_bo_vm_update_pte(p, vm); + if (r) + return r; + } - return r; + return amdgpu_cs_sync_rings(p); } static int amdgpu_cs_handle_lockup(struct amdgpu_device *adev, int r) -- cgit v1.2.3 From b5f5acbc87052e1bd8ada6915e1dedd856da767d Mon Sep 17 00:00:00 2001 From: Christian König Date: Wed, 29 Jun 2016 13:26:41 +0200 Subject: drm/amdgpu: fix user fence handling once more MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same problem as with the VM page tables. The user fence address must be determined before the job is scheduled, not when the IB is executed. This fixes a security problem where user fences could be used to overwrite any part of VRAM. Signed-off-by: Christian König Reviewed-by: Chunming Zhou Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 12 ++++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 7 ++----- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 1 - 4 files changed, 9 insertions(+), 14 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index cd7d15941a83..4fc879e594aa 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1278,8 +1278,7 @@ struct amdgpu_job { uint32_t oa_base, oa_size; /* user fence handling */ - struct amdgpu_bo *uf_bo; - uint32_t uf_offset; + uint64_t uf_addr; uint64_t uf_sequence; }; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 95b248159947..475c95f670f8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -216,11 +216,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data) if (ret) goto free_all_kdata; - if (p->uf_entry.robj) { - p->job->uf_bo = amdgpu_bo_ref(p->uf_entry.robj); - p->job->uf_offset = uf_offset; - } - + if (p->uf_entry.robj) + p->job->uf_addr = uf_offset; kfree(chunk_array); return 0; @@ -502,6 +499,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } } + if (p->uf_entry.robj) + p->job->uf_addr += amdgpu_bo_gpu_offset(p->uf_entry.robj); + error_validate: if (r) { amdgpu_vm_move_pt_bos_in_lru(p->adev, &fpriv->vm); @@ -767,7 +767,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev, } /* UVD & VCE fw doesn't support user fences */ - if (parser->job->uf_bo && ( + if (parser->job->uf_addr && ( parser->job->ring->type == AMDGPU_RING_TYPE_UVD || parser->job->ring->type == AMDGPU_RING_TYPE_VCE)) return -EINVAL; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c index 34e35423b78e..0bf6c1b330be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c @@ -203,11 +203,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs, } /* wrap the last IB with fence */ - if (job && job->uf_bo) { - uint64_t addr = amdgpu_bo_gpu_offset(job->uf_bo); - - addr += job->uf_offset; - amdgpu_ring_emit_fence(ring, addr, job->uf_sequence, + if (job && job->uf_addr) { + amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence, AMDGPU_FENCE_FLAG_64BIT); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index b50a8450fcae..87b75d726ae8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -91,7 +91,6 @@ static void amdgpu_job_free_resources(struct amdgpu_job *job) amdgpu_ib_free(job->adev, &job->ibs[i], f); fence_put(job->fence); - amdgpu_bo_unref(&job->uf_bo); amdgpu_sync_free(&job->sync); } -- cgit v1.2.3 From a5fb4ec29c74a16ce1c269e52bc85ca86ee41e81 Mon Sep 17 00:00:00 2001 From: Christian König Date: Wed, 29 Jun 2016 15:10:31 +0200 Subject: drm/amdgpu: earlier free SA resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the time we don't have a fence associated with the resource smaller. Signed-off-by: Christian König Reviewed-by: Chunming Zhou Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 4fc879e594aa..9f70546594a8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -760,6 +760,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs, int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size, struct amdgpu_job **job); +void amdgpu_job_free_resources(struct amdgpu_job *job); void amdgpu_job_free(struct amdgpu_job *job); int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring, struct amd_sched_entity *entity, void *owner, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 475c95f670f8..0db32e3b13c0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -855,6 +855,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, p->fence = fence_get(fence); cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, fence); job->uf_sequence = cs->out.handle; + amdgpu_job_free_resources(job); trace_amdgpu_cs_ioctl(job); amd_sched_entity_push_job(&job->base); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index 347962ea17ab..cdcfda6618d0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -79,7 +79,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size, return r; } -static void amdgpu_job_free_resources(struct amdgpu_job *job) +void amdgpu_job_free_resources(struct amdgpu_job *job) { struct fence *f; unsigned i; @@ -127,6 +127,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring, job->owner = owner; job->ctx = entity->fence_context; *f = fence_get(fence); + amdgpu_job_free_resources(job); amd_sched_entity_push_job(&job->base); return 0; @@ -179,7 +180,6 @@ static struct fence *amdgpu_job_run(struct amd_sched_job *sched_job) err: job->fence = fence; - amdgpu_job_free_resources(job); return fence; } -- cgit v1.2.3 From 595a9cd68c132e474ee5daf97067d4d15c618739 Mon Sep 17 00:00:00 2001 From: Christian König Date: Thu, 30 Jun 2016 10:52:03 +0200 Subject: drm/amdgpu: remove fence parameter from amd_sched_job_init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We return the fence as part of the job structur anyway, no need to do this twice. Signed-off-by: Christian König Reviewed-by: Chunming Zhou Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 8 +++----- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 ++--- drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 4 +--- drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 2 +- 4 files changed, 7 insertions(+), 12 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 0db32e3b13c0..0307ff5887c5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -836,15 +836,13 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, { struct amdgpu_ring *ring = p->job->ring; struct amd_sched_entity *entity = &p->ctx->rings[ring->idx].entity; - struct fence *fence; struct amdgpu_job *job; int r; job = p->job; p->job = NULL; - r = amd_sched_job_init(&job->base, &ring->sched, - entity, p->filp, &fence); + r = amd_sched_job_init(&job->base, &ring->sched, entity, p->filp); if (r) { amdgpu_job_free(job); return r; @@ -852,8 +850,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, job->owner = p->filp; job->ctx = entity->fence_context; - p->fence = fence_get(fence); - cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, fence); + p->fence = fence_get(&job->base.s_fence->finished); + cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, p->fence); job->uf_sequence = cs->out.handle; amdgpu_job_free_resources(job); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index cdcfda6618d0..327f4df55c86 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -113,20 +113,19 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring, struct amd_sched_entity *entity, void *owner, struct fence **f) { - struct fence *fence; int r; job->ring = ring; if (!f) return -EINVAL; - r = amd_sched_job_init(&job->base, &ring->sched, entity, owner, &fence); + r = amd_sched_job_init(&job->base, &ring->sched, entity, owner); if (r) return r; job->owner = owner; job->ctx = entity->fence_context; - *f = fence_get(fence); + *f = fence_get(&job->base.s_fence->finished); amdgpu_job_free_resources(job); amd_sched_entity_push_job(&job->base); diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c index 2baedc4e667d..3405372cda0c 100644 --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c @@ -403,7 +403,7 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job) int amd_sched_job_init(struct amd_sched_job *job, struct amd_gpu_scheduler *sched, struct amd_sched_entity *entity, - void *owner, struct fence **fence) + void *owner) { job->sched = sched; job->s_entity = entity; @@ -415,8 +415,6 @@ int amd_sched_job_init(struct amd_sched_job *job, INIT_LIST_HEAD(&job->node); INIT_DELAYED_WORK(&job->work_tdr, amd_sched_job_timedout); - if (fence) - *fence = &job->s_fence->finished; return 0; } diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h index 3e989b153282..9024dc5592e3 100644 --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h @@ -151,5 +151,5 @@ void amd_sched_fence_finished(struct amd_sched_fence *fence); int amd_sched_job_init(struct amd_sched_job *job, struct amd_gpu_scheduler *sched, struct amd_sched_entity *entity, - void *owner, struct fence **fence); + void *owner); #endif -- cgit v1.2.3