Re: [PATCH v6 19/20] drm/xe/svm: Implement prefetch support for SVM ranges

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 02-05-2025 20:09, Thomas Hellström wrote:
> On Fri, 2025-05-02 at 20:05 +0530, Ghimiray, Himal Prasad wrote:
>>
>>
>> On 02-05-2025 19:12, Thomas Hellström wrote:
>>> On Wed, 2025-04-30 at 17:49 +0530, Himal Prasad Ghimiray wrote:
>>>> This commit adds prefetch support for SVM ranges, utilizing the
>>>> existing ioctl vm_bind functionality to achieve this.
>>>>
>>>> v2: rebase
>>>>
>>>> v3:
>>>>      - use xa_for_each() instead of manual loop
>>>>      - check range is valid and in preferred location before
>>>> adding to
>>>>        xarray
>>>>      - Fix naming conventions
>>>>      - Fix return condition as -ENODATA instead of -EAGAIN
>>>> (Matthew
>>>> Brost)
>>>>      - Handle sparsely populated cpu vma range (Matthew Brost)
>>>>
>>>> v4:
>>>>      - fix end address to find next cpu vma in case of -ENOENT
>>>>
>>>> v5:
>>>>      - Move find next vma logic to drm gpusvm layer
>>>>      - Avoid mixing declaration and logic
>>>>
>>>> v6:
>>>>     - Use new function names
>>>>     - Move eviction logic to prefetch_ranges
>>>>
>>>> Cc: Matthew Brost <matthew.brost@intel.com>
>>>> Signed-off-by: Himal Prasad Ghimiray
>>>> <himal.prasad.ghimiray@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/xe/xe_pt.c |  58 ++++++++---
>>>>    drivers/gpu/drm/xe/xe_vm.c | 197
>>>> +++++++++++++++++++++++++++++++++++--
>>>>    2 files changed, 234 insertions(+), 21 deletions(-)
>>>>
>>>
>>> I was thinking a bit around the UAPI, and prefetching foreign
>>> device
>>> regions. That doesn't really fit into the current UAPI.
>>>
>>> What are your thoughts here. IMO this should align somewhat to what
>>> we
>>> do for madvise().
>>>
>>> Let's say we want to prefetch into a foreign device memory region
>>> identified by an fd? I figure we could add that later as an
>>> extension.
>>>
>>> But how does the UMD API for this look? Do we cover current usage
>>> with
>>> the existing UAPI syntax?
>>
>> During Initial discussions one of the way we discussed was using a
>> flag
>> something like DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC for prefetch to
>> madvise assigned preferred location.
>>
>> so it will be two step process from UMD perspective:
>> 1) madvise sets up preferred location: which should also be used in
>> pagefault handler.
>>
>> 2) If user wants to prefetch to same location, user passes region as
>> DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC to vm_bind ioctl.
>>
>> https://patchwork.freedesktop.org/patch/647180/?series=146290&rev=4
> 
> Indeed, thanks for reminding me.
> 
>>
>> or we can simply pass u32 devmem_fd as prefetch_mem_region_instance
>> to
>> vm_bind ioctl,
> 
> But that wouldn't work if we have published UMD using the
> mem_region_instance as the local mem_region_instance, right?

Thats correct.

> 
> So if we want that we'd need an extension?

True. IMO sticking to DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC  makes more sense.

> 
> /Thomas
> 
> 
> 
>>   if we want it to be single step, but w/o madvise that
>> wont work for pf handler.
> 
> 
> 
>>
>> /Himal>
>>> /Thomas
>>>
>>
>