Re: [PATCH 1/2] drm/xe/svm: Ensure data will be migrated to system if indicated by madvise.

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Oct 08, 2025 at 04:42:41AM -0700, Matthew Brost wrote:
> On Wed, Oct 08, 2025 at 01:10:49PM +0200, Thomas Hellström wrote:
> > If the location madvise() is set to
> > DRM_XE_PREFERRED_LOC_DEFAULT_SYSTEM, the drm_pagemap in the
> > SVM gpu fault handler will be set to NULL. However there is nothing
> > that explicitly migrates the data to system if it is already present
> > in device memory.
> > 
> > In that case, set the device memory owner to NULL to ensure
> > data gets properly migrated to system on page-fault.
> > 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_svm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> > index 7e2db71ff34e..fd906eb03d71 100644
> > --- a/drivers/gpu/drm/xe/xe_svm.c
> > +++ b/drivers/gpu/drm/xe/xe_svm.c
> > @@ -1034,6 +1034,9 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma,
> >  	if (err)
> >  		return err;
> >  
> > +	dpagemap = xe_vma_resolve_pagemap(vma, tile);
> > +	if (!dpagemap && !ctx.devmem_only)
> > +		ctx.device_private_page_owner = NULL;
> 
> I think xe_svm_range_is_valid below can still make the page fault
> handler bail out if the memory is in valid in VRAM. That a really odd
> case but maybe it needs to be handled?
> 
> What certainly needs to be handled in xe_svm_range_is_valid is if system
> is the preferred placement, the range has mappings, and is in system. If
> we get page fault storms on system memory address every fault will issue
> a rebind.
> 

Correction - I misread the xe_svm_range_is_valid logic - the 2d case is
handled.

Patch looks good, the first case still exists but unsure if we really
need to handle that case.

Anyways:
Reviewed-by: Matthew Brost <matthew.brost@intel.com> 

> Matt
> 
> >  	range = xe_svm_range_find_or_insert(vm, fault_addr, vma, &ctx);
> >  
> >  	if (IS_ERR(range))
> > -- 
> > 2.51.0
> >