Re: [RFC/POC PATCH 00/12] POC SVM implementation in AMDGPU based on drm_gpusvm

["Huang, Honglei1" <honghuan@amd.com>]

On 4/23/2026 3:18 PM, Matthew Brost wrote:
...
>>>>>>> This clarifies a lot. This is what we’d call in Xe “preemption fence”
>>>>>>> mode for a VM. Anytime memory is moved, we trigger a GPU preemption and
>>>>>>> resume. We don’t actually support SVM in this case; instead, we use
>>>>>>> “userptr binds,” which are built on gpusvm for page
>>>>>>> collection. However,
>>>>>>> we don’t support migrating memory to the device—though we could.
>>>>>>>
>>>>>>> I’d look at how we converted 'userptr' to be based on GPU SVM [2]. In
>>>>>>> this case, don’t maintain a range tree, as those—as you
>>>>>>> suggest—are more
>>>>>>> of an on-demand fault driver concern. Instead, just embed 'struct
>>>>>>> drm_gpusvm_pages' in the VMA struct defined by the IOCTLs..
>>>>>>>
>>>>>>> We could extend this to support migrating 'userptr', but we
>>>>>>> just haven’t
>>>>>>> done that yet—this may be what you want to do in “XNACK off..
>>>>>>>
>>>>>>> [2] https://patchwork.freedesktop.org/series/146553/
>>>>>>>
>>>>>>
>>>>>> Actually we need to swith the xnack mode between on and off, so
>>>>>> in xnack off
>>>>>> mode, the driver operats in "implicit  prefetch mode". This may
>>>>>> be due to
>>>>>> compatibility with older hardware and the need for UMD runtime. We will
>>>>>> further discuss the handling method under xnack off internally.
>>>>>>
>>>
>>> Hi Matt,
>>>
>>> I studied the xe_userptr code and the conversion series [2] you
>>> pointed to.
>>>
>>> I have a question that:
>>> Would it be possible to reuse drm_gpusvm_range to handle the hardware
>>> without gpu fault feature(xnack off mode).
>>
>> That’s not how we’ve done it. We embedded drm_gpusvm_pages into our VMA
>> structure and then attached a notifier. The notifier attachment is
>> open-coded on the Xe side, and this could be normalized and opened up
>> for common driver use cases.

The way in xe_userptr likes the implementation in kfd_svm: embeded 
physical pages into structure and attach same size notifier.
But kfd_svm is an implementation of SVM semantics, which supports 
partial unmap, doesn't need explicitly delete userptr ioctl calling when 
remove , and doesn't need a explicitly userptr flag when creating.
And actually there is also a existing implementation for userptr 
semantics in amdgpu kfd: KFD_IOC_ALLOC_MEM_FLAGS_USERPTR.
If the no gpu fault mode can not use the drm gpu svm fram work, use the 
same way for xe_userptr, it seems like doing the duplicate work.

I think the core gap is we are trying to use the drmgpu_svm to implement 
a SVM semantics driver for no gpu fault hardware instead of userptr 
semantics.

>>
>> The problem with reusing drm_gpusvm_range directly is that a VMA may
>> span multiple gpusvm notifiers—i.e., it can be larger than the notifier
>> size. Of course, we could rework this as well.

So the "VMA spans multiple gpusvm notifiers" concern: I'd like to 
clarify that this is not actually a blocker for amdgpu's XNACK-off path, 
because amdgpu does not try to represent one user ioctl virtual address 
interval as a single drm_gpusvm_range.

we walk the attr interval and call drm_gpusvm_range_find_or_insert() 
repeatedly, letting gpusvm pick chunk aligned ranges bounded by 
notifier_size. One ioctl interval will create N chunk sized ranges.

>>
> 
> Sorry for the double reply—I just glanced at the latest series. I don’t
> think creating a range per page of the userptr is desirable. While it
> would work, from a time-complexity point of view I don’t think this is
> ideal.
> 
> The issue with spans across multiple notifiers is real, though.
> 
> My rough idea would be:
> 
> - Give drivers an interface to create larger ranges.

So maybe we do not need to create larger ranges if we call 
drm_gpusvm_range_find_or_insert() repeatedly.

> 
> - If the range fits inside a single notifier’s size → done.
> 
> - If the range spans multiple notifier sizes → round up to a power of
>    two and create a larger notifier. This may overlap with existing
>    notifiers, which is likely fine given that interval trees support
>    overlaps (?). We’d need to double-check and test this. If overlapping
>    notifiers are not acceptable, we’d need some heavy-handed notifier merge
>    logic—it will be complicated, but isolated, so once we get it right
>    everyone can use it.

If we call drm_gpusvm_range_find_or_insert() repeatedly the drmgpu_svm 
will create the corresponding notifier correctly as far as I can see.

Regards,
Honglei

> 
> - Finally, make sure that individual userptr pages can reside at any
>    location.
> 
> Over conversely:
> 
> - Normalize embedding of drm_gpusvm_pages in VMA structs + notifier
>    creation
> 
> - Make sure that individual userptr pages can reside at any location.

> 
> Both options actually sound really similar after typing this out.
> 
> Matt
> 
>> So either way, the Xe userptr + gpusvm implementation should be refined
>> further for common driver use.
>>
>>>
>>> Reusing drm_gpusvm_range for the XNACK-off case would simplify our
>>> implementation considerably, it already provides large page chunk
>>> optimization, can reuse the existing migration infrastructure.
>>>
>>> Building these on top of a standalone drm_gpusvm_pages
>>> would mean reimplementing much of what the range layer already offers.
>>> It would also let us keep a single code path for both XNACK modes,
>>> which reduces maintenance burden and avoids behavioral difference.
>>>
>>> Would this direction be acceptable, or do you see concerns with reusing
>>> the range infrastructure for the no-fault case?
>>>
>>
>> If you prefer something like insert a range exactly here + create range
>> + notifier I think that completely reasonable direction and Xe would
>> likely switch over to using this.
>>
>> I guess my only concern is sub-userptr migration. We are trending
>> towards allowing userptrs to being migrated either via prefetch IOCTLs
>> or access counters on the GPU side - access counter we'd likely a single
>> 2M page at time migration within the userptr. get_pages() supports mixed
>> mappings between VRAM + system but likely needs some more work to really
>> make this complete though.
>>
>> Matt
>>   
>>> Regards,
>>> Honglei
...