Re: [RFC PATCH 1/4] drm/amdkfd: Improve amdgpu_vm_handle_moved

Am 17.03.22 um 01:20 schrieb Felix Kuehling:

Let amdgpu_vm_handle_moved update all BO VA mappings of BOs reserved by the caller. This will be useful for handling extra BO VA mappings in KFD VMs that are managed through the render node API.

Yes, that change is on my TODO list for quite a while as well.

TODO: This may also allow simplification of amdgpu_cs_vm_handling. See the TODO comment in the code.

No, that won't work just yet.

We need to change the TLB flush detection for that, but I'm already working on those as well.

Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com

Please update the TODO, with that done: Reviewed-by: Christian König christian.koenig@amd.com

---

drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 +++++- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 18 +++++++++++++----- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ++- 4 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index d162243d8e78..10941f0d8dde 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -826,6 +826,10 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) return r; }

• /* TODO: Is this loop still needed, or could this be handled by

• * amdgpu_vm_handle_moved, now that it can handle all BOs that are
  

• * reserved under p->ticket?
  

• */
  

  amdgpu_bo_list_for_each_entry(e, p->bo_list) { /* ignore duplicates */ bo = ttm_to_amdgpu_bo(e->tv.bo);

@@ -845,7 +849,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) return r; }

• r = amdgpu_vm_handle_moved(adev, vm);

• r = amdgpu_vm_handle_moved(adev, vm, &p->ticket); if (r) return r;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index 579adfafe4d0..50805613c38c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -414,7 +414,7 @@ amdgpu_dma_buf_move_notify(struct dma_buf_attachment *attach)

r = amdgpu_vm_clear_freed(adev, vm, NULL);
if (!r)

• 	r = amdgpu_vm_handle_moved(adev, vm);
  

• 	r = amdgpu_vm_handle_moved(adev, vm, ticket);
  
  

  if (r && r != -EBUSY) DRM_ERROR("Failed to invalidate VM page tables (%d))\n",

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index fc4563cf2828..726b42c6d606 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -2190,11 +2190,12 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,

• PTs have to be reserved!

*/ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,

• 	   struct amdgpu_vm *vm)
  

• 	   struct amdgpu_vm *vm,
  

• 	   struct ww_acquire_ctx *ticket)
  

  { struct amdgpu_bo_va *bo_va, *tmp; struct dma_resv *resv;

• bool clear;

• bool clear, unlock; int r;

  list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {

@@ -2212,17 +2213,24 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev, spin_unlock(&vm->invalidated_lock);

/* Try to reserve the BO to avoid clearing its ptes */

• if (!amdgpu_vm_debug && dma_resv_trylock(resv))
  

• if (!amdgpu_vm_debug && dma_resv_trylock(resv)) {
  clear = false;
  

• 	unlock = true;
  

• /* The caller is already holding the reservation lock */
  

• } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
  

• 	clear = false;
  

• 	unlock = false;
  

  /* Somebody else is using the BO right now */

• else
  

• } else {
  clear = true;
  

• 	unlock = false;
  

• }
  
  

  r = amdgpu_vm_bo_update(adev, bo_va, clear, NULL); if (r) return r;

• if (!clear)
  

• if (unlock)
  dma_resv_unlock(resv);
  

  spin_lock(&vm->invalidated_lock); }

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index a40a6a993bb0..120a76aaae75 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -396,7 +396,8 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct dma_fence **fence); int amdgpu_vm_handle_moved(struct amdgpu_device *adev,

• 	   struct amdgpu_vm *vm);
  

• 	   struct amdgpu_vm *vm,
  

• 	   struct ww_acquire_ctx *ticket);
  

  int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev, struct amdgpu_device *bo_adev, struct amdgpu_vm *vm, bool immediate,