public inbox for intel-xe@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages
@ 2026-04-09  1:55 Matthew Brost
  2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
  2026-04-17 11:42 ` [PATCH] drm/gpusvm, pagemap: " Thomas Hellström
  0 siblings, 2 replies; 5+ messages in thread
From: Matthew Brost @ 2026-04-09  1:55 UTC (permalink / raw)
  To: intel-xe, dri-devel; +Cc: francois.dugast, himal.prasad.ghimiray

Update drm_pagemap_page_zone_device_data() to derive the pgmap ops from
the page and compare them against the DRM pagemap ops. If the ops do not
match, return NULL.

Also harden two risky call sites by checking for NULL after
hmm_range_fault() or migrate_vma_setup() when migrating to device
memory, as it is possible to encounter device pages that are not owned
by DRM pagemap.

Suggested-by: sashiko.dev
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/drm_gpusvm.c  |  5 +++++
 drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
 include/drm/drm_pagemap.h     |  5 ++++-
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 365a9c0b522a..b3cccd047a21 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 			struct drm_pagemap_zdd *__zdd =
 				drm_pagemap_page_zone_device_data(page);
 
+			if (!__zdd) {
+				err = -EINVAL;
+				goto err_unmap;
+			}
+
 			if (!ctx->allow_mixed &&
 			    zdd != __zdd && i > 0) {
 				err = -EOPNOTSUPP;
diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
index d82ea7ccb8da..95c951c5b569 100644
--- a/drivers/gpu/drm/drm_pagemap.c
+++ b/drivers/gpu/drm/drm_pagemap.c
@@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
 				own_pages++;
 				goto next;
 			}
-			cur.dpagemap = src_zdd->dpagemap;
-			cur.ops = src_zdd->devmem_allocation->ops;
-			cur.device = cur.dpagemap->drm->dev;
-			pages[i] = src_page;
+			if (src_zdd) {
+				cur.dpagemap = src_zdd->dpagemap;
+				cur.ops = src_zdd->devmem_allocation->ops;
+				cur.device = cur.dpagemap->drm->dev;
+				pages[i] = src_page;
+			} else {
+				npages = i;
+				err = -EINVAL;
+				goto err_finalize;
+			}
 		}
 		if (!pages[i]) {
 			cur.dpagemap = NULL;
diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
index 95eb4b66b057..9b7c50932db5 100644
--- a/include/drm/drm_pagemap.h
+++ b/include/drm/drm_pagemap.h
@@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap *dpagemap);
  * drm_pagemap_page_zone_device_data() - Page to zone_device_data
  * @page: Pointer to the page
  *
- * Return: Page's zone_device_data
+ * Return: Page's zone_device_data if owned by DRM pagemap, NULL otherwise
  */
 static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct page *page)
 {
 	struct folio *folio = page_folio(page);
 
+	if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
+		return NULL;
+
 	return folio_zone_device_data(folio);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/gpusvm,pagemap: Do not assume DRM pagemap owns device pages
  2026-04-09  1:55 [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages Matthew Brost
@ 2026-04-10 12:46 ` Francois Dugast
  2026-04-10 20:02   ` Matthew Brost
  2026-04-17 11:42 ` [PATCH] drm/gpusvm, pagemap: " Thomas Hellström
  1 sibling, 1 reply; 5+ messages in thread
From: Francois Dugast @ 2026-04-10 12:46 UTC (permalink / raw)
  To: Matthew Brost; +Cc: intel-xe, dri-devel, himal.prasad.ghimiray

On Wed, Apr 08, 2026 at 06:55:12PM -0700, Matthew Brost wrote:
> Update drm_pagemap_page_zone_device_data() to derive the pgmap ops from
> the page and compare them against the DRM pagemap ops. If the ops do not
> match, return NULL.
> 
> Also harden two risky call sites by checking for NULL after
> hmm_range_fault() or migrate_vma_setup() when migrating to device
> memory, as it is possible to encounter device pages that are not owned
> by DRM pagemap.

Shouldn't we also harden other calls to drm_pagemap_page_zone_device_data() in

    drm_pagemap_migrate_map_device_private_pages()
    drm_pagemap_migrate_unmap_pages()
    drm_pagemap_migrate_populate_ram_pfn()
    __drm_pagemap_migrate_to_ram()
    drm_pagemap_folio_free()
    drm_pagemap_migrate_to_ram()
    drm_pagemap_page_to_dpagemap()

