On 6/12/2026 10:38 PM, Huang, Honglei wrote:
+static int
+amdgpu_svm_attr_validate_range_vma(struct amdgpu_svm_attr_tree
*attr_tree,
+ unsigned long start_page,
+ unsigned long last_page)
+{
+ struct vm_area_struct *vma;
+ struct mm_struct *mm;
+ unsigned long start, end;
+ int ret = 0;
+
+ if (start_page > last_page)
+ return -EINVAL;
+
+ if (last_page == ULONG_MAX)
+ return -EINVAL;
+
+ start = start_page << PAGE_SHIFT;
+ end = (last_page + 1) << PAGE_SHIFT;
+ mm = attr_tree->svm->gpusvm.mm;
+ if (!mm)
+ return -EFAULT;
+
+ mmap_read_lock(mm);
+ while (start < end) {
+ vma = amdgpu_svm_check_vma(mm, start);
+ if (IS_ERR(vma)) {
+ ret = PTR_ERR(vma);
+ break;
+ }
+
+ start = min(end, vma->vm_end);
+ }
That's complete nonsense as well.
The result becomes invalid as soon as you drop the lock again, so just
completely drop the check.
hmm_range fault will tell us if the fault was succesful or not.
Got it will drop.>
I dropped that check in my local fix, but there is a regression:
A KFD test failed after dropping the VMA check:
KFDSVMRangeTest.InvalidRangeTest
it does a set attr on an invalid
address: 0x10000 and expects it to fail. Removing the check from the
fault path is correct hmm_range_fault() reports the real outcome
there.
But in xnack on mode set/get-attr are asynchronous fault ioctls with
lazy fault to report errors, so an unmapped range was silently accepted.
I double checked what Xe does, actually Xe SVM also do legality checks,
but it checks xe_vma instead of mm vma, see code below.
Abandoning or retaining the check are all acceptable. I am only
reporting the test results and the current approach of XE SVM here. I
just need a decision on my end.
Xe implementation:
int xe_vm_madvise_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
{
...
if (!madvise_args_are_sane(vm->xe, args)) {
err = -EINVAL;
goto put_vm;
}
...
err = xe_vm_alloc_madvise_vma(vm, args->start, args->range);
if (err)
goto madv_fini;
err = get_vmas(vm, &madvise_range);
if (err || !madvise_range.num_vmas)
goto madv_fini; /* no vmas in range -> rejected */
...
}
drm_gpuvm_for_each_va_range(gpuva, &vm->gpuvm, addr, addr + range) {
struct xe_vma *vma = gpuva_to_vma(gpuva);
if (xe_vma_bo(vma))
madvise_range->has_bo_vmas = true;
else if (xe_vma_is_cpu_addr_mirror(vma) ||
xe_vma_is_userptr(vma))
madvise_range->has_svm_userptr_vmas = true;
...
Regards,
Honglei
Regards,
Christian.
