On 6/12/2026 8:21 PM, Christian König wrote:
On 6/12/26 11:09, Huang Rui wrote:From: Honglei Huang <[email protected]> Implement GPU page table mapping in amdgpu_svm_range.c: - amdgpu_svm_range_pages_valid: check notifier sequence validity - amdgpu_svm_range_is_valid: validate gpu_mapped and attr_flags against the requested attribute set, under MMU notifier lock. - amdgpu_svm_range_zap_ptes: clear GPU PTEs via amdgpu_vm_update_range - amdgpu_svm_range_attr_pte_flags: compute PTE flags per GC IP version - amdgpu_svm_range_lock_vm_pd: acquire VM page directory by drm_exec - amdgpu_svm_range_update_gpu_range: DMA segment coalescing; recomputes PTE flags per segment from entry->proto via attr_pte_flags, then programs PTEs under notifier lock - amdgpu_svm_range_find_or_insert: wrapper of drm gpu SVM find or insert API. - amdgpu_svm_range_get_pages: wrapper of drm gpu SVM get pages. - amdgpu_svm_range_evict: thin wrapper of drm_gpusvm_range_evict - amdgpu_svm_range_update_mapping: full pipeline (lock PD, validate pages, program PTEs, update PDEs, flush TLB, record attr_flags and mapped state); takes attrs + read_only and forwards them to update_gpu_range - amdgpu_svm_range_map_attrs: cursor based iteration across attr tree, creating/mapping ranges - Add to amdgpu_svm.h driver private interconnect tags used in drm_pagemap_addr.proto: AMDGPU_INTERCONNECT_VRAM = DRM_INTERCONNECT_DRIVER AMDGPU_INTERCONNECT_P2P = AMDGPU_INTERCONNECT_VRAM + 1 Signed-off-by: Honglei Huang <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_svm.h | 4 + drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c | 488 ++++++++++++++++++ 2 files changed, 492 insertions(+) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.h index 9884a1f834d67..ac84b80f750f8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.h @@ -27,6 +27,7 @@#include <drm/amdgpu_drm.h>#include <drm/drm_gpusvm.h> +#include <drm/drm_pagemap.h> #include <linux/atomic.h> #include <linux/kref.h> #include <linux/list.h> @@ -44,6 +45,9 @@ struct amdgpu_svm_attrs; struct drm_device; struct drm_file;+#define AMDGPU_INTERCONNECT_VRAM DRM_INTERCONNECT_DRIVER+#define AMDGPU_INTERCONNECT_P2P (AMDGPU_INTERCONNECT_VRAM + 1) + enum amdgpu_svm_xnack_mode { AMDGPU_SVM_XNACK_OFF, AMDGPU_SVM_XNACK_ON, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c new file mode 100644 index 0000000000000..e1b392b2cf56a --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c @@ -0,0 +1,488 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT +/* + * Copyright 2026 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +#include "amdgpu_svm.h" +#include "amdgpu_svm_attr.h" +#include "amdgpu_svm_range.h" +#include "amdgpu_svm_fault.h" +#include "amdgpu.h" +#include "amdgpu_vm.h" + +#include <drm/drm_exec.h> +#include <drm/drm_pagemap.h> + +#include <linux/mmu_notifier.h> +#include <uapi/linux/kfd_ioctl.h> + +bool +amdgpu_svm_range_pages_valid(struct amdgpu_svm *svm, + struct amdgpu_svm_range *range) +{ + struct drm_gpusvm_range *base = &range->base; + + lockdep_assert_held(&svm->gpusvm.notifier_lock); + + if (base->pages.flags.unmapped || base->pages.flags.partial_unmap) + return false; + + return drm_gpusvm_range_pages_valid(&svm->gpusvm, base); +} + +bool amdgpu_svm_range_is_valid(struct amdgpu_svm *svm, + struct amdgpu_svm_range *range, + const struct amdgpu_svm_attrs *attrs) +{ + bool valid; + + drm_gpusvm_notifier_lock(&svm->gpusvm); + valid = range->gpu_mapped && + range->attr_flags == attrs->flags && + amdgpu_svm_range_pages_valid(svm, range); + drm_gpusvm_notifier_unlock(&svm->gpusvm); + + return valid;That looks completely broken to me. The valid state looses meaning as soon as the lock is dropped.
Got it will drop the notifier lock here, but actaully the usage just mirror the xe_svm_range_is_valid, it check the xe_svm_range_has_pagemap under the notifier lock, and according to the documents in drm_gpusvm.c:1311:
... * This function determines if a GPU SVM range pages are valid. Expected be* called holding gpusvm->notifier_lock and as the last step before committing a * GPU binding. This is akin to a notifier seqno check in the HMM documentation
...
static bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_pages *svm_pages)
{
lockdep_assert_held(&gpusvm->notifier_lock);
return svm_pages->flags.has_devmem_pages ||
svm_pages->flags.has_dma_mapping;
}the valid check must be in notifier lock, or the lockdep in frame work will go wrong.
+} + + +int +amdgpu_svm_range_zap_ptes(struct amdgpu_svm *svm, + struct amdgpu_svm_range *range, + unsigned long start_page, + unsigned long last_page) +{ + struct dma_fence *fence = NULL; + unsigned int flags; + int ret; + + if (last_page < start_page) + return 0; + + flags = memalloc_noreclaim_save(); + ret = amdgpu_vm_update_range(svm->adev, svm->vm, false, true, true, false, + NULL, start_page, last_page, 0, 0, 0, NULL, + NULL, &fence); + memalloc_noreclaim_restore(flags);That's the same nonsense we already had in the existing KFD implementation. The whole patch set needs to be rebased on top of my patches to fix this.+ + if (!ret && fence) {That needs to block for the fence even if the return value is non zero.+ ret = dma_fence_wait(fence, false);+ if (ret < 0) + AMDGPU_SVM_TRACE( + "notifier unmap fence wait failed: ret=%d [0x%lx-0x%lx]-0x%lx\n", + ret, start_page, last_page, + last_page - start_page + 1);Drop that check and warning.+ } + + dma_fence_put(fence); + return ret; +} + +uint64_t +amdgpu_svm_range_attr_pte_flags(struct amdgpu_svm *svm, + const struct amdgpu_svm_attrs *attrs, + bool read_only, + enum drm_interconnect_protocol proto) +{ + uint32_t flags = attrs->flags; + uint32_t mapping_flags = 0; + uint32_t gc_ip_version = amdgpu_ip_version(svm->adev, GC_HWIP, 0); + uint64_t pte_flags; + bool snoop = proto != AMDGPU_INTERCONNECT_VRAM; + bool coherent = flags & (AMDGPU_SVM_ATTR_BIT_COHERENT | + AMDGPU_SVM_ATTR_BIT_EXT_COHERENT); + bool ext_coherent = flags & AMDGPU_SVM_ATTR_BIT_EXT_COHERENT; + unsigned int mtype_local, mtype_remote; + bool is_aid_a1; + bool is_local = (proto == AMDGPU_INTERCONNECT_VRAM); + bool is_vram = is_local || (proto == AMDGPU_INTERCONNECT_P2P); + + switch (gc_ip_version) { + case IP_VERSION(9, 4, 1): + case IP_VERSION(9, 4, 2): + if (is_local) { + mapping_flags |= coherent ? + AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW; + /* 9.4.2 local VRAM with XGMI keeps snoop */ + if (gc_ip_version == IP_VERSION(9, 4, 2) && + svm->adev->gmc.xgmi.connected_to_cpu) + snoop = true; + } else { + mapping_flags |= coherent ? + AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC; + /* TODO: migration: re enable snoop for same hive */ + } + break; + case IP_VERSION(9, 4, 3): + case IP_VERSION(9, 4, 4): + case IP_VERSION(9, 5, 0): + if (ext_coherent) + mtype_local = AMDGPU_VM_MTYPE_CC; + else + mtype_local = amdgpu_mtype_local == 1 ? AMDGPU_VM_MTYPE_NC : + amdgpu_mtype_local == 2 ? AMDGPU_VM_MTYPE_CC : + AMDGPU_VM_MTYPE_RW; + snoop = true; + if (is_vram) { + if (is_local) { + /* local HBM region close to partition */ + mapping_flags |= mtype_local; + } else if (!ext_coherent) { + /* TODO: add same hive check */ + mapping_flags |= AMDGPU_VM_MTYPE_NC; + } else if (gc_ip_version < IP_VERSION(9, 5, 0)) { + /* TODO: add same hive check */ + mapping_flags |= AMDGPU_VM_MTYPE_UC; + } else { + mapping_flags |= ext_coherent ? + AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC; + } + } else if (svm->adev->flags & AMD_IS_APU) { + /* On NUMA systems, locality is determined per-page + * in amdgpu_gmc_override_vm_pte_flags. + */ + if (num_possible_nodes() <= 1) + mapping_flags |= mtype_local; + else + mapping_flags |= ext_coherent ? + AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC; + } else { + if (gc_ip_version < IP_VERSION(9, 5, 0) || ext_coherent) + mapping_flags |= AMDGPU_VM_MTYPE_UC; + else + mapping_flags |= AMDGPU_VM_MTYPE_NC; + } + break; + case IP_VERSION(12, 0, 0): + case IP_VERSION(12, 0, 1): + mapping_flags |= AMDGPU_VM_MTYPE_NC; + break; + case IP_VERSION(12, 1, 0): + is_aid_a1 = (svm->adev->rev_id & 0x10); + mtype_local = amdgpu_mtype_local == 0 ? AMDGPU_VM_MTYPE_RW : + amdgpu_mtype_local == 1 ? AMDGPU_VM_MTYPE_NC : + is_aid_a1 ? AMDGPU_VM_MTYPE_RW : AMDGPU_VM_MTYPE_NC; + mtype_remote = is_aid_a1 ? AMDGPU_VM_MTYPE_NC : AMDGPU_VM_MTYPE_UC; + snoop = true; + + if (is_local) { + mapping_flags |= mtype_local; + } else if (ext_coherent) { + mapping_flags |= AMDGPU_VM_MTYPE_UC; + } else { + /* system memory or remote VRAM */ + mapping_flags |= mtype_remote; + } + break; + default: + mapping_flags |= coherent ? + AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC; + break; + }All of this needs to be in HW IP version specific files.
Got it will do. Where should I place those code? gmc_v9_0/12_0.c? or someplace else?
Regards, Christian.+ + if (flags & AMDGPU_SVM_ATTR_BIT_GPU_EXEC) + mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE; + + pte_flags = AMDGPU_PTE_VALID; + pte_flags |= is_local ? 0 : AMDGPU_PTE_SYSTEM; + pte_flags |= snoop ? AMDGPU_PTE_SNOOPED : 0; + if (gc_ip_version >= IP_VERSION(12, 0, 0)) + pte_flags |= AMDGPU_PTE_IS_PTE; + + amdgpu_gmc_get_vm_pte(svm->adev, svm->vm, NULL, mapping_flags, &pte_flags); + pte_flags |= AMDGPU_PTE_READABLE; + if (!(flags & AMDGPU_SVM_ATTR_BIT_GPU_RO) && !read_only) + pte_flags |= AMDGPU_PTE_WRITEABLE; + + if (gc_ip_version == IP_VERSION(12, 1, 0) && + svm->adev->have_atomics_support) + pte_flags |= AMDGPU_PTE_BUS_ATOMICS; + + return pte_flags; +} + + + +int amdgpu_svm_range_lock_vm_pd(struct amdgpu_svm *svm, struct drm_exec *exec, + bool intr) +{ + unsigned int exec_flags = DRM_EXEC_IGNORE_DUPLICATES; + int ret; + + if (intr) + exec_flags |= DRM_EXEC_INTERRUPTIBLE_WAIT; + + drm_exec_init(exec, exec_flags, 0); + drm_exec_until_all_locked(exec) { + ret = amdgpu_vm_lock_pd(svm->vm, exec, 1); + drm_exec_retry_on_contention(exec); + if (ret) { + drm_exec_fini(exec); + return ret; + } + } + + return 0; +} + +int +amdgpu_svm_range_update_gpu_range(struct amdgpu_svm *svm, + struct amdgpu_svm_range *range, + const struct amdgpu_svm_attrs *attrs, + bool read_only, + bool flush_tlb, + bool wait_fence, + struct dma_fence **fence) +{ + struct drm_gpusvm_range *base = &range->base; + + lockdep_assert_held(&svm->gpusvm.notifier_lock); + + const unsigned long range_start_page = drm_gpusvm_range_start(base) >> PAGE_SHIFT; + const unsigned long range_end_page = drm_gpusvm_range_end(base) >> PAGE_SHIFT; + const unsigned long npages = range_end_page - range_start_page; + unsigned long mapped_pages = 0; + unsigned long dma_idx = 0; + int ret; + + if (!base->pages.dma_addr || !npages) + return -EINVAL; + + while (mapped_pages < npages) { + const struct drm_pagemap_addr *entry = &base->pages.dma_addr[dma_idx++]; + unsigned long seg_pages = min_t(unsigned long, 1UL << entry->order, + npages - mapped_pages); + uint64_t pte_flags; + unsigned long start_page, last_page; + bool is_last_seg; + + if (entry->proto != DRM_INTERCONNECT_SYSTEM) + return -EOPNOTSUPP; + + pte_flags = amdgpu_svm_range_attr_pte_flags(svm, attrs, + read_only, + entry->proto); + + start_page = range_start_page + mapped_pages; + last_page = start_page + seg_pages - 1; + mapped_pages += seg_pages; + is_last_seg = mapped_pages == npages; + + ret = amdgpu_vm_update_range(svm->adev, svm->vm, false, false, + flush_tlb && is_last_seg, true, NULL, + start_page, last_page, pte_flags, + 0, entry->addr, NULL, NULL, + wait_fence && is_last_seg ? fence : NULL); + if (ret) + return ret; + } + + return 0; +} + +struct amdgpu_svm_range * +amdgpu_svm_range_find_or_insert(struct amdgpu_svm *svm, unsigned long addr, + unsigned long gpuva_start, unsigned long gpuva_end, + struct drm_gpusvm_ctx *ctx) +{ + struct drm_gpusvm_range *r; + +retry: + r = drm_gpusvm_range_find_or_insert(&svm->gpusvm, addr, + gpuva_start, gpuva_end, ctx); + /* + * UMD doesn't set RO for some RO VMAs, but the drm gpu svm framework + * denies no RO flag range insert for RO VMAs, so treat + * -EPERM as an indication of RO and retry if not set. + */ + if (PTR_ERR_OR_ZERO(r) == -EPERM && !ctx->read_only) { + ctx->read_only = true; + goto retry; + } + + if (IS_ERR(r)) + return ERR_CAST(r); + + return to_amdgpu_svm_range(r); +} + +int amdgpu_svm_range_get_pages(struct amdgpu_svm *svm, + struct drm_gpusvm_range *range, + struct drm_gpusvm_ctx *ctx) +{ + int ret; + +retry: + ret = drm_gpusvm_range_get_pages(&svm->gpusvm, range, ctx); + /* + * HMM returns -EPERM when write access is requested for a read-only + * VMA. Retry as read-only so the eventual GPU mapping follows the CPU + * VMA permissions. + */ + if (ret == -EPERM && !ctx->read_only) { + ctx->read_only = true; + goto retry; + } + + if (ret == -EOPNOTSUPP) { + AMDGPU_SVM_ERR( + "range get pages -EOPNOTSUPP, evict and retry: gpuva=[0x%lx-0x%lx) ret=%d\n", + drm_gpusvm_range_start(range), + drm_gpusvm_range_end(range), ret); + amdgpu_svm_range_evict(svm, range); + } + + return ret; +} + +void amdgpu_svm_range_evict(struct amdgpu_svm *svm, + struct drm_gpusvm_range *range) +{ + if (!range->pages.flags.has_devmem_pages) + return; + + drm_gpusvm_range_evict(&svm->gpusvm, range); +} + +int amdgpu_svm_range_update_mapping(struct amdgpu_svm *svm, + struct amdgpu_svm_range *range, + const struct amdgpu_svm_attrs *attrs, + bool read_only, + bool intr, bool wait, + bool flush_tlb) +{ + struct drm_exec exec; + struct dma_fence *fence = NULL; + unsigned int flags; + int ret; + + ret = amdgpu_svm_range_lock_vm_pd(svm, &exec, intr); + if (ret) + return ret; + + flags = memalloc_noreclaim_save(); + drm_gpusvm_notifier_lock(&svm->gpusvm); + + if (!amdgpu_svm_range_pages_valid(svm, range)) { + amdgpu_svm_range_invalidate_gpu_mapping(range); + ret = -EAGAIN; + } else { + ret = amdgpu_svm_range_update_gpu_range(svm, range, attrs, + read_only, flush_tlb, + wait, wait ? &fence : NULL); + } + + drm_gpusvm_notifier_unlock(&svm->gpusvm); + memalloc_noreclaim_restore(flags); + + if (!ret && fence) + dma_fence_wait(fence, intr); + dma_fence_put(fence); + + if (!ret) + ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, false); + + if (!ret) { + if (flush_tlb) + svm->flush_tlb(svm); + WRITE_ONCE(range->attr_flags, attrs->flags); + WRITE_ONCE(range->gpu_mapped, true); + range->validate_timestamp = ktime_get_boottime(); + } + + drm_exec_fini(&exec); + return ret; +} + +int +amdgpu_svm_range_map_attrs(struct amdgpu_svm *svm, + const struct amdgpu_svm_attrs *attrs, + unsigned long start, unsigned long end) +{ + unsigned long addr = start; + int ret; + bool devmem_possible = amdgpu_svm_devmem_possible(svm); + bool need_vram_migration = devmem_possible && + amdgpu_svm_attr_prefer_vram(attrs); + struct drm_gpusvm_ctx map_ctx = { + .devmem_possible = devmem_possible, + .devmem_only = need_vram_migration, + .check_pages_threshold = devmem_possible ? SZ_64K : 0, + }; + + while (addr < end) { + struct amdgpu_svm_range *range; + unsigned long next_addr; + /* reset read_only every iteration, amdgpu_svm_range_find_or_insert may change it */ + map_ctx.read_only = !!(attrs->flags & AMDGPU_SVM_ATTR_BIT_GPU_RO); + + range = amdgpu_svm_range_find_or_insert(svm, addr, + addr, end, + &map_ctx); + if (IS_ERR(range)) { + AMDGPU_SVM_ERR( + "failed to find/insert range for gpuva 0x%lx [0x%lx-0x%lx), ret=%ld\n", + addr, start, end, PTR_ERR(range)); + return PTR_ERR(range); + } + + next_addr = drm_gpusvm_range_end(&range->base); + if (next_addr <= addr) + return -EINVAL; + + if (amdgpu_svm_range_is_valid(svm, range, attrs)) { + addr = next_addr; + continue; + } + + /* TODO: add migration */ + + AMDGPU_SVM_RANGE_DEBUG(range, "GET PAGES"); + + ret = amdgpu_svm_range_get_pages(svm, &range->base, + &map_ctx); + if (ret) { + AMDGPU_SVM_ERR("failed to get pages for range [0x%lx-0x%lx), ret=%d\n", + drm_gpusvm_range_start(&range->base), + drm_gpusvm_range_end(&range->base), ret); + return ret; + } + + AMDGPU_SVM_RANGE_DEBUG(range, "UPDATE MAPPING"); + + ret = amdgpu_svm_range_update_mapping(svm, range, attrs, + map_ctx.read_only, + true, true, + true); + if (ret) { + AMDGPU_SVM_ERR("failed to update gpu map for range [0x%lx-0x%lx), ret=%d\n", + drm_gpusvm_range_start(&range->base), + drm_gpusvm_range_end(&range->base), ret); + return ret; + } + + addr = next_addr; + } + + return 0; +} +
