[PATCH v2 06/10] drm/amdgpu: DMA map/unmap when updating GPU mappings

Zeng, Oak Oak.Zeng at amd.com
Tue Apr 27 00:23:31 UTC 2021



Regards,
Oak 

 

On 2021-04-21, 9:31 PM, "dri-devel on behalf of Felix Kuehling" <dri-devel-bounces at lists.freedesktop.org on behalf of Felix.Kuehling at 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.Kuehling at 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.
    +	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 at lists.freedesktop.org
    https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Coak.zeng%40amd.com%7C867f4b956e9d4d2e539208d9052e6140%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637546519028781182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=VM31e7X1NRqmm7u%2BLTzCqTO0c2fHa0j6PmIXT24eJY8%3D&reserved=0



More information about the dri-devel mailing list