Re: [PATCH v2 2/2] drm/amdkfd: validate SVM ranges against the target GPU address limits

[Felix Kuehling <felix.kuehling@amd.com>]

On 2026-10-16 13:05, Daniel Huang wrote:
> KFD SVM maps memory in GPUVM at the same virtual address used by the
> CPU, but nothing bounds that address against what the target GPU can
> represent. kfd_ioctl_svm() checks only page alignment and a non-zero
> size, so the PFNs reach amdgpu_vm_update_range() unvalidated. Unlike
> the SVM path, the BO mapping path validates the address range through
> amdgpu_vm_verify_parameters().
>
> Disabling SVM when the GPUVM cannot cover AMDGPU_GMC_HOLE_START closes
> the case where the GPUVM is globally too small, but it compares the
> GPUVM against the GPU's own address hole. When the CPU address space
> is larger than the GPU's, the comparison succeeds and SVM stays
> enabled while individual addresses can still fall outside the GPUVM.
>
> On a 5-level paging CPU with a 4-level GPU, mmap() returns addresses
> above the GPU limit when userspace passes a hint above
> DEFAULT_MAP_WINDOW. Passing such an address to KFD SVM indexes beyond
> the allocated root page-table entries in amdgpu_vm_pt_descendant().
>
> This gap was found by the Sashiko AI review bot, during review of the
> first patch in this series.
>
> Validate every range against the limits of the GPU it is being mapped
> on, using both vm_manager.max_pfn and the GPUVM address hole, and
> check the end of the range rather than only its start.
>
> GPUs in one process can have different address limits, so the range is
> checked per GPU. svm_range_validate_and_map() skips the GPUs that
> cannot represent the range instead of failing the whole mapping,
> matching how it already skips GPUs outside the XGMI hive.
> svm_range_map_to_gpu() keeps a hard check so that no out-of-range PFN
> can reach amdgpu_vm_update_range() through any caller.
>
> Page fault recovery is rejected early in svm_range_restore_pages(), and
> svm_range_unmap_from_gpu() skips ranges that were never mappable rather
> than returning an error, since unmap also runs on the eviction and
> process teardown paths.

AFAICT, this breaks mapping of memory to GPUs without any feedback to 
user mode (until a VM fault kills the application). It also duplicates 
the check many times (every time a range is mapped or unmapped for any 
reason). I'd rather reject address ranges that aren't GPU-addressable as 
early as possible in the set-attribute calls that create SVM ranges. 
That way user mode can observe the failure when it happens, which makes 
the problem easier to debug or recover from.

In other words, save the minimum virtual address limit of all supported 
GPUs used in a process in process->svms during 
svm_range_list_init. Check virtual addresses against that limit in 
svm_range_add (for ranges created by set_attribute ioctls) and 
svm_range_create_unregistered_range (for ranges created by page fault 
interrupts).

Regards,
   Felix


>
> Link: https://sashiko.dev/#/patchset/20261016145129.69645-1-danielhu%40amd.com
> Fixes: f80fe9d3c114 ("drm/amdkfd: map svm range to GPUs")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Daniel Huang <danielhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 42 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.h | 27 ++++++++++++++++++
>   2 files changed, 69 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index cfb428c72686..c3e4f0367522 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1427,6 +1427,16 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		return -EINVAL;
>   	}
>   
> +	/* Such a range was rejected by svm_range_map_to_gpu() and so was
> +	 * never mapped. Skip it rather than failing, since unmap is also
> +	 * used on the eviction and process teardown paths.
> +	 */
> +	if (!kfd_svm_range_fits_gpu(adev, start, last)) {
> +		pr_debug("CPU[0x%llx 0x%llx] exceeds GPUVM, skipping unmap\n",
> +			 start, last);
> +		return 0;
> +	}
> +
>   	return amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL, gpu_start,
>   				      gpu_end, init_pte_value, 0, 0, NULL, NULL,
>   				      fence);
> @@ -1504,6 +1514,16 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
>   		return -EINVAL;
>   	}
>   
> +	/* Last line of defence before amdgpu_vm_update_range() walks the
> +	 * page tables with these PFNs.
> +	 */
> +	if (npages && !kfd_svm_range_fits_gpu(adev, last_start,
> +					      last_start + npages - 1)) {
> +		pr_debug("CPU[0x%lx 0x%lx] exceeds GPUVM, canceling map\n",
> +			 last_start, last_start + npages - 1);
> +		return -EINVAL;
> +	}
> +
>   	for (i = offset; i < offset + npages; i++) {
>   		uint64_t gpu_start;
>   		uint64_t gpu_end;
> @@ -1604,6 +1624,20 @@ svm_range_map_to_gpus(struct svm_range *prange, unsigned long offset,
>   			continue;
>   		}
>   
> +		/* GPUs in the same process can have different address limits.
> +		 * Skip the ones that cannot represent this range instead of
> +		 * failing the whole mapping.
> +		 */
> +		if (npages && !kfd_svm_range_fits_gpu(pdd->dev->adev,
> +						      prange->start + offset,
> +						      prange->start + offset +
> +						      npages - 1)) {
> +			pr_debug("device idx %d cannot map CPU[0x%lx 0x%lx]\n",
> +				 gpuidx, prange->start + offset,
> +				 prange->start + offset + npages - 1);
> +			continue;
> +		}
> +
>   		set_bit(gpuidx, prange->bitmap_mapped);
>   
>   		r = svm_range_map_to_gpu(pdd, prange, offset, npages, readonly,
> @@ -3096,6 +3130,14 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
>   		return -EFAULT;
>   	}
>   
> +	/* addr is a CPU page index here. Reject faults the GPUVM cannot
> +	 * represent before any range is looked up or migrated.
> +	 */
> +	if (!kfd_svm_range_fits_gpu(adev, addr, addr)) {
> +		pr_debug("fault address 0x%llx exceeds GPUVM\n", addr);
> +		return -EFAULT;
> +	}
> +
>   	p = kfd_lookup_process_by_pasid(pasid, NULL);
>   	if (!p) {
>   		pr_debug("kfd process not founded pasid 0x%x\n", pasid);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> index 6235ce0f988e..57d4d79a6266 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> @@ -216,6 +216,33 @@ static inline bool kfd_svm_vm_covers_user_va(struct amdgpu_device *adev)
>   		AMDGPU_GMC_HOLE_START;
>   }
>   
> +/*
> + * A range is mappable on adev only if it lies below both the page table
> + * limit and the GPUVM address hole. start and last are CPU page indices
> + * and are inclusive, matching svm_range.start and svm_range.last.
> + *
> + * Each GPU is checked against its own limits, so a range that only some
> + * GPUs can represent is rejected for those GPUs alone.
> + *
> + * The parameter must be named adev since AMDGPU_GMC_HOLE_START references
> + * adev->vm_manager.max_level.
> + */
> +static inline bool kfd_svm_range_fits_gpu(struct amdgpu_device *adev,
> +					  uint64_t start, uint64_t last)
> +{
> +	uint64_t limit;
> +
> +	/* Rejects empty and wrapped ranges, so last bounds start as well. */
> +	if (start > last)
> +		return false;
> +
> +	/* First CPU page the GPUVM cannot represent. */
> +	limit = min(adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT,
> +		    AMDGPU_GMC_HOLE_START) >> PAGE_SHIFT;
> +
> +	return last < limit;
> +}
> +
>   /* SVM API and HMM page migration work together, device memory type
>    * is initialized to not 0 when page migration register device memory.
>    */