Francois

> 
> Suggested-by: sashiko.dev
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/drm_gpusvm.c  |  5 +++++
>  drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
>  include/drm/drm_pagemap.h     |  5 ++++-
>  3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 365a9c0b522a..b3cccd047a21 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>  			struct drm_pagemap_zdd *__zdd =
>  				drm_pagemap_page_zone_device_data(page);
>  
> +			if (!__zdd) {
> +				err = -EINVAL;
> +				goto err_unmap;
> +			}
> +
>  			if (!ctx->allow_mixed &&
>  			    zdd != __zdd && i > 0) {
>  				err = -EOPNOTSUPP;
> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index d82ea7ccb8da..95c951c5b569 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
> @@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
>  				own_pages++;
>  				goto next;
>  			}
> -			cur.dpagemap = src_zdd->dpagemap;
> -			cur.ops = src_zdd->devmem_allocation->ops;
> -			cur.device = cur.dpagemap->drm->dev;
> -			pages[i] = src_page;
> +			if (src_zdd) {
> +				cur.dpagemap = src_zdd->dpagemap;
> +				cur.ops = src_zdd->devmem_allocation->ops;
> +				cur.device = cur.dpagemap->drm->dev;
> +				pages[i] = src_page;
> +			} else {
> +				npages = i;
> +				err = -EINVAL;
> +				goto err_finalize;
> +			}
>  		}
>  		if (!pages[i]) {
>  			cur.dpagemap = NULL;
> diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> index 95eb4b66b057..9b7c50932db5 100644
> --- a/include/drm/drm_pagemap.h
> +++ b/include/drm/drm_pagemap.h
> @@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap *dpagemap);
>   * drm_pagemap_page_zone_device_data() - Page to zone_device_data
>   * @page: Pointer to the page
>   *
> - * Return: Page's zone_device_data
> + * Return: Page's zone_device_data if owned by DRM pagemap, NULL otherwise
>   */
>  static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct page *page)
>  {
>  	struct folio *folio = page_folio(page);
>  
> +	if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
> +		return NULL;
> +
>  	return folio_zone_device_data(folio);
>  }
>  
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/gpusvm,pagemap: Do not assume DRM pagemap owns device pages
  2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
@ 2026-04-10 20:02   ` Matthew Brost
  2026-04-11  9:06     ` Francois Dugast
  0 siblings, 1 reply; 5+ messages in thread
From: Matthew Brost @ 2026-04-10 20:02 UTC (permalink / raw)
  To: Francois Dugast; +Cc: intel-xe, dri-devel, himal.prasad.ghimiray

On Fri, Apr 10, 2026 at 02:46:54PM +0200, Francois Dugast wrote:
> On Wed, Apr 08, 2026 at 06:55:12PM -0700, Matthew Brost wrote:
> > Update drm_pagemap_page_zone_device_data() to derive the pgmap ops from
> > the page and compare them against the DRM pagemap ops. If the ops do not
> > match, return NULL.
> > 
> > Also harden two risky call sites by checking for NULL after
> > hmm_range_fault() or migrate_vma_setup() when migrating to device
> > memory, as it is possible to encounter device pages that are not owned
> > by DRM pagemap.
> 
> Shouldn't we also harden other calls to drm_pagemap_page_zone_device_data() in
> 
>     drm_pagemap_migrate_map_device_private_pages()
>     drm_pagemap_migrate_unmap_pages()

We sanitize prior to this in drm_pagemap_migrate_to_devmem or are
operating on pages handed back via populate_devmem_pfn.

>     drm_pagemap_migrate_populate_ram_pfn()

Operating on page handed back via populate_devmem_pfn. Also wouldn't
NULL ptr reference.

>     __drm_pagemap_migrate_to_ram()
>     drm_pagemap_folio_free()
>     drm_pagemap_migrate_to_ram()

These are in the vops path of pagemap we check against.

>     drm_pagemap_page_to_dpagemap()

We sanitize prior to this in drm_gpusvm_get_pages.

Thus all the above sites I figure a warn is enough as it would indicate
a fairly serious bug in drm gpusvm/pagemap/calling driver which this
code completely controls.

The case where I do sanitize - after collection via hmm_range_fault,
migrate_vma_setup, I think it could be possible an outside driver has
moved pages to private (very unlikely) and this driver also tries to
move, so abort rather NULL ptr dereference.

So basically mitagated the 2 risky places with sanitization. Ofc we
could check this everywhere...

Matt

> 
> Francois
> 
> > 
> > Suggested-by: sashiko.dev
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/drm_gpusvm.c  |  5 +++++
> >  drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
> >  include/drm/drm_pagemap.h     |  5 ++++-
> >  3 files changed, 19 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > index 365a9c0b522a..b3cccd047a21 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > @@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> >  			struct drm_pagemap_zdd *__zdd =
> >  				drm_pagemap_page_zone_device_data(page);
> >  
> > +			if (!__zdd) {
> > +				err = -EINVAL;
> > +				goto err_unmap;
> > +			}
> > +
> >  			if (!ctx->allow_mixed &&
> >  			    zdd != __zdd && i > 0) {
> >  				err = -EOPNOTSUPP;
> > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> > index d82ea7ccb8da..95c951c5b569 100644
> > --- a/drivers/gpu/drm/drm_pagemap.c
> > +++ b/drivers/gpu/drm/drm_pagemap.c
> > @@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
> >  				own_pages++;
> >  				goto next;
> >  			}
> > -			cur.dpagemap = src_zdd->dpagemap;
> > -			cur.ops = src_zdd->devmem_allocation->ops;
> > -			cur.device = cur.dpagemap->drm->dev;
> > -			pages[i] = src_page;
> > +			if (src_zdd) {
> > +				cur.dpagemap = src_zdd->dpagemap;
> > +				cur.ops = src_zdd->devmem_allocation->ops;
> > +				cur.device = cur.dpagemap->drm->dev;
> > +				pages[i] = src_page;
> > +			} else {
> > +				npages = i;
> > +				err = -EINVAL;
> > +				goto err_finalize;
> > +			}
> >  		}
> >  		if (!pages[i]) {
> >  			cur.dpagemap = NULL;
> > diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> > index 95eb4b66b057..9b7c50932db5 100644
> > --- a/include/drm/drm_pagemap.h
> > +++ b/include/drm/drm_pagemap.h
> > @@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap *dpagemap);
> >   * drm_pagemap_page_zone_device_data() - Page to zone_device_data
> >   * @page: Pointer to the page
> >   *
> > - * Return: Page's zone_device_data
> > + * Return: Page's zone_device_data if owned by DRM pagemap, NULL otherwise
> >   */
> >  static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct page *page)
> >  {
> >  	struct folio *folio = page_folio(page);
> >  
> > +	if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
> > +		return NULL;
> > +
> >  	return folio_zone_device_data(folio);
> >  }
> >  
> > -- 
> > 2.34.1
> > 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/gpusvm,pagemap: Do not assume DRM pagemap owns device pages
  2026-04-10 20:02   ` Matthew Brost
@ 2026-04-11  9:06     ` Francois Dugast
  0 siblings, 0 replies; 5+ messages in thread
From: Francois Dugast @ 2026-04-11  9:06 UTC (permalink / raw)
  To: Matthew Brost; +Cc: intel-xe, dri-devel, himal.prasad.ghimiray

On Fri, Apr 10, 2026 at 01:02:00PM -0700, Matthew Brost wrote:
> On Fri, Apr 10, 2026 at 02:46:54PM +0200, Francois Dugast wrote:
> > On Wed, Apr 08, 2026 at 06:55:12PM -0700, Matthew Brost wrote:
> > > Update drm_pagemap_page_zone_device_data() to derive the pgmap ops from
> > > the page and compare them against the DRM pagemap ops. If the ops do not
> > > match, return NULL.
> > > 
> > > Also harden two risky call sites by checking for NULL after
> > > hmm_range_fault() or migrate_vma_setup() when migrating to device
> > > memory, as it is possible to encounter device pages that are not owned
> > > by DRM pagemap.
> > 
> > Shouldn't we also harden other calls to drm_pagemap_page_zone_device_data() in
> > 
> >     drm_pagemap_migrate_map_device_private_pages()
> >     drm_pagemap_migrate_unmap_pages()
> 
> We sanitize prior to this in drm_pagemap_migrate_to_devmem or are
> operating on pages handed back via populate_devmem_pfn.
> 
> >     drm_pagemap_migrate_populate_ram_pfn()
> 
> Operating on page handed back via populate_devmem_pfn. Also wouldn't
> NULL ptr reference.
> 
> >     __drm_pagemap_migrate_to_ram()
> >     drm_pagemap_folio_free()
> >     drm_pagemap_migrate_to_ram()
> 
> These are in the vops path of pagemap we check against.
> 
> >     drm_pagemap_page_to_dpagemap()
> 
> We sanitize prior to this in drm_gpusvm_get_pages.
> 
> Thus all the above sites I figure a warn is enough as it would indicate
> a fairly serious bug in drm gpusvm/pagemap/calling driver which this
> code completely controls.
> 
> The case where I do sanitize - after collection via hmm_range_fault,
> migrate_vma_setup, I think it could be possible an outside driver has
> moved pages to private (very unlikely) and this driver also tries to
> move, so abort rather NULL ptr dereference.

Indeed, thanks for pointing it out and for the explanation.

Reviewed-by: Francois Dugast <francois.dugast@intel.com>

> 
> So basically mitagated the 2 risky places with sanitization. Ofc we
> could check this everywhere...
> 
> Matt
> 
> > 
> > Francois
> > 
> > > 
> > > Suggested-by: sashiko.dev
> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_gpusvm.c  |  5 +++++
> > >  drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
> > >  include/drm/drm_pagemap.h     |  5 ++++-
> > >  3 files changed, 19 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > > index 365a9c0b522a..b3cccd047a21 100644
> > > --- a/drivers/gpu/drm/drm_gpusvm.c
> > > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > > @@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> > >  			struct drm_pagemap_zdd *__zdd =
> > >  				drm_pagemap_page_zone_device_data(page);
> > >  
> > > +			if (!__zdd) {
> > > +				err = -EINVAL;
> > > +				goto err_unmap;
> > > +			}
> > > +
> > >  			if (!ctx->allow_mixed &&
> > >  			    zdd != __zdd && i > 0) {
> > >  				err = -EOPNOTSUPP;
> > > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> > > index d82ea7ccb8da..95c951c5b569 100644
> > > --- a/drivers/gpu/drm/drm_pagemap.c
> > > +++ b/drivers/gpu/drm/drm_pagemap.c
> > > @@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
> > >  				own_pages++;
> > >  				goto next;
> > >  			}
> > > -			cur.dpagemap = src_zdd->dpagemap;
> > > -			cur.ops = src_zdd->devmem_allocation->ops;
> > > -			cur.device = cur.dpagemap->drm->dev;
> > > -			pages[i] = src_page;
> > > +			if (src_zdd) {
> > > +				cur.dpagemap = src_zdd->dpagemap;
> > > +				cur.ops = src_zdd->devmem_allocation->ops;
> > > +				cur.device = cur.dpagemap->drm->dev;
> > > +				pages[i] = src_page;
> > > +			} else {
> > > +				npages = i;
> > > +				err = -EINVAL;
> > > +				goto err_finalize;
> > > +			}
> > >  		}
> > >  		if (!pages[i]) {
> > >  			cur.dpagemap = NULL;
> > > diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> > > index 95eb4b66b057..9b7c50932db5 100644
> > > --- a/include/drm/drm_pagemap.h
> > > +++ b/include/drm/drm_pagemap.h
> > > @@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap *dpagemap);
> > >   * drm_pagemap_page_zone_device_data() - Page to zone_device_data
> > >   * @page: Pointer to the page
> > >   *
> > > - * Return: Page's zone_device_data
> > > + * Return: Page's zone_device_data if owned by DRM pagemap, NULL otherwise
> > >   */
> > >  static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct page *page)
> > >  {
> > >  	struct folio *folio = page_folio(page);
> > >  
> > > +	if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
> > > +		return NULL;
> > > +
> > >  	return folio_zone_device_data(folio);
> > >  }
> > >  
> > > -- 
> > > 2.34.1
> > > 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages
  2026-04-09  1:55 [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages Matthew Brost
  2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
@ 2026-04-17 11:42 ` Thomas Hellström
  1 sibling, 0 replies; 5+ messages in thread
From: Thomas Hellström @ 2026-04-17 11:42 UTC (permalink / raw)
  To: Matthew Brost, intel-xe, dri-devel; +Cc: francois.dugast, himal.prasad.ghimiray

On Wed, 2026-04-08 at 18:55 -0700, Matthew Brost wrote:
> Update drm_pagemap_page_zone_device_data() to derive the pgmap ops
> from
> the page and compare them against the DRM pagemap ops. If the ops do
> not
> match, return NULL.
> 
> Also harden two risky call sites by checking for NULL after
> hmm_range_fault() or migrate_vma_setup() when migrating to device
> memory, as it is possible to encounter device pages that are not
> owned
> by DRM pagemap.
> 
> Suggested-by: sashiko.dev

Is there a proper (fake) email we can use?

> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/drm_gpusvm.c  |  5 +++++
>  drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
>  include/drm/drm_pagemap.h     |  5 ++++-
>  3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c
> b/drivers/gpu/drm/drm_gpusvm.c
> index 365a9c0b522a..b3cccd047a21 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm
> *gpusvm,
>  			struct drm_pagemap_zdd *__zdd =
>  				drm_pagemap_page_zone_device_data(pa
> ge);
>  
> +			if (!__zdd) {
> +				err = -EINVAL;
> +				goto err_unmap;
> +			}
> +

Here, __zdd is never dereferenced. As I understand it, the current
behaviour (before this patch) is that pages from mixed pagemaps are
generating an -EOPNOTSUPP, re-trying migration, which I think is the
correct behaviour, save possibly for foreign device-coherent pages.

Also see below reasoning WRT "owner".

>  			if (!ctx->allow_mixed &&
>  			    zdd != __zdd && i > 0) {
>  				err = -EOPNOTSUPP;
> diff --git a/drivers/gpu/drm/drm_pagemap.c
> b/drivers/gpu/drm/drm_pagemap.c
> index d82ea7ccb8da..95c951c5b569 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
> @@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct
> drm_pagemap_devmem *devmem_allocation,
>  				own_pages++;
>  				goto next;
>  			}
> -			cur.dpagemap = src_zdd->dpagemap;
> -			cur.ops = src_zdd->devmem_allocation->ops;
> -			cur.device = cur.dpagemap->drm->dev;
> -			pages[i] = src_page;
> +			if (src_zdd) {
> +				cur.dpagemap = src_zdd->dpagemap;
> +				cur.ops = src_zdd-
> >devmem_allocation->ops;
> +				cur.device = cur.dpagemap->drm->dev;
> +				pages[i] = src_page;
> +			} else {

This shouldn't happen, because we select device_private pages based on
dev_private owner. IMO we shouldn't allow non drm_pagemap
device_private pages sharing owner because we wouldn't now how to
migrate them.

> +				npages = i;
> +				err = -EINVAL;
> +				goto err_finalize;
> +			}
>  		}
>  		if (!pages[i]) {
>  			cur.dpagemap = NULL;
> diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> index 95eb4b66b057..9b7c50932db5 100644
> --- a/include/drm/drm_pagemap.h
> +++ b/include/drm/drm_pagemap.h
> @@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap
> *dpagemap);
>   * drm_pagemap_page_zone_device_data() - Page to zone_device_data
>   * @page: Pointer to the page
>   *
> - * Return: Page's zone_device_data
> + * Return: Page's zone_device_data if owned by DRM pagemap, NULL
> otherwise
>   */
>  static inline struct drm_pagemap_zdd
> *drm_pagemap_page_zone_device_data(struct page *page)
>  {
>  	struct folio *folio = page_folio(page);
>  
> +	if (WARN_ON_ONCE(page_pgmap(page)->ops !=
> drm_pagemap_pagemap_ops_get()))

Driver could be handing a separate ops. Like if we wanted to check for
wedging / unplug before trying to migrate back to system, so this
wouldn't work.

So in general I don't think this patch is correct. Perhaps we should
look at updating the docs around owner assignment, though and also
perhaps make drm_gpusvm_get_pages() a bit more permissive?

/Thomas


> +		return NULL;
> +
>  	return folio_zone_device_data(folio);
>  }
>  

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-17 11:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-09  1:55 [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages Matthew Brost
2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
2026-04-10 20:02   ` Matthew Brost
2026-04-11  9:06     ` Francois Dugast
2026-04-17 11:42 ` [PATCH] drm/gpusvm, pagemap: " Thomas Hellström

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox