Am 2021-04-26 um 8:23 p.m. schrieb Zeng, Oak:
> Regards,
> Oak 
>
>  
>
> On 2021-04-21, 9:31 PM, "dri-devel on behalf of Felix Kuehling" 
> <dri-devel-boun...@lists.freedesktop.org on behalf of felix.kuehl...@amd.com> 
> wrote:
>
>     DMA map kfd_mem_attachments in update_gpuvm_pte. This function is called
>     with the BO and page tables reserved, so we can safely update the DMA
>     mapping.
>
>     DMA unmap when a BO is unmapped from a GPU and before updating mappings
>     in restore workers.
>
>     Signed-off-by: Felix Kuehling <felix.kuehl...@amd.com>
>     ---
>      .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 56 ++++++++++---------
>      1 file changed, 29 insertions(+), 27 deletions(-)
>
>     diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>     index 49d1af4aa5f1..7d25d886b98c 100644
>     --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>     +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>     @@ -961,11 +961,12 @@ static int unreserve_bo_and_vms(struct 
> bo_vm_reservation_context *ctx,
>       return ret;
>      }
>
>     -static int unmap_bo_from_gpuvm(struct amdgpu_device *adev,
>     +static void unmap_bo_from_gpuvm(struct kgd_mem *mem,
>                               struct kfd_mem_attachment *entry,
>                               struct amdgpu_sync *sync)
>      {
>       struct amdgpu_bo_va *bo_va = entry->bo_va;
>     + struct amdgpu_device *adev = entry->adev;
>       struct amdgpu_vm *vm = bo_va->base.vm;
>
>       amdgpu_vm_bo_unmap(adev, bo_va, entry->va);
>     @@ -974,15 +975,20 @@ static int unmap_bo_from_gpuvm(struct amdgpu_device 
> *adev,
>
>       amdgpu_sync_fence(sync, bo_va->last_pt_update);
>
>     - return 0;
>     + kfd_mem_dmaunmap_attachment(mem, entry);
>      }
>
>     -static int update_gpuvm_pte(struct amdgpu_device *adev,
>     -         struct kfd_mem_attachment *entry,
>     -         struct amdgpu_sync *sync)
>     +static int update_gpuvm_pte(struct kgd_mem *mem,
>     +                     struct kfd_mem_attachment *entry,
>     +                     struct amdgpu_sync *sync)
>      {
>     - int ret;
>       struct amdgpu_bo_va *bo_va = entry->bo_va;
>     + struct amdgpu_device *adev = entry->adev;
>     + int ret;
>     +
>     + ret = kfd_mem_dmamap_attachment(mem, entry);
> Should the dma mapping be done in the kfd_mem_attach function on a memory 
> object is attached to a vm the first time? Since each memory object can be 
> mapped to many GPU or many VMs, by doing dma mapping the first it is attached 
> can simplify the logics. Or even simpler, maybe we can just just dma map when 
> a memory object is created - it wastes some iommu page table entry but really 
> simplify the logic in this patch series. I found this series is not very easy 
> to understand.

The DMA mapping must be updated every time the physical memory
allocation changes, e.g. after a BO was evicted and restored. Basically,
if the physical pages of the BO change, we need to update the DMA
mapping to point to those new pages. Therefore I added this in the
update_gpu_vm_pte function, which is called after a BO has been
validated the first time, or revalidated after an eviction.

You'll also see that I call dmaunmap in the re-validation cases (in the
restore workers below) to ensure that we don't leak DMA mappings.

Regards,
  Felix


>     + if (ret)
>     +         return ret;
>
>       /* Update the page tables  */
>       ret = amdgpu_vm_bo_update(adev, bo_va, false);
>     @@ -994,14 +1000,15 @@ static int update_gpuvm_pte(struct amdgpu_device 
> *adev,
>       return amdgpu_sync_fence(sync, bo_va->last_pt_update);
>      }
>
>     -static int map_bo_to_gpuvm(struct amdgpu_device *adev,
>     -         struct kfd_mem_attachment *entry, struct amdgpu_sync *sync,
>     -         bool no_update_pte)
>     +static int map_bo_to_gpuvm(struct kgd_mem *mem,
>     +                    struct kfd_mem_attachment *entry,
>     +                    struct amdgpu_sync *sync,
>     +                    bool no_update_pte)
>      {
>       int ret;
>
>       /* Set virtual address for the allocation */
>     - ret = amdgpu_vm_bo_map(adev, entry->bo_va, entry->va, 0,
>     + ret = amdgpu_vm_bo_map(entry->adev, entry->bo_va, entry->va, 0,
>                              amdgpu_bo_size(entry->bo_va->base.bo),
>                              entry->pte_flags);
>       if (ret) {
>     @@ -1013,7 +1020,7 @@ static int map_bo_to_gpuvm(struct amdgpu_device 
> *adev,
>       if (no_update_pte)
>               return 0;
>
>     - ret = update_gpuvm_pte(adev, entry, sync);
>     + ret = update_gpuvm_pte(mem, entry, sync);
>       if (ret) {
>               pr_err("update_gpuvm_pte() failed\n");
>               goto update_gpuvm_pte_failed;
>     @@ -1022,7 +1029,7 @@ static int map_bo_to_gpuvm(struct amdgpu_device 
> *adev,
>       return 0;
>
>      update_gpuvm_pte_failed:
>     - unmap_bo_from_gpuvm(adev, entry, sync);
>     + unmap_bo_from_gpuvm(mem, entry, sync);
>       return ret;
>      }
>
>     @@ -1596,7 +1603,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>               pr_debug("\t map VA 0x%llx - 0x%llx in entry %p\n",
>                        entry->va, entry->va + bo_size, entry);
>
>     -         ret = map_bo_to_gpuvm(adev, entry, ctx.sync,
>     +         ret = map_bo_to_gpuvm(mem, entry, ctx.sync,
>                                     is_invalid_userptr);
>               if (ret) {
>                       pr_err("Failed to map bo to gpuvm\n");
>     @@ -1635,7 +1642,6 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>      int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
>               struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv)
>      {
>     - struct amdgpu_device *adev = get_amdgpu_device(kgd);
>       struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
>       struct amdkfd_process_info *process_info = avm->process_info;
>       unsigned long bo_size = mem->bo->tbo.base.size;
>     @@ -1670,13 +1676,8 @@ int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
>               pr_debug("\t unmap VA 0x%llx - 0x%llx from entry %p\n",
>                        entry->va, entry->va + bo_size, entry);
>
>     -         ret = unmap_bo_from_gpuvm(adev, entry, ctx.sync);
>     -         if (ret == 0) {
>     -                 entry->is_mapped = false;
>     -         } else {
>     -                 pr_err("failed to unmap VA 0x%llx\n", mem->va);
>     -                 goto unreserve_out;
>     -         }
>     +         unmap_bo_from_gpuvm(mem, entry, ctx.sync);
>     +         entry->is_mapped = false;
>
>               mem->mapped_to_gpu_memory--;
>               pr_debug("\t DEC mapping count %d\n",
>     @@ -2053,9 +2054,8 @@ static int validate_invalid_user_pages(struct 
> amdkfd_process_info *process_info)
>                       if (!attachment->is_mapped)
>                               continue;
>
>     -                 ret = update_gpuvm_pte((struct amdgpu_device *)
>     -                                        attachment->adev,
>     -                                        attachment, &sync);
>     +                 kfd_mem_dmaunmap_attachment(mem, attachment);
>     +                 ret = update_gpuvm_pte(mem, attachment, &sync);
>                       if (ret) {
>                               pr_err("%s: update PTE failed\n", __func__);
>                               /* make sure this gets validated again */
>     @@ -2257,9 +2257,11 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void 
> *info, struct dma_fence **ef)
>                       goto validate_map_fail;
>               }
>               list_for_each_entry(attachment, &mem->attachments, list) {
>     -                 ret = update_gpuvm_pte((struct amdgpu_device *)
>     -                                       attachment->adev, attachment,
>     -                                       &sync_obj);
>     +                 if (!attachment->is_mapped)
>     +                         continue;
>     +
>     +                 kfd_mem_dmaunmap_attachment(mem, attachment);
>     +                 ret = update_gpuvm_pte(mem, attachment, &sync_obj);
>                       if (ret) {
>                               pr_debug("Memory eviction: update PTE failed. 
> Try again\n");
>                               goto validate_map_fail;
>     -- 
>     2.31.1
>
>     _______________________________________________
>     dri-devel mailing list
>     dri-devel@lists.freedesktop.org
>     https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to