It's mandatory that we have this check inside the VM handling or otherwise page table allocation and filling PTEs doesn't work correctly.
This allows to remove the buggy SVM range lock, but that's not part of this patch set. Only compile tested! Signed-off-by: Christian König <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 ++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++- .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h | 10 ++++++ drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 32 ++++++++----------- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index d4e207db83c9..a368327cc025 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1093,6 +1093,7 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params, * @vram_base: base for vram mappings * @res: ttm_resource to map * @pages_addr: DMA addresses to use for mapping + * @hmm_range: to check validity of DMA addresses * @fence: optional resulting fence * * Fill in the page table entries between @start and @last. Allocate and free @@ -1106,7 +1107,9 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_sync *sync, uint64_t start, uint64_t last, uint64_t flags, uint64_t offset, uint64_t vram_base, struct ttm_resource *res, - dma_addr_t *pages_addr, struct dma_fence **fence) + dma_addr_t *pages_addr, + struct amdgpu_hmm_range *hmm_range, + struct dma_fence **fence) { struct amdgpu_vm_tlb_seq_struct *tlb_cb; struct amdgpu_vm_update_params params; @@ -1139,6 +1142,7 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm, params.adev = adev; params.vm = vm; params.pages_addr = pages_addr; + params.hmm_range = hmm_range; params.needs_flush = flush_tlb; params.override_pte = allow_override && adev->gmc.override_pte; INIT_LIST_HEAD(¶ms.tlb_flush_waitlist); @@ -1432,7 +1436,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va, r = amdgpu_vm_map_range(adev, vm, flush_tlb, !uncached, &sync, mapping->start, mapping->last, update_flags, mapping->offset, - vram_base, mem, pages_addr, + vram_base, mem, pages_addr, NULL, last_update); if (r) goto error_free; @@ -1634,7 +1638,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev, r = amdgpu_vm_map_range(adev, vm, true, false, &sync, mapping->start, mapping->last, - 0, 0, 0, NULL, NULL, &f); + 0, 0, 0, NULL, NULL, NULL, &f); amdgpu_vm_free_mapping(adev, vm, mapping, f); if (r) { dma_fence_put(f); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 7db3c3577949..ec1196d390bb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -459,7 +459,9 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_sync *sync, uint64_t start, uint64_t last, uint64_t flags, uint64_t offset, uint64_t vram_base, struct ttm_resource *res, - dma_addr_t *pages_addr, struct dma_fence **fence); + dma_addr_t *pages_addr, + struct amdgpu_hmm_range *hmm_range, + struct dma_fence **fence); int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_sync *sync, uint64_t start, uint64_t last, uint64_t flags, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h index 29c74920f3b0..d47951437f0d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h @@ -26,6 +26,7 @@ #include <linux/types.h> #include <linux/list.h> +#include "amdgpu_hmm.h" #include "amdgpu_vm.h" struct amdgpu_device; @@ -72,6 +73,13 @@ struct amdgpu_vm_update_params { */ dma_addr_t *pages_addr; + /** + * @hmm_range: + * + * Used to check the validity of pages_addr. + */ + struct amdgpu_hmm_range *hmm_range; + /** * @job: job to used for hw submission */ @@ -153,6 +161,8 @@ static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params *p) p->saved_flags = memalloc_noreclaim_save(); if (p->vm->evicting) return -EBUSY; + if (p->hmm_range && !amdgpu_hmm_range_valid(p->hmm_range)) + return -EAGAIN; return 0; } diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index d0ea20dea3e1..d506afff684e 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c @@ -1432,7 +1432,8 @@ svm_range_unmap_from_gpus(struct svm_range *prange, unsigned long start, static int svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange, unsigned long offset, unsigned long npages, bool readonly, - dma_addr_t *dma_addr, struct amdgpu_device *bo_adev, + dma_addr_t *dma_addr, struct amdgpu_hmm_range *hmm_range, + struct amdgpu_device *bo_adev, struct dma_fence **fence, bool flush_tlb) { struct amdgpu_device *adev = pdd->dev->adev; @@ -1492,7 +1493,7 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange, gpu_start, gpu_end, pte_flags, (last_start - prange->start) << PAGE_SHIFT, bo_adev ? bo_adev->vm_manager.vram_base_offset : 0, - NULL, dma_addr, &vm->last_update); + NULL, dma_addr, hmm_range, &vm->last_update); for (j = last_start - prange->start; j <= i; j++) dma_addr[j] |= last_domain; @@ -1519,7 +1520,9 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange, } static int -svm_range_map_to_gpus(struct svm_range *prange, unsigned long offset, +svm_range_map_to_gpus(struct svm_range *prange, + struct amdgpu_hmm_range *hmm_range, + unsigned long offset, unsigned long npages, bool readonly, unsigned long *bitmap, bool wait, bool flush_tlb) { @@ -1553,7 +1556,7 @@ svm_range_map_to_gpus(struct svm_range *prange, unsigned long offset, } r = svm_range_map_to_gpu(pdd, prange, offset, npages, readonly, - prange->dma_addr[gpuidx], + prange->dma_addr[gpuidx], hmm_range, bo_adev, wait ? &fence : NULL, flush_tlb); if (r) @@ -1823,18 +1826,6 @@ static int svm_range_validate_and_map(struct mm_struct *mm, svm_range_lock(prange); - /* Free backing memory of hmm_range if it was initialized - * Override return value to TRY AGAIN only if prior returns - * were successful - */ - if (range && !amdgpu_hmm_range_valid(range) && !r) { - pr_debug("hmm update the range, need validate again\n"); - r = -EAGAIN; - } - - /* Free the hmm range */ - amdgpu_hmm_range_free(range); - if (!r && !list_empty(&prange->child_list)) { pr_debug("range split by unmap in parallel, validate again\n"); r = -EAGAIN; @@ -1846,11 +1837,16 @@ static int svm_range_validate_and_map(struct mm_struct *mm, if (map_start_vma <= map_last_vma) { offset = map_start_vma - prange->start; npages = map_last_vma - map_start_vma + 1; - r = svm_range_map_to_gpus(prange, offset, npages, readonly, - ctx->bitmap, wait, flush_tlb); + r = svm_range_map_to_gpus(prange, range, offset, + npages, readonly, + ctx->bitmap, wait, + flush_tlb); } } + /* Free the hmm range */ + amdgpu_hmm_range_free(range); + if (!r && next == end) prange->mapped_to_gpu = true; -- 2.43.0
