[PATCH] drm/amdkfd: Handle lack of READ permissions in SVM mapping

[Chen, Xiaogang]

On 7/22/2025 11:24 AM, Kent Russell wrote:
> HMM assumes that pages have READ permissions by default. Inside
> svm_range_validate_and_map, we add READ permissions then add WRITE
> permissions if the VMA isn't read-only. This will conflict with regions
> that only have PROT_WRITE or have PROT_NONE. When that happens,
Why read-only conflict with PROT_WRITE or have PROT_NONE? They are 
vma->vm_flags that specifies the vma protection. User can change its 
value at runtime. Is user not allowed to change it from read-only to 
PROT_NONE?
> svm_range_validate_and_map will continue to retry, silently, giving the
> impression of a hang.
>
> If pages don't have READ permissions, simply unmap them and continue. If
> they weren't mapped in the first place, this would be a no-op. Since x86
> doesn't support write-only, and PROT_NONE doesn't allow reads or writes
> anyways, this will allow the svm range validation to continue without
> getting stuck in a loop forever on mappings we can't use with HMM.
>
> Signed-off-by: Kent Russell<kent.russell at amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index e23b5a0f31f2..10b70b941b11 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1713,6 +1713,24 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   
>   			next = min(vma->vm_end, end);
>   			npages = (next - addr) >> PAGE_SHIFT;
> +			/* HMM requires at least READ permissions. If provided with PROT_NONE,
> +			 * unmap the memory. If it's not already mapped, this is a no-op
> +			 * If PROT_WRITE is provided without READ, warn first then unmap
> +			 */
> +			if (!(vma->vm_flags & VM_READ)) {
> +				unsigned long e, s;
> +
> +				if (vma->vm_flags & VM_WRITE)
> +					pr_warn("VM_WRITE without VM_READ is not supported");
> +				s = max(start, prange->start);
> +				e = min(end, prange->last);
> +				if (e >= s)
> +					svm_range_unmap_from_gpus(prange, s, e,
> +						       KFD_SVM_UNMAP_TRIGGER_UNMAP_FROM_CPU);
> +				addr = next;
> +				continue;
> +			}
> +
>   			WRITE_ONCE(p->svms.faulting_task, current);
>   			r = amdgpu_hmm_range_get_pages(&prange->notifier, addr, npages,
>   						       readonly, owner, NULL,

It seems the real problem is at amdgpu_hmm_range_get_pages. It always 
set HMM_PFN_REQ_FAULT to hmm_range->default_flags. HMM_PFN_REQ_FAULT 
means the page is faultable and a future call with HMM_PFN_REQ_FAULT 
could succeed. When vma->vm_flags is PROT_NONE the vma is not faultable, 
so hmm_range->default_flags should be not set to HMM_PFN_REQ_FAULT to 
avoid hmm_range_fault fault this vma.

Regards

Xiaogang



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20250722/af9471aa/attachment.htm>