On 2025-10-03 16:27, Felix Kuehling wrote:
On 2025-10-03 14:15, Philip Yang wrote:
svm_range_unmap_from_gpus uses page aligned start, end address, the end
address is inclusive.
Fixes: 38c55f6719f7 ("drm/amdkfd: Handle lack of READ permissions in
SVM mapping")
Signed-off-by: Philip Yang <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index e8a15751c125..0aadd20be56a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1723,10 +1723,9 @@ static int svm_range_validate_and_map(struct
mm_struct *mm,
svm_range_lock(prange);
if (vma->vm_flags & VM_WRITE)
pr_debug("VM_WRITE without VM_READ is not
supported");
- s = max(start, prange->start);
- e = min(end, prange->last);
- if (e >= s)
- r = svm_range_unmap_from_gpus(prange, s, e,
+ s = max(addr >> PAGE_SHIFT, prange->start);
+ e = s + npages - 1;
This is confusing. The old code was clamping prange->start/last with
start/end. start/end are based on map_start/map_end. Your updated code
uses npages, which comes from the VMA, which may not start at the same
address as the prange or the map_start. So I think just using npages
here is incorrect.
For restore_pages, the map_start and map_end could be partial range, but
we should not get here as vm_flags VM_NONE or VM_WRITE. For svm restore
worker, map_start, map_end is entire prange, start is from loop variable
addr, npages clamp to VMA end, to go over VMAs of prange.
You also completely removed the condition that s >= e. I think that's
OK, since all callers make sure that map_start/map_end fall inside
prange->start/last.
What are you actually trying to unmap here? Do you want to unmap the
entire prange, the part of prange inside map_start..map_end, or the
part of prange that overlaps with the VMA, or something else?
For svm restore work no VMA case in next patch, this will unmap entire
prange.
Regards,
Philip
Regards,
Felix
+ r = svm_range_unmap_from_gpus(prange, s, e,
KFD_SVM_UNMAP_TRIGGER_UNMAP_FROM_CPU);
svm_range_unlock(prange);
/* If unmap returns non-zero, we'll bail on the
next for loop
