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

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Fri, 2025-05-02 at 21:41 +0530, Ghimiray, Himal Prasad wrote:
> 
> 
> 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.

OK, then for the uapi part
Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


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