<html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 2024-01-26 09:59, Christian König wrote: </div> <blockquote type="cite" cite="mid:9c59557b-f6e3-45a9-bd4d-bddea9339e92@amd.com">Am 26.01.24 um 15:38 schrieb Philip Yang: <blockquote type="cite">svm range support partial migration and mapping update, for size 4MB virtual address 4MB alignment and physical address continuous range, if mapping to GPU with fs=10, after updating mapping of the first 2MB, if the second 2MB mapping fs=10 in cache TLB, this causes the first 2MB access to the stale mapping. </blockquote> Well that sounds fishy. When that happens with (for example) 4MiB and 2MiB, why doesn't it happen with 8KiB and 4KiB as well? </blockquote> unmap svm range is aligned to granularity size, if the range size is 8KB (all within one 2MB granularity range), it will be mapped/unmapped as 8KB, even if only 4KB is migrated. This is handled in another patch series "amd/amdkfd: Unmap range from GPU based on granularity". Regards, Philip <blockquote type="cite" cite="mid:9c59557b-f6e3-45a9-bd4d-bddea9339e92@amd.com"> Christian. <blockquote type="cite"> Limit the maximum fragment size to granularity size, 2MB by default, with the mapping and unmapping based on gramularity size, to solve this issue. The change is only for SVM map/unmap range, no change for gfx and legacy API path. Signed-off-by: Philip Yang <a class="moz-txt-link-rfc2396E" href="mailto:Philip.Yang@amd.com"><Philip.Yang@amd.com></a> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 12 +++++++----- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 22 ++++++++++++---------- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 9 +++++---- 4 files changed, 26 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index ed4a8c5d26d7..a2bef94cb959 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -897,6 +897,7 @@ static void amdgpu_vm_tlb_seq_cb(struct dma_fence *fence, * @res: ttm_resource to map * @pages_addr: DMA addresses to use for mapping * @fence: optional resulting fence + * @frag_size: max map fragment size * * Fill in the page table entries between @start and @last. * @@ -908,7 +909,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct dma_resv *resv, 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) + struct dma_fence **fence, unsigned int frag_size) { struct amdgpu_vm_update_params params; struct amdgpu_vm_tlb_seq_struct *tlb_cb; @@ -1016,7 +1017,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm, } tmp = start + num_entries; - r = amdgpu_vm_ptes_update(&params, start, tmp, addr, flags); + r = amdgpu_vm_ptes_update(&params, start, tmp, addr, flags, frag_size); if (r) goto error_free; @@ -1197,7 +1198,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va, !uncached, resv, mapping->start, mapping->last, update_flags, mapping->offset, vram_base, mem, pages_addr, - last_update); + last_update, 0); if (r) return r; } @@ -1392,7 +1393,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev, r = amdgpu_vm_update_range(adev, vm, false, false, true, false, resv, mapping->start, mapping->last, init_pte_value, 0, 0, NULL, NULL, - &f); + &f, 0); amdgpu_vm_free_mapping(adev, vm, mapping, f); if (r) { dma_fence_put(f); @@ -2733,7 +2734,8 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid, } r = amdgpu_vm_update_range(adev, vm, true, false, false, false, - NULL, addr, addr, flags, value, 0, NULL, NULL, NULL); + NULL, addr, addr, flags, value, 0, NULL, NULL, + NULL, 0); if (r) goto error_unlock; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 666698a57192..b34466b5086f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -465,7 +465,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct dma_resv *resv, 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); + struct dma_fence **fence, unsigned int frag_size); int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va, bool clear); @@ -531,7 +531,7 @@ int amdgpu_vm_pde_update(struct amdgpu_vm_update_params *params, struct amdgpu_vm_bo_base *entry); int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params, uint64_t start, uint64_t end, - uint64_t dst, uint64_t flags); + uint64_t dst, uint64_t flags, unsigned int frag_size); void amdgpu_vm_pt_free_work(struct work_struct *work); #if defined(CONFIG_DEBUG_FS) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index a160265ddc07..4647f700f1c6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c @@ -860,12 +860,14 @@ static void amdgpu_vm_pte_update_flags(struct amdgpu_vm_update_params *params, * @flags: hw mapping flags * @frag: resulting fragment size * @frag_end: end of this fragment + * @frag_size: max map fragment size * * Returns the first possible fragment for the start and end address. */ static void amdgpu_vm_pte_fragment(struct amdgpu_vm_update_params *params, uint64_t start, uint64_t end, uint64_t flags, - unsigned int *frag, uint64_t *frag_end) + unsigned int *frag, uint64_t *frag_end, + unsigned int frag_size) { /** * The MC L1 TLB supports variable sized pages, based on a fragment @@ -893,7 +895,7 @@ static void amdgpu_vm_pte_fragment(struct amdgpu_vm_update_params *params, if (params->adev->asic_type < CHIP_VEGA10) max_frag = params->adev->vm_manager.fragment_size; else - max_frag = 31; + max_frag = frag_size ? frag_size : 31; /* system pages are non continuously */ if (params->pages_addr) { @@ -904,12 +906,10 @@ static void amdgpu_vm_pte_fragment(struct amdgpu_vm_update_params *params, /* This intentionally wraps around if no bit is set */ *frag = min_t(unsigned int, ffs(start) - 1, fls64(end - start) - 1); - if (*frag >= max_frag) { + if (*frag >= max_frag) *frag = max_frag; - *frag_end = end & ~((1ULL << max_frag) - 1); - } else { - *frag_end = start + (1 << *frag); - } + + *frag_end = start + (1 << *frag); } /** @@ -920,6 +920,7 @@ static void amdgpu_vm_pte_fragment(struct amdgpu_vm_update_params *params, * @end: end of GPU address range * @dst: destination address to map to, the next dst inside the function * @flags: mapping flags + * @frag_size: max map fragment size * * Update the page tables in the range @start - @end. * @@ -928,7 +929,7 @@ static void amdgpu_vm_pte_fragment(struct amdgpu_vm_update_params *params, */ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params, uint64_t start, uint64_t end, - uint64_t dst, uint64_t flags) + uint64_t dst, uint64_t flags, unsigned int frag_size) { struct amdgpu_device *adev = params->adev; struct amdgpu_vm_pt_cursor cursor; @@ -938,7 +939,7 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params, /* figure out the initial fragment */ amdgpu_vm_pte_fragment(params, frag_start, end, flags, &frag, - &frag_end); + &frag_end, frag_size); /* walk over the address space and update the PTs */ amdgpu_vm_pt_start(adev, params->vm, start, &cursor); @@ -1040,7 +1041,8 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params, if (frag_start >= frag_end) { /* figure out the next fragment */ amdgpu_vm_pte_fragment(params, frag_start, end, - flags, &frag, &frag_end); + flags, &frag, &frag_end, + frag_size); if (frag < shift) break; } diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index ed35a490fd9e..d71b2c8bf51a 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c @@ -1488,7 +1488,7 @@ svm_range_get_pte_flags(struct kfd_node *node, static int svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm, uint64_t start, uint64_t last, - struct dma_fence **fence) + struct dma_fence **fence, unsigned int frag_size) { uint64_t init_pte_value = 0; @@ -1496,7 +1496,7 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm, return amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL, start, last, init_pte_value, 0, 0, NULL, NULL, - fence); + fence, frag_size); } /** @@ -1579,7 +1579,7 @@ svm_range_unmap_from_gpus(struct svm_range *prange, unsigned long start, r = svm_range_unmap_from_gpu(pdd->dev->adev, drm_priv_to_vm(pdd->drm_priv), - start, last, &fence); + start, last, &fence, prange->granularity); if (r) break; @@ -1647,7 +1647,8 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange, 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, &vm->last_update, + prange->granularity); for (j = last_start - prange->start; j <= i; j++) dma_addr[j] |= last_domain; </blockquote> </blockquote> </body> </html>