Re: [PATCH] drm/xe: Allow mixed mappings for userptr

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

On Tue, Sep 23, 2025 at 04:52:30PM +0200, Thomas Hellström wrote:
> Hi,
> 
> On Wed, 2025-09-17 at 11:28 -0700, Matthew Brost wrote:
> > Compute kernels often issue memory copies immediately after
> > completion.
> > If the memory being copied is an SVM pointer that was faulted into
> > the
> > device and then bound via userptr, it is undesirable to move that
> > memory. Worse, if userptr is mixed between system and device memory,
> > the
> > bind operation may be rejected.
> > 
> > Xe already has the necessary plumbing to support userptr with mixed
> > mappings. This update modifies GPUSVM's get_pages to correctly locate
> > pages in such mixed mapping scenarios.
> > 
> > Fixes: ("9e9787414882 drm/xe/userptr: replace xe_hmm with gpusvm")
> > Signed-off-bt: Matthew Brost <matthew.brost@intel.com>
> 
> s/bt/by/
> 
> Perhaps we need to let the PAT index discussion land before we merge
> this. If we support multiple placements we might need multiple PAT
> indices...
> 

We'd likely just get the pat_index from the userptr bind here, which the
UMD likely expects to correspond to system memory. Either way, let's
wrap up the PAT index discussion before introducing more complexity.
Whatever we decide on will probably require a fixes patch here.
Alternatively, we could take a suboptimal route in the fixes path by
evicting userptr from device memory upon bind — similar to what was
necessary in stable kernels before the userptr → GPUSVM transition
landed.

Matt

> Otherwise LGTM.
> 
> Thanks,
> Thomas
> 
> 
> > ---
> >  drivers/gpu/drm/drm_gpusvm.c    | 6 ++++--
> >  drivers/gpu/drm/xe/xe_userptr.c | 1 +
> >  include/drm/drm_gpusvm.h        | 4 ++++
> >  3 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c
> > b/drivers/gpu/drm/drm_gpusvm.c
> > index eeeeb99cfdf6..64c0ea70eae3 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > @@ -1361,7 +1361,8 @@ int drm_gpusvm_get_pages(struct drm_gpusvm
> > *gpusvm,
> >  		order = drm_gpusvm_hmm_pfn_to_order(pfns[i], i,
> > npages);
> >  		if (is_device_private_page(page) ||
> >  		    is_device_coherent_page(page)) {
> > -			if (zdd != page->zone_device_data && i > 0)
> > {
> > +			if (!ctx->allow_mixed &&
> > +			    zdd != page->zone_device_data && i > 0)
> > {
> >  				err = -EOPNOTSUPP;
> >  				goto err_unmap;
> >  			}
> > @@ -1397,7 +1398,8 @@ int drm_gpusvm_get_pages(struct drm_gpusvm
> > *gpusvm,
> >  		} else {
> >  			dma_addr_t addr;
> >  
> > -			if (is_zone_device_page(page) || pagemap) {
> > +			if (is_zone_device_page(page) ||
> > +			    (pagemap && !ctx->allow_mixed)) {
> >  				err = -EOPNOTSUPP;
> >  				goto err_unmap;
> >  			}
> > diff --git a/drivers/gpu/drm/xe/xe_userptr.c
> > b/drivers/gpu/drm/xe/xe_userptr.c
> > index 91d09af71ced..c628f58c085c 100644
> > --- a/drivers/gpu/drm/xe/xe_userptr.c
> > +++ b/drivers/gpu/drm/xe/xe_userptr.c
> > @@ -54,6 +54,7 @@ int xe_vma_userptr_pin_pages(struct xe_userptr_vma
> > *uvma)
> >  	struct xe_device *xe = vm->xe;
> >  	struct drm_gpusvm_ctx ctx = {
> >  		.read_only = xe_vma_read_only(vma),
> > +		.allow_mixed = true,
> >  	};
> >  
> >  	lockdep_assert_held(&vm->lock);
> > diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> > index 5434048a2ca4..97d2f57914bb 100644
> > --- a/include/drm/drm_gpusvm.h
> > +++ b/include/drm/drm_gpusvm.h
> > @@ -235,6 +235,9 @@ struct drm_gpusvm {
> >   * @read_only: operating on read-only memory
> >   * @devmem_possible: possible to use device memory
> >   * @devmem_only: use only device memory
> > + * @allow_mixed: Allow mixed mappings in get pages. Mixing between
> > system and
> > + *               single dpagemap is supported, mixing between
> > multiple dpagemap
> > + *               is unsupported.
> >   *
> >   * Context that is DRM GPUSVM is operating in (i.e. user arguments).
> >   */
> > @@ -245,6 +248,7 @@ struct drm_gpusvm_ctx {
> >  	unsigned int read_only :1;
> >  	unsigned int devmem_possible :1;
> >  	unsigned int devmem_only :1;
> > +	unsigned int allow_mixed :1;
> >  };
> >  
> >  int drm_gpusvm_init(struct drm_gpusvm *gpusvm,
>