On 10/16/26 16:51, Daniel Huang wrote: > KFD SVM maps memory in GPUVM at the same virtual address used by > the CPU. However, svm_range_map_to_gpu() passes the corresponding > PFNs to amdgpu_vm_update_range() without checking them against > vm_manager.max_pfn. > > If the GPUVM address range is smaller than the CPU address range, > the page-table walker can index beyond the allocated root page-table > entries. Unlike the SVM path, the BO mapping path validates the > address range through amdgpu_vm_verify_parameters(). > > An explicit amdgpu.vm_size override can trigger this condition by > configuring a smaller GPUVM than the driver would normally select. > > Reproduced on a Radeon 780M (gfx1103) with amdgpu.vm_size=64. > Repeated hipMallocManaged() allocations attempted to map CPU > address 0x74bd48000000, well beyond the 64 GiB GPUVM limit. > The out-of-bounds root entry contained SLUB poison with > slub_debug=FZPU and recycled heap data without it, resulting in > a general protection fault: > > RIP: 0010:amdgpu_vm_cpu_update+0x2f/0x140 [amdgpu] > amdgpu_vm_pte_update_flags+0xfa/0x1e0 [amdgpu] > amdgpu_vm_ptes_update+0x38b/0xa00 [amdgpu] > amdgpu_vm_update_range+0x279/0x930 [amdgpu] > svm_range_map_to_gpu+0x13d/0x400 [amdgpu] > svm_range_validate_and_map+0x8d3/0xaa0 [amdgpu] > svm_ioctl+0x66/0x80 [amdgpu] > kfd_ioctl+0x40a/0x650 [amdgpu] > > As discussed in [1], rejecting out-of-range PFNs in > amdgpu_vm_update_range() is too late. SVM must not be initialized > when the GPUVM cannot represent the required CPU addresses. > > Prevent this by disabling KFD SVM support when the GPUVM address > range is smaller than AMDGPU_GMC_HOLE_START. This prevents > affected devices from advertising SVM support and excludes them > from SVM mappings. Warn from kfd_update_svm_support_properties(), > which runs after the device memory pgmap is registered, so that > only devices that would otherwise have advertised SVM report the > loss of the capability. > > [1] > https://lore.kernel.org/amd-gfx/[email protected]/ > > Fixes: f80fe9d3c114 ("drm/amdkfd: map svm range to GPUs") > Closes: https://github.com/ROCm/legacy-rocm-build/issues/6691 > Cc: [email protected]
I think we should drop the Fixes and CC stable tag, that is not really a bug fix. > Signed-off-by: Daniel Huang <[email protected]> > --- > > Tested on a Radeon 780M (gfx1103, PCI 1002:15bf), kernel > 7.0.0-38-generic, and ROCm 7.13.0-2. > > The reproducer performs 24 iterations of hipMallocManaged(1 GiB), > hipMemset(), and hipFree() per run, with 40 runs per attempt. > > With amdgpu.vm_size=64: > - Stock module: kernel Oops within approximately 2 seconds on > every attempt. > - Patched module: completed 40/40 runs in two attempts, with > stock and patched modules tested alternately on the same kernel. > > With the default GPUVM size, the patched module preserved SVM > support, emitted no warning, and completed the same test. > > Two questions for maintainers: > > 1. I interpreted "SVM must fail to initialize" as disabling > the SVM capability for affected devices rather than failing > KFD initialization or rejecting the vm_size override. > Would a different approach be preferable? I think that this is ok. The amdgpu.vm_size override is only for driver testing and not something end users are supposed to change in any way. So that here is actually a really minor issue. > > 2. This check also affects GMC v6/v7/v8, whose GPUVM address > ranges cannot reach AMDGPU_GMC_HOLE_START. Although these > devices do not register device-private memory, an APU with > apu_prefer_gtt can currently advertise SVM support. > > Should the check apply to these devices as well, or should > it be limited to explicitly configured amdgpu.vm_size > overrides? So we still support GMC v6/v7/v8 with KFD? I think we have dropped that a while ago. If we still support this then that would indeed be problematic. Regards, Christian. > drivers/gpu/drm/amd/amdkfd/kfd_svm.h | 18 ++++++++++++++++-- > drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 14 +++++++++++++- > 2 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h > b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h > index c7d7adae4476..6235ce0f988e 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h > @@ -203,11 +203,25 @@ struct kfd_process_device * > svm_range_get_pdd_by_node(struct svm_range *prange, struct kfd_node *node); > void svm_range_list_lock_and_flush_work(struct svm_range_list *svms, struct > mm_struct *mm); > > +/* > + * SVM uses CPU virtual addresses for GPU mappings. Disable SVM if the GPUVM > + * address range cannot cover the required CPU address range. > + * > + * The parameter must be named adev since AMDGPU_GMC_HOLE_START references > + * adev->vm_manager.max_level. > + */ > +static inline bool kfd_svm_vm_covers_user_va(struct amdgpu_device *adev) > +{ > + return (adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT) >= > + AMDGPU_GMC_HOLE_START; > +} > + > /* SVM API and HMM page migration work together, device memory type > * is initialized to not 0 when page migration register device memory. > */ > -#define KFD_IS_SVM_API_SUPPORTED(adev) ((adev)->kfd.pgmap.type != 0 ||\ > - ((adev)->apu_prefer_gtt)) > +#define KFD_IS_SVM_API_SUPPORTED(adev) > \ > + (((adev)->kfd.pgmap.type != 0 || (adev)->apu_prefer_gtt) && \ > + kfd_svm_vm_covers_user_va(adev)) > > void svm_range_bo_unref_async(struct svm_range_bo *svm_bo); > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c > b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c > index 35b3abe57b80..5de19fa27d12 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c > @@ -2471,8 +2471,20 @@ void kfd_update_svm_support_properties(struct > amdgpu_device *adev) > sys_props.generation_count++; > else > dev_err(adev->dev, "Failed to update SVM > support properties. ret=%d\n", ret); > - } else > + } else { > +#if IS_ENABLED(CONFIG_HSA_AMD_SVM) > + /* Report it rather than letting the capability > + * silently vanish from the KFD topology. > + */ > + if ((adev->kfd.pgmap.type || adev->apu_prefer_gtt) && > + !kfd_svm_vm_covers_user_va(adev)) > + dev_warn_once(adev->dev, > + "GPUVM size 0x%llx is too small > for KFD SVM; SVM disabled (amdgpu.vm_size=%d)\n", > + adev->vm_manager.max_pfn << > AMDGPU_GPU_PAGE_SHIFT, > + amdgpu_vm_size); > +#endif > dev->node_props.capability &= ~HSA_CAP_SVMAPI_SUPPORTED; > + } > } > up_write(&topology_lock); > }
