* [PATCH v2 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
2026-06-17 10:35 [PATCH v2 0/5] drm/gpusvm: split MM and device state across gpusvm/range/pages Honglei Huang
@ 2026-06-17 10:35 ` Honglei Huang
2026-06-17 10:50 ` sashiko-bot
2026-06-17 10:35 ` [PATCH v2 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages Honglei Huang
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Honglei Huang @ 2026-06-17 10:35 UTC (permalink / raw)
To: sima, matthew.brost, rodrigo.vivi, thomas.hellstrom, dakr
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Oak.Zeng, Jenny-Jing.Liu, Philip.Yang, Xiaogang.Chen, Ray.Huang,
Lingshan.Zhu, Junhua.Shen, Yiru.Ma, amd-gfx, dri-devel, intel-xe,
honghuan
From: Honglei Huang <honghuan@amd.com>
drm_gpusvm_pages_flags currently mixes two status:
- MM / virtual-address state: whether the range has been (partially)
unmapped by the Linux MM, these follow the lifetime of the VMA and
are a single per VA range fact.
- Device mapping state: has_devmem_pages and has_dma_mapping,
which describe the current page mapping status held by device
itself.
Keeping both on the pages object blurs the semantics of the
abstraction of pages and VA range. So move the MM state flags onto the
range, and keep drm_gpusvm_pages_flags strictly for mapping state.
- Introduce drm_gpusvm_range_flags { migrate_devmem, unmapped,
partial_unmap } on drm_gpusvm_range.
- Shrink drm_gpusvm_pages_flags to just has_devmem_pages and
has_dma_mapping.
Side effect: drivers now need to check the unmap flags in the driver
itself to avoid handling the unmapped pages.
Mirror that bit onto drm_gpusvm_pages so the framework can
still short circuit drm_gpusvm_get_pages() under the notifier lock, and
make drm_gpusvm_range_set_unmapped() propagate it to the backing pages.
This follows Matt's review fixup for the v0 series; see the Link below.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Link: https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/623f6a50c037d9e44f6c9fbe6859a0ba7ad50177
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 14 +++++++++++---
drivers/gpu/drm/xe/xe_svm.c | 11 ++++++-----
include/drm/drm_gpusvm.h | 30 +++++++++++++++++++++++++-----
3 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 958cb605aed..6cb38c76dc7 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -641,7 +641,7 @@ drm_gpusvm_range_alloc(struct drm_gpusvm *gpusvm,
range->itree.last = ALIGN(fault_addr + 1, chunk_size) - 1;
INIT_LIST_HEAD(&range->entry);
range->pages.notifier_seq = LONG_MAX;
- range->pages.flags.migrate_devmem = migrate_devmem ? 1 : 0;
+ range->flags.migrate_devmem = migrate_devmem ? 1 : 0;
return range;
}
@@ -1784,20 +1784,28 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_has_mapping);
/**
* drm_gpusvm_range_set_unmapped() - Mark a GPU SVM range as unmapped
* @range: Pointer to the GPU SVM range structure.
+ * @pages: Pointer to the GPU SVM pages structure(s).
+ * @pages_count: Number of GPU SVM pages structure(s) passed in.
* @mmu_range: Pointer to the MMU notifier range structure.
*
* This function marks a GPU SVM range as unmapped and sets the partial_unmap flag
* if the range partially falls within the provided MMU notifier range.
*/
void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
+ struct drm_gpusvm_pages *pages,
+ unsigned int pages_count,
const struct mmu_notifier_range *mmu_range)
{
+ unsigned int i;
+
lockdep_assert_held_write(&range->gpusvm->notifier_lock);
- range->pages.flags.unmapped = true;
+ range->flags.unmapped = true;
+ for (i = 0; i < pages_count; ++i)
+ pages[i].flags.unmapped = true;
if (drm_gpusvm_range_start(range) < mmu_range->start ||
drm_gpusvm_range_end(range) > mmu_range->end)
- range->pages.flags.partial_unmap = true;
+ range->flags.partial_unmap = true;
}
EXPORT_SYMBOL_GPL(drm_gpusvm_range_set_unmapped);
diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
index e1651e70c8f..05578e187d2 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -134,7 +134,8 @@ xe_svm_garbage_collector_add_range(struct xe_vm *vm, struct xe_svm_range *range,
range_debug(range, "GARBAGE COLLECTOR ADD");
- drm_gpusvm_range_set_unmapped(&range->base, mmu_range);
+ drm_gpusvm_range_set_unmapped(&range->base, &range->pages, 1,
+ mmu_range);
spin_lock(&vm->svm.garbage_collector.lock);
if (list_empty(&range->garbage_collector_link))
@@ -166,7 +167,7 @@ xe_svm_range_notifier_event_begin(struct xe_vm *vm, struct drm_gpusvm_range *r,
range_debug(range, "NOTIFIER");
/* Skip if already unmapped or if no binding exist */
- if (range->base.pages.flags.unmapped || !range->tile_present)
+ if (range->base.flags.unmapped || !range->tile_present)
return 0;
range_debug(range, "NOTIFIER - EXECUTE");
@@ -1136,7 +1137,7 @@ bool xe_svm_range_needs_migrate_to_vram(struct xe_svm_range *range, struct xe_vm
struct xe_vm *vm = range_to_vm(&range->base);
u64 range_size = xe_svm_range_size(range);
- if (!range->base.pages.flags.migrate_devmem || !dpagemap)
+ if (!range->base.flags.migrate_devmem || !dpagemap)
return false;
xe_assert(vm->xe, IS_DGFX(vm->xe));
@@ -1248,7 +1249,7 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma,
xe_svm_range_fault_count_stats_incr(gt, range);
- if (ctx.devmem_only && !range->base.pages.flags.migrate_devmem) {
+ if (ctx.devmem_only && !range->base.flags.migrate_devmem) {
err = -EACCES;
goto out;
}
@@ -1623,7 +1624,7 @@ int xe_svm_alloc_vram(struct xe_svm_range *range, const struct drm_gpusvm_ctx *c
int err, retries = 1;
bool write_locked = false;
- xe_assert(range_to_vm(&range->base)->xe, range->base.pages.flags.migrate_devmem);
+ xe_assert(range_to_vm(&range->base)->xe, range->base.flags.migrate_devmem);
range_debug(range, "ALLOCATE VRAM");
migration_state = drm_gpusvm_scan_mm(&range->base,
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index 8a4d7134a9a..251a7266a73 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -109,9 +109,7 @@ struct drm_gpusvm_notifier {
/**
* struct drm_gpusvm_pages_flags - Structure representing a GPU SVM pages flags
*
- * @migrate_devmem: Flag indicating whether the pages can be migrated to device memory
* @unmapped: Flag indicating if the pages has been unmapped
- * @partial_unmap: Flag indicating if the pages has been partially unmapped
* @has_devmem_pages: Flag indicating if the pages has devmem pages
* @has_dma_mapping: Flag indicating if the pages has a DMA mapping
* @__flags: Flags for pages in u16 form (used for READ_ONCE)
@@ -119,11 +117,8 @@ struct drm_gpusvm_notifier {
struct drm_gpusvm_pages_flags {
union {
struct {
- /* All flags below must be set upon creation */
- u16 migrate_devmem : 1;
/* All flags below must be set / cleared under notifier lock */
u16 unmapped : 1;
- u16 partial_unmap : 1;
u16 has_devmem_pages : 1;
u16 has_dma_mapping : 1;
};
@@ -151,6 +146,27 @@ struct drm_gpusvm_pages {
struct drm_gpusvm_pages_flags flags;
};
+/**
+ * struct drm_gpusvm_range_flags - Range-level GPU SVM flags
+ *
+ * @migrate_devmem: Flag indicating whether the range can be migrated to device memory
+ * @unmapped: Flag indicating if the range has been unmapped
+ * @partial_unmap: Flag indicating if the range has been partially unmapped
+ * @__flags: All flags in u16 form (used for READ_ONCE)
+ */
+struct drm_gpusvm_range_flags {
+ union {
+ struct {
+ /* All flags below must be set upon creation */
+ u16 migrate_devmem : 1;
+ /* All flags below must be set / cleared under notifier lock */
+ u16 unmapped : 1;
+ u16 partial_unmap : 1;
+ };
+ u16 __flags;
+ };
+};
+
/**
* struct drm_gpusvm_range - Structure representing a GPU SVM range
*
@@ -160,6 +176,7 @@ struct drm_gpusvm_pages {
* @itree: Interval tree node for the range (inserted in GPU SVM notifier)
* @entry: List entry to fast interval tree traversal
* @pages: The pages for this range.
+ * @flags: Flags for range see &struct drm_gpusvm_range_flags
*
* This structure represents a GPU SVM range used for tracking memory ranges
* mapped in a DRM device.
@@ -171,6 +188,7 @@ struct drm_gpusvm_range {
struct interval_tree_node itree;
struct list_head entry;
struct drm_gpusvm_pages pages;
+ struct drm_gpusvm_range_flags flags;
};
/**
@@ -310,6 +328,8 @@ drm_gpusvm_range_find(struct drm_gpusvm_notifier *notifier, unsigned long start,
unsigned long end);
void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
+ struct drm_gpusvm_pages *pages,
+ unsigned int pages_count,
const struct mmu_notifier_range *mmu_range);
int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
2026-06-17 10:35 ` [PATCH v2 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags Honglei Huang
@ 2026-06-17 10:50 ` sashiko-bot
2026-06-17 13:54 ` Matthew Brost
0 siblings, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-06-17 10:50 UTC (permalink / raw)
To: Honglei Huang; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] Compilation error due to missing `pages` member in `struct xe_svm_range`.
Pre-existing issues:
- [High] Critical error paths swallow error codes by jumping to `out`, causing an infinite GPU page fault livelock.
--
commit c7d50cf1df054885b8dfd4565b263894bfd104b3
Author: Honglei Huang <honghuan@amd.com>
Subject: drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
This commit splits the MM state flags out of drm_gpusvm_pages_flags and moves
them to a new drm_gpusvm_range_flags structure. It keeps the mapping state
flags in drm_gpusvm_pages_flags and updates usage in the amd and xe drivers.
Link: https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/623f6a50c037d9e44f6c9fbe6859a0ba7ad50177
> diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> index e1651e70c8f01..05578e187d204 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -134,7 +134,8 @@ xe_svm_garbage_collector_add_range(struct xe_vm *vm, struct xe_svm_range *range,
>
> range_debug(range, "GARBAGE COLLECTOR ADD");
>
> - drm_gpusvm_range_set_unmapped(&range->base, mmu_range);
> + drm_gpusvm_range_set_unmapped(&range->base, &range->pages, 1,
> + mmu_range);
[Severity: Low]
Does this cause a compilation error due to pages not being a direct member
of struct xe_svm_range? It looks like the pages struct is located inside the
embedded base field (range->base.pages).
While this might be fixed in a later patch in the series, should each commit
compile successfully on its own to preserve bisection?
[ ... ]
> @@ -1248,7 +1249,7 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma,
>
> xe_svm_range_fault_count_stats_incr(gt, range);
>
> - if (ctx.devmem_only && !range->base.pages.flags.migrate_devmem) {
> + if (ctx.devmem_only && !range->base.flags.migrate_devmem) {
> err = -EACCES;
> goto out;
> }
[Severity: High]
This is a pre-existing issue, but can this error path swallow the error code by
jumping to the out label?
Looking at __xe_svm_handle_pagefault(), the out label appears to
unconditionally return 0:
out:
xe_svm_range_fault_us_stats_incr(gt, range, start);
return 0;
If we set err = -EACCES and jump to out, or if xe_svm_range_get_pages()
fails and jumps to out later in the function, does returning 0 cause the
caller to believe the page fault was handled successfully?
Since the pages weren't mapped, wouldn't the GPU immediately retry the
instruction and fault again, potentially causing an infinite livelock? Should
these error paths jump to err_out instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617103602.1491609-1-honglei1.huang@amd.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
2026-06-17 10:50 ` sashiko-bot
@ 2026-06-17 13:54 ` Matthew Brost
0 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2026-06-17 13:54 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Honglei Huang, dri-devel
On Wed, Jun 17, 2026 at 10:50:25AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Low] Compilation error due to missing `pages` member in `struct xe_svm_range`.
>
> Pre-existing issues:
> - [High] Critical error paths swallow error codes by jumping to `out`, causing an infinite GPU page fault livelock.
> --
>
> commit c7d50cf1df054885b8dfd4565b263894bfd104b3
> Author: Honglei Huang <honghuan@amd.com>
> Subject: drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
>
> This commit splits the MM state flags out of drm_gpusvm_pages_flags and moves
> them to a new drm_gpusvm_range_flags structure. It keeps the mapping state
> flags in drm_gpusvm_pages_flags and updates usage in the amd and xe drivers.
>
> Link: https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/623f6a50c037d9e44f6c9fbe6859a0ba7ad50177
>
> > diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> > index e1651e70c8f01..05578e187d204 100644
> > --- a/drivers/gpu/drm/xe/xe_svm.c
> > +++ b/drivers/gpu/drm/xe/xe_svm.c
> > @@ -134,7 +134,8 @@ xe_svm_garbage_collector_add_range(struct xe_vm *vm, struct xe_svm_range *range,
> >
> > range_debug(range, "GARBAGE COLLECTOR ADD");
> >
> > - drm_gpusvm_range_set_unmapped(&range->base, mmu_range);
> > + drm_gpusvm_range_set_unmapped(&range->base, &range->pages, 1,
> > + mmu_range);
>
> [Severity: Low]
> Does this cause a compilation error due to pages not being a direct member
> of struct xe_svm_range? It looks like the pages struct is located inside the
> embedded base field (range->base.pages).
>
> While this might be fixed in a later patch in the series, should each commit
> compile successfully on its own to preserve bisection?
>
So s/range->pages/range->base.pages/
> [ ... ]
>
> > @@ -1248,7 +1249,7 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma,
> >
> > xe_svm_range_fault_count_stats_incr(gt, range);
> >
> > - if (ctx.devmem_only && !range->base.pages.flags.migrate_devmem) {
> > + if (ctx.devmem_only && !range->base.flags.migrate_devmem) {
> > err = -EACCES;
> > goto out;
> > }
>
> [Severity: High]
> This is a pre-existing issue, but can this error path swallow the error code by
> jumping to the out label?
>
> Looking at __xe_svm_handle_pagefault(), the out label appears to
> unconditionally return 0:
>
> out:
> xe_svm_range_fault_us_stats_incr(gt, range, start);
> return 0;
>
> If we set err = -EACCES and jump to out, or if xe_svm_range_get_pages()
> fails and jumps to out later in the function, does returning 0 cause the
> caller to believe the page fault was handled successfully?
>
> Since the pages weren't mapped, wouldn't the GPU immediately retry the
> instruction and fault again, potentially causing an infinite livelock? Should
> these error paths jump to err_out instead?
>
Yep, posted a fix here: https://patchwork.freedesktop.org/series/168716/
Matt
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617103602.1491609-1-honglei1.huang@amd.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages
2026-06-17 10:35 [PATCH v2 0/5] drm/gpusvm: split MM and device state across gpusvm/range/pages Honglei Huang
2026-06-17 10:35 ` [PATCH v2 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags Honglei Huang
@ 2026-06-17 10:35 ` Honglei Huang
2026-06-17 10:49 ` sashiko-bot
2026-06-17 10:36 ` [PATCH v2 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages Honglei Huang
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Honglei Huang @ 2026-06-17 10:35 UTC (permalink / raw)
To: sima, matthew.brost, rodrigo.vivi, thomas.hellstrom, dakr
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Oak.Zeng, Jenny-Jing.Liu, Philip.Yang, Xiaogang.Chen, Ray.Huang,
Lingshan.Zhu, Junhua.Shen, Yiru.Ma, amd-gfx, dri-devel, intel-xe,
honghuan
From: Honglei Huang <honghuan@amd.com>
drm_gpusvm_pages is the layer that actually represents physical
pages/mappings it owns the dma_addr array, the dma_iova_state...
With the previous patch, so drm_gpusvm_pages is now strictly about
physical pages and their DMA view.
Since now the drm_gpusvm_pages instance is inherently bound to one
specific drm_device, make that ownership explicit by giving
drm_gpusvm_pages its own drm_device handle, and drive all DMA through
it instead of through the gpusvm:
- Add drm to struct drm_gpusvm_pages and route all DMA in
drm_gpusvm_get_pages() / __drm_gpusvm_unmap_pages() through
svm_pages->drm instead of gpusvm->drm.
- Bind svm_pages->drm where the pages object is initialised
(drm_gpusvm_range_alloc() and the xe userptr setup) and require
it to be set on entry to drm_gpusvm_get_pages(); the dma device
is immutable for the lifetime of the pages instance. A later
patch introduces drm_gpusvm_init_pages() to centralise this.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 30 ++++++++++++++++++++----------
drivers/gpu/drm/xe/xe_userptr.c | 2 ++
include/drm/drm_gpusvm.h | 2 ++
3 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 6cb38c76dc7..d6407c94cdd 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -641,6 +641,7 @@ drm_gpusvm_range_alloc(struct drm_gpusvm *gpusvm,
range->itree.last = ALIGN(fault_addr + 1, chunk_size) - 1;
INIT_LIST_HEAD(&range->entry);
range->pages.notifier_seq = LONG_MAX;
+ range->pages.drm = gpusvm->drm;
range->flags.migrate_devmem = migrate_devmem ? 1 : 0;
return range;
@@ -1135,11 +1136,16 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
unsigned long npages)
{
struct drm_pagemap *dpagemap = svm_pages->dpagemap;
- struct device *dev = gpusvm->drm->dev;
+ struct device *dev;
unsigned long i, j;
lockdep_assert_held(&gpusvm->notifier_lock);
+ if (!svm_pages->drm)
+ return;
+
+ dev = svm_pages->drm->dev;
+
if (svm_pages->flags.has_dma_mapping) {
struct drm_gpusvm_pages_flags flags = {
.__flags = svm_pages->flags.__flags,
@@ -1421,6 +1427,9 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
DMA_BIDIRECTIONAL;
struct dma_iova_state *state = &svm_pages->state;
+ if (!svm_pages->drm)
+ return -EINVAL;
+
retry:
if (time_after(jiffies, timeout))
return -EBUSY;
@@ -1520,7 +1529,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
pagemap = page_pgmap(page);
dpagemap = drm_pagemap_page_to_dpagemap(page);
- if (drm_WARN_ON(gpusvm->drm, !dpagemap)) {
+ if (drm_WARN_ON(svm_pages->drm, !dpagemap)) {
/*
* Raced. This is not supposed to happen
* since hmm_range_fault() should've migrated
@@ -1532,10 +1541,10 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
}
svm_pages->dma_addr[j] =
dpagemap->ops->device_map(dpagemap,
- gpusvm->drm->dev,
+ svm_pages->drm->dev,
page, order,
dma_dir);
- if (dma_mapping_error(gpusvm->drm->dev,
+ if (dma_mapping_error(svm_pages->drm->dev,
svm_pages->dma_addr[j].addr)) {
err = -EFAULT;
goto err_unmap;
@@ -1555,11 +1564,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
}
if (!i)
- dma_iova_try_alloc(gpusvm->drm->dev, state,
+ dma_iova_try_alloc(svm_pages->drm->dev, state,
0, npages * PAGE_SIZE);
if (dma_use_iova(state)) {
- err = dma_iova_link(gpusvm->drm->dev, state,
+ err = dma_iova_link(svm_pages->drm->dev, state,
hmm_pfn_to_phys(pfns[i]),
svm_pages->state_offset,
PAGE_SIZE << order,
@@ -1570,11 +1579,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
addr = state->addr + svm_pages->state_offset;
svm_pages->state_offset += PAGE_SIZE << order;
} else {
- addr = dma_map_page(gpusvm->drm->dev,
+ addr = dma_map_page(svm_pages->drm->dev,
page, 0,
PAGE_SIZE << order,
dma_dir);
- if (dma_mapping_error(gpusvm->drm->dev, addr)) {
+ if (dma_mapping_error(svm_pages->drm->dev, addr)) {
err = -EFAULT;
goto err_unmap;
}
@@ -1590,7 +1599,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
}
if (dma_use_iova(state)) {
- err = dma_iova_sync(gpusvm->drm->dev, state, 0,
+ err = dma_iova_sync(svm_pages->drm->dev, state, 0,
svm_pages->state_offset);
if (err)
goto err_unmap;
@@ -1640,7 +1649,8 @@ int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_range *range,
const struct drm_gpusvm_ctx *ctx)
{
- return drm_gpusvm_get_pages(gpusvm, &range->pages, gpusvm->mm,
+ return drm_gpusvm_get_pages(gpusvm, &range->pages,
+ gpusvm->mm,
&range->notifier->notifier,
drm_gpusvm_range_start(range),
drm_gpusvm_range_end(range), ctx);
diff --git a/drivers/gpu/drm/xe/xe_userptr.c b/drivers/gpu/drm/xe/xe_userptr.c
index 6761005c0b9..1b540e62af6 100644
--- a/drivers/gpu/drm/xe/xe_userptr.c
+++ b/drivers/gpu/drm/xe/xe_userptr.c
@@ -390,6 +390,7 @@ int xe_userptr_setup(struct xe_userptr_vma *uvma, unsigned long start,
unsigned long range)
{
struct xe_userptr *userptr = &uvma->userptr;
+ struct xe_vm *vm = xe_vma_vm(&uvma->vma);
int err;
INIT_LIST_HEAD(&userptr->invalidate_link);
@@ -402,6 +403,7 @@ int xe_userptr_setup(struct xe_userptr_vma *uvma, unsigned long start,
return err;
userptr->pages.notifier_seq = LONG_MAX;
+ userptr->pages.drm = &vm->xe->drm;
return 0;
}
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index 251a7266a73..842353afb27 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -129,6 +129,7 @@ struct drm_gpusvm_pages_flags {
/**
* struct drm_gpusvm_pages - Structure representing a GPU SVM mapped pages
*
+ * @drm: The DRM device that owns the dma mappings
* @dma_addr: Device address array
* @dpagemap: The struct drm_pagemap of the device pages we're dma-mapping.
* Note this is assuming only one drm_pagemap per range is allowed.
@@ -138,6 +139,7 @@ struct drm_gpusvm_pages_flags {
* @flags: Flags for the range; see &struct drm_gpusvm_pages_flags
*/
struct drm_gpusvm_pages {
+ struct drm_device *drm;
struct drm_pagemap_addr *dma_addr;
struct drm_pagemap *dpagemap;
struct dma_iova_state state;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages
2026-06-17 10:35 ` [PATCH v2 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages Honglei Huang
@ 2026-06-17 10:49 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-17 10:49 UTC (permalink / raw)
To: Honglei Huang; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Medium] The `userptr->pages.drm` field is initialized after the MMU interval notifier is inserted, exposing it to concurrent lockless access by notifier callbacks.
Pre-existing issues:
- [High] Device pages are leaked on the error path in `drm_gpusvm_get_pages()` because `svm_pages->dpagemap` is not populated before jumping to the unmap cleanup logic.
- [High] `__drm_gpusvm_unmap_pages()` incorrectly passes system pages to the `device_unmap` callback when a mixed range is mapped on an IOVA-enabled device.
- [High] Uninitialized memory read of `svm_pages->dma_addr[0].dir` in the error path of `__drm_gpusvm_unmap_pages`.
--
commit 5025fa9183334de09c92fc07865022970a487d35
Author: Honglei Huang <honghuan@amd.com>
Subject: drm/gpusvm: embed struct drm_device into drm_gpusvm_pages
drm_gpusvm_pages is the layer that actually represents physical
pages/mappings it owns the dma_addr array, the dma_iova_state...
With the previous patch, so drm_gpusvm_pages is now strictly about
physical pages and their DMA view.
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 6cb38c76dc7be..d6407c94cdda3 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1135,11 +1136,16 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> unsigned long npages)
> {
> struct drm_pagemap *dpagemap = svm_pages->dpagemap;
> - struct device *dev = gpusvm->drm->dev;
> + struct device *dev;
> unsigned long i, j;
>
> lockdep_assert_held(&gpusvm->notifier_lock);
>
> + if (!svm_pages->drm)
> + return;
> +
> + dev = svm_pages->drm->dev;
> +
> if (svm_pages->flags.has_dma_mapping) {
[Severity: High]
This is a pre-existing issue, but I wanted to ask about the logic when IOVA is
used in __drm_gpusvm_unmap_pages(). When use_iova is true, system pages
evaluating this check:
if (!use_iova && addr->proto == DRM_INTERCONNECT_SYSTEM)
dma_unmap_page(dev, addr->addr, PAGE_SIZE << addr->order, addr->dir);
else if (dpagemap && dpagemap->ops->device_unmap)
dpagemap->ops->device_unmap(dpagemap, dev, addr);
would fall through to the else if branch. If the SVM range contains both
system and device pages, dpagemap would be non-NULL, causing the system pages
to be erroneously passed to dpagemap->ops->device_unmap(). Could this result
in an incompatible address format being passed to the device pagemap driver?
[Severity: High]
Another pre-existing issue in this same function: when mapping the very first
page fails in drm_gpusvm_get_pages(), the error path calls this unmap function
with npages (or num_dma_mapped) set to 0. The code here does:
if (use_iova)
dma_iova_destroy(dev, &svm_pages->state, svm_pages->state_offset,
svm_pages->dma_addr[0].dir, 0);
Since svm_pages->dma_addr is allocated with kvmalloc_objs() (which does not
zero memory) and num_dma_mapped is 0, is it possible this passes
uninitialized memory as the DMA direction?
[ ... ]
> @@ -1590,7 +1599,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> }
>
> if (dma_use_iova(state)) {
> - err = dma_iova_sync(gpusvm->drm->dev, state, 0,
> + err = dma_iova_sync(svm_pages->drm->dev, state, 0,
> svm_pages->state_offset);
> if (err)
> goto err_unmap;
[Severity: High]
This isn't a bug introduced by this patch, but I have a question about the
error path here. In drm_gpusvm_get_pages(), if an error occurs while mapping
a device page:
if (dma_mapping_error(svm_pages->drm->dev, svm_pages->dma_addr[j].addr)) {
err = -EFAULT;
goto err_unmap;
}
The jump to err_unmap happens before svm_pages->dpagemap is populated
at the end of the loop. When __drm_gpusvm_unmap_pages() is then called,
it relies on dpagemap which resolves from svm_pages->dpagemap (which is
still NULL). Does this mean the device's unmap callback is skipped, leaking
all device pages previously mapped in the current batch?
[ ... ]
> diff --git a/drivers/gpu/drm/xe/xe_userptr.c b/drivers/gpu/drm/xe/xe_userptr.c
> index 6761005c0b900..1b540e62af6c3 100644
> --- a/drivers/gpu/drm/xe/xe_userptr.c
> +++ b/drivers/gpu/drm/xe/xe_userptr.c
> @@ -390,6 +390,7 @@ int xe_userptr_setup(struct xe_userptr_vma *uvma, unsigned long start,
> unsigned long range)
> {
> struct xe_userptr *userptr = &uvma->userptr;
> + struct xe_vm *vm = xe_vma_vm(&uvma->vma);
> int err;
>
> INIT_LIST_HEAD(&userptr->invalidate_link);
> @@ -402,6 +403,7 @@ int xe_userptr_setup(struct xe_userptr_vma *uvma, unsigned long start,
> return err;
>
> userptr->pages.notifier_seq = LONG_MAX;
> + userptr->pages.drm = &vm->xe->drm;
>
> return 0;
> }
[Severity: Medium]
Could this ordering cause a data race? The MMU interval notifier is inserted
just before this assignment:
err = mmu_interval_notifier_insert(&userptr->notifier, current->mm,
start, range,
&vma_userptr_notifier_ops);
if (err)
return err;
This immediately exposes the notifier to asynchronous invalidation callbacks.
If the invalidation callback executes concurrently on another CPU, it might
read userptr->pages.drm before it is initialized here, leading to potential
KCSAN warnings or lockless access issues. Should the initialization of
userptr->pages.drm happen before the notifier is inserted?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617103602.1491609-1-honglei1.huang@amd.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages
2026-06-17 10:35 [PATCH v2 0/5] drm/gpusvm: split MM and device state across gpusvm/range/pages Honglei Huang
2026-06-17 10:35 ` [PATCH v2 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags Honglei Huang
2026-06-17 10:35 ` [PATCH v2 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages Honglei Huang
@ 2026-06-17 10:36 ` Honglei Huang
2026-06-17 10:56 ` sashiko-bot
2026-06-17 10:36 ` [PATCH v2 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range Honglei Huang
2026-06-17 10:36 ` [PATCH v2 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level Honglei Huang
4 siblings, 1 reply; 11+ messages in thread
From: Honglei Huang @ 2026-06-17 10:36 UTC (permalink / raw)
To: sima, matthew.brost, rodrigo.vivi, thomas.hellstrom, dakr
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Oak.Zeng, Jenny-Jing.Liu, Philip.Yang, Xiaogang.Chen, Ray.Huang,
Lingshan.Zhu, Junhua.Shen, Yiru.Ma, amd-gfx, dri-devel, intel-xe,
honghuan
From: Honglei Huang <honghuan@amd.com>
With drm_gpusvm_pages now self contained, make xe stop relying
on the drm_gpusvm_range pages and take responsibility for the page
lifecycle on the driver side.
Driver side (xe):
- Embed struct drm_gpusvm_pages in xe_svm_range and route all
xe accesses through it instead of range->base.pages.
- Initialise the embedded pages via drm_gpusvm_init_pages(), which
binds the owning &xe->drm up front, and take over the page
lifecycle: xe_svm_range_get_pages() calls drm_gpusvm_get_pages()
directly; the notifier event_end and xe_svm_range_free() paths
drive unmap/free on the embedded pages object.
- Convert the open-coded userptr pages init in xe_userptr_setup()
to the same drm_gpusvm_init_pages() helper.
- Switch xe_svm_range_pages_valid() to drm_gpusvm_pages_valid().
Framework side (drm_gpusvm):
- Add a small inline drm_gpusvm_init_pages() helper that records the
owning drm_device and initialises the per-pages state, giving
drivers a single hook to extend.
- Export drm_gpusvm_pages_valid() to let driver owned pages
can query mapping state without going through a range.
- Lifecycle change: drm_gpusvm_range_remove() no longer *triggers*
unmap/free of the embedded pages. The unmap/free logic itself stays
in the framework -- drm_gpusvm_free_pages() still performs the DMA
unmap (as an idempotent backstop) and frees the dma_addr array --
but the driver now owns *when* it runs, since the driver owns the
drm_gpusvm_pages object.
Side effect / contract: a driver that owns a drm_gpusvm_pages is now
responsible for its lifecycle: drm_gpusvm_init_pages() before first
use, and drm_gpusvm_free_pages() when the owner goes away. Xe does the
latter from its ops->range_free callback, which the framework invokes
once the range refcount drops to zero in drm_gpusvm_range_remove().
The timely DMA unmap for the IOMMU security model still happens in the
notifier invalidate path via drm_gpusvm_unmap_pages(); the unmap inside
drm_gpusvm_free_pages() is only a backstop for pages that were never
invalidated.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 14 ++++++++------
drivers/gpu/drm/xe/xe_pt.c | 2 +-
drivers/gpu/drm/xe/xe_svm.c | 22 +++++++++++++++-------
drivers/gpu/drm/xe/xe_svm.h | 6 ++++--
drivers/gpu/drm/xe/xe_userptr.c | 3 +--
include/drm/drm_gpusvm.h | 19 +++++++++++++++++++
6 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index d6407c94cdd..7af535853c3 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1228,12 +1228,15 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_free_pages);
* This function removes the specified GPU SVM range and also removes the parent
* GPU SVM notifier if no more ranges remain in the notifier. The caller must
* hold a lock to protect range and notifier removal.
+ *
+ * This function does not unmap or free the drm_gpusvm_pages; the driver owns
+ * that lifecycle and is expected to release them from its
+ * &drm_gpusvm_ops.range_free callback (invoked once the range refcount drops
+ * to zero via drm_gpusvm_range_put() below).
*/
void drm_gpusvm_range_remove(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_range *range)
{
- unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
- drm_gpusvm_range_end(range));
struct drm_gpusvm_notifier *notifier;
drm_gpusvm_driver_lock_held(gpusvm);
@@ -1245,8 +1248,6 @@ void drm_gpusvm_range_remove(struct drm_gpusvm *gpusvm,
return;
drm_gpusvm_notifier_lock(gpusvm);
- __drm_gpusvm_unmap_pages(gpusvm, &range->pages, npages);
- __drm_gpusvm_free_pages(gpusvm, &range->pages);
__drm_gpusvm_range_remove(notifier, range);
drm_gpusvm_notifier_unlock(gpusvm);
@@ -1325,13 +1326,14 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_range_put);
*
* Return: True if GPU SVM range has valid pages, False otherwise
*/
-static bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_pages *svm_pages)
+bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
+ struct drm_gpusvm_pages *svm_pages)
{
lockdep_assert_held(&gpusvm->notifier_lock);
return svm_pages->flags.has_devmem_pages || svm_pages->flags.has_dma_mapping;
}
+EXPORT_SYMBOL_GPL(drm_gpusvm_pages_valid);
/**
* drm_gpusvm_range_pages_valid() - GPU SVM range pages valid
diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index 2669ff5ee74..e82b0d8fab1 100644
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -758,7 +758,7 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
return -EAGAIN;
}
if (xe_svm_range_has_dma_mapping(range)) {
- xe_res_first_dma(range->base.pages.dma_addr, 0,
+ xe_res_first_dma(range->pages.dma_addr, 0,
xe_svm_range_size(range),
&curs);
xe_svm_range_debug(range, "BIND PREPARE - MIXED");
diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
index 05578e187d2..77af0a8de63 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -66,7 +66,7 @@ static bool xe_svm_range_in_vram(struct xe_svm_range *range)
struct drm_gpusvm_pages_flags flags = {
/* Pairs with WRITE_ONCE in drm_gpusvm.c */
- .__flags = READ_ONCE(range->base.pages.flags.__flags),
+ .__flags = READ_ONCE(range->pages.flags.__flags),
};
return flags.has_devmem_pages;
@@ -96,7 +96,7 @@ static struct xe_vm *range_to_vm(struct drm_gpusvm_range *r)
(r__)->base.gpusvm, \
xe_svm_range_in_vram((r__)) ? 1 : 0, \
xe_svm_range_has_vram_binding((r__)) ? 1 : 0, \
- (r__)->base.pages.notifier_seq, \
+ (r__)->pages.notifier_seq, \
xe_svm_range_start((r__)), xe_svm_range_end((r__)), \
xe_svm_range_size((r__)))
@@ -115,6 +115,7 @@ xe_svm_range_alloc(struct drm_gpusvm *gpusvm)
return NULL;
INIT_LIST_HEAD(&range->garbage_collector_link);
+ drm_gpusvm_init_pages(&range->pages, &gpusvm_to_vm(gpusvm)->xe->drm);
xe_vm_get(gpusvm_to_vm(gpusvm));
return &range->base;
@@ -122,8 +123,10 @@ xe_svm_range_alloc(struct drm_gpusvm *gpusvm)
static void xe_svm_range_free(struct drm_gpusvm_range *range)
{
+ drm_gpusvm_free_pages(range->gpusvm, &(to_xe_range(range)->pages),
+ drm_gpusvm_range_size(range) >> PAGE_SHIFT);
xe_vm_put(range_to_vm(range));
- kfree(range);
+ kfree(to_xe_range(range));
}
static void
@@ -209,7 +212,8 @@ xe_svm_range_notifier_event_end(struct xe_vm *vm, struct drm_gpusvm_range *r,
xe_svm_assert_in_notifier(vm);
- drm_gpusvm_range_unmap_pages(&vm->svm.gpusvm, r, &ctx);
+ drm_gpusvm_unmap_pages(&vm->svm.gpusvm, &(to_xe_range(r)->pages),
+ drm_gpusvm_range_size(r) >> PAGE_SHIFT, &ctx);
if (!xe_vm_is_closed(vm) && mmu_range->event == MMU_NOTIFY_UNMAP)
xe_svm_garbage_collector_add_range(vm, to_xe_range(r),
mmu_range);
@@ -953,7 +957,7 @@ void xe_svm_fini(struct xe_vm *vm)
static bool xe_svm_range_has_pagemap_locked(const struct xe_svm_range *range,
const struct drm_pagemap *dpagemap)
{
- return range->base.pages.dpagemap == dpagemap;
+ return range->pages.dpagemap == dpagemap;
}
static bool xe_svm_range_has_pagemap(struct xe_svm_range *range,
@@ -1018,7 +1022,7 @@ bool xe_svm_range_validate(struct xe_vm *vm,
if (dpagemap)
ret = ret && xe_svm_range_has_pagemap_locked(range, dpagemap);
else
- ret = ret && !range->base.pages.dpagemap;
+ ret = ret && !range->pages.dpagemap;
xe_svm_notifier_unlock(vm);
@@ -1508,7 +1512,11 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
{
int err = 0;
- err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx);
+ err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages,
+ vm->svm.gpusvm.mm,
+ &range->base.notifier->notifier,
+ drm_gpusvm_range_start(&range->base),
+ drm_gpusvm_range_end(&range->base), ctx);
if (err == -EOPNOTSUPP) {
range_debug(range, "PAGE FAULT - EVICT PAGES");
drm_gpusvm_range_evict(&vm->svm.gpusvm, &range->base);
diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
index b7b8eeacf19..1423ab2f1d6 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -31,6 +31,8 @@ struct xe_vram_region;
struct xe_svm_range {
/** @base: base drm_gpusvm_range */
struct drm_gpusvm_range base;
+ /** @pages: Page/DMA mapping state for this range (single drm_device). */
+ struct drm_gpusvm_pages pages;
/**
* @garbage_collector_link: Link into VM's garbage collect SVM range
* list. Protected by VM's garbage collect lock.
@@ -74,7 +76,7 @@ struct xe_pagemap {
*/
static inline bool xe_svm_range_pages_valid(struct xe_svm_range *range)
{
- return drm_gpusvm_range_pages_valid(range->base.gpusvm, &range->base);
+ return drm_gpusvm_pages_valid(range->base.gpusvm, &range->pages);
}
int xe_devm_add(struct xe_tile *tile, struct xe_vram_region *vr);
@@ -132,7 +134,7 @@ void *xe_svm_private_page_owner(struct xe_vm *vm, bool force_smem);
static inline bool xe_svm_range_has_dma_mapping(struct xe_svm_range *range)
{
lockdep_assert_held(&range->base.gpusvm->notifier_lock);
- return range->base.pages.flags.has_dma_mapping;
+ return range->pages.flags.has_dma_mapping;
}
/**
diff --git a/drivers/gpu/drm/xe/xe_userptr.c b/drivers/gpu/drm/xe/xe_userptr.c
index 1b540e62af6..33ae39ac856 100644
--- a/drivers/gpu/drm/xe/xe_userptr.c
+++ b/drivers/gpu/drm/xe/xe_userptr.c
@@ -402,8 +402,7 @@ int xe_userptr_setup(struct xe_userptr_vma *uvma, unsigned long start,
if (err)
return err;
- userptr->pages.notifier_seq = LONG_MAX;
- userptr->pages.drm = &vm->xe->drm;
+ drm_gpusvm_init_pages(&userptr->pages, &vm->xe->drm);
return 0;
}
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index 842353afb27..e32d3bcb47b 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -310,6 +310,9 @@ void drm_gpusvm_range_put(struct drm_gpusvm_range *range);
bool drm_gpusvm_range_pages_valid(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_range *range);
+bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
+ struct drm_gpusvm_pages *svm_pages);
+
int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_range *range,
const struct drm_gpusvm_ctx *ctx);
@@ -350,6 +353,22 @@ void drm_gpusvm_free_pages(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_pages *svm_pages,
unsigned long npages);
+/**
+ * drm_gpusvm_init_pages() - Initialize a freshly allocated drm_gpusvm_pages
+ * @svm_pages: Pointer to the drm_gpusvm_pages to initialize.
+ * @drm: The DRM device that will own DMA mappings for this pages object.
+ *
+ * Drivers that embed one or more drm_gpusvm_pages in their own range
+ * structure must call this once on each pages instance after allocation,
+ * before the first drm_gpusvm_get_pages() / unmap / free.
+ */
+static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
+ struct drm_device *drm)
+{
+ svm_pages->drm = drm;
+ svm_pages->notifier_seq = LONG_MAX;
+}
+
/**
* enum drm_gpusvm_scan_result - Scan result from the drm_gpusvm_scan_mm() function.
* @DRM_GPUSVM_SCAN_UNPOPULATED: At least one page was not present or inaccessible.
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages
2026-06-17 10:36 ` [PATCH v2 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages Honglei Huang
@ 2026-06-17 10:56 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-17 10:56 UTC (permalink / raw)
To: Honglei Huang; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Device memory mappings are leaked in the error path of drm_gpusvm_get_pages() because device_unmap is skipped.
--
commit 25e4cbe499297aaf84b70f35d35aa1d25e43829b
Author: Honglei Huang <honghuan@amd.com>
drm/xe: have xe_svm_range embed one drm_gpusvm_pages
With drm_gpusvm_pages now self contained, make xe stop relying
on the drm_gpusvm_range pages and take responsibility for the page
lifecycle on the driver side.
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index d6407c94cdda3..7af535853c380 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
[ ... ]
> @@ -1245,8 +1248,6 @@ void drm_gpusvm_range_remove(struct drm_gpusvm *gpusvm,
> return;
>
> drm_gpusvm_notifier_lock(gpusvm);
> - __drm_gpusvm_unmap_pages(gpusvm, &range->pages, npages);
> - __drm_gpusvm_free_pages(gpusvm, &range->pages);
[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
drm_gpusvm_get_pages() leak device memory mappings?
In drm_gpusvm_get_pages(), if a device page is successfully mapped via
dpagemap->ops->device_map(), but a subsequent page mapping fails, the code
jumps to the err_unmap label and calls __drm_gpusvm_unmap_pages() to clean up:
err_unmap:
svm_pages->flags.has_dma_mapping = true;
__drm_gpusvm_unmap_pages(gpusvm, svm_pages, num_dma_mapped);
Inside __drm_gpusvm_unmap_pages(), the loop uses the dpagemap loaded from
svm_pages->dpagemap to resolve the callback:
else if (dpagemap && dpagemap->ops->device_unmap)
dpagemap->ops->device_unmap(dpagemap,
dev, addr);
Since svm_pages->dpagemap isn't updated until the end of drm_gpusvm_get_pages()
upon complete success, it remains NULL (or holds a stale reference) during
err_unmap. Will this evaluate to false and skip the required device_unmap
operations for the newly mapped device pages?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617103602.1491609-1-honglei1.huang@amd.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range
2026-06-17 10:35 [PATCH v2 0/5] drm/gpusvm: split MM and device state across gpusvm/range/pages Honglei Huang
` (2 preceding siblings ...)
2026-06-17 10:36 ` [PATCH v2 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages Honglei Huang
@ 2026-06-17 10:36 ` Honglei Huang
2026-06-17 10:52 ` sashiko-bot
2026-06-17 10:36 ` [PATCH v2 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level Honglei Huang
4 siblings, 1 reply; 11+ messages in thread
From: Honglei Huang @ 2026-06-17 10:36 UTC (permalink / raw)
To: sima, matthew.brost, rodrigo.vivi, thomas.hellstrom, dakr
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Oak.Zeng, Jenny-Jing.Liu, Philip.Yang, Xiaogang.Chen, Ray.Huang,
Lingshan.Zhu, Junhua.Shen, Yiru.Ma, amd-gfx, dri-devel, intel-xe,
honghuan
From: Honglei Huang <honghuan@amd.com>
Since the pages the physical pages and MM VA range has been abstractly
separated. Unbinding a single form of physical page from the MM VA
range, brings flexibility to the drm gpu SVM framework, transfer the
way of management of MM and device physical pages to the driver layer.
framework's range embedded pages object and its range level wrappers
have no users left. Remove the following:
- Drop pages in drm_gpusvm_range.
- Drop drm_gpusvm_range_pages_valid(), drm_gpusvm_range_get_pages()
and drm_gpusvm_range_unmap_pages(); drivers should use the
drm_gpusvm_pages helpers (drm_gpusvm_pages_valid,
drm_gpusvm_get_pages, drm_gpusvm_unmap_pages) directly on a
pages object they own.
- Drop the notifier_seq seeding in drm_gpusvm_range_alloc();
drivers initialise notifier_seq on their own pages object.
Update the DOC: overview to match the new model: document struct
drm_gpusvm_pages and the driver owned 1:1 / N:1 layouts, and rewrite the
usage examples to operate on a driver embedded pages object by the
drm_gpusvm_pages helpers instead of the removed range level wrappers.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 161 ++++++++++++++++++-----------------
include/drm/drm_gpusvm.h | 13 ---
2 files changed, 82 insertions(+), 92 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 7af535853c3..bb186719b17 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -48,6 +48,45 @@
* event. As mentioned above, ranges are tracked in a notifier's Red-Black
* tree.
*
+ * - Pages:
+ * struct drm_gpusvm_pages holds the DMA mapping state for a range of
+ * CPU virtual addresses: the DMA mapped device addresses,
+ * the device private pagemap, the IOVA state, the per mapping
+ * notifier sequence number, and the drm_device that owns the DMA
+ * mappings.
+ * A driver embeds one or more struct drm_gpusvm_pages alongside its
+ * struct drm_gpusvm_range, choosing one of two layouts:
+ *
+ * 1:1 - one drm_gpusvm_pages per range (one drm_device). Simplest
+ * layout; to mirror a VA range on several devices a driver uses a
+ * separate range (and notifier) per device, so the HMM fault is taken
+ * once per device.
+ *
+ * N:1 - one drm_gpusvm_pages per drm_device, all sharing one range and
+ * notifier; only the per-device DMA mapping differs. The instances must
+ * sit in contiguous memory so a single drm_gpusvm_range_set_unmapped()
+ * can mark them all. A driver can keep one instance inline for the single
+ * device case and switch to a heap array only when more devices join,
+ * e.g.
+ *
+ * struct driver_range {
+ * struct drm_gpusvm_range base;
+ * unsigned int num_pages; // 1: inline_pages, >1: pages[]
+ * union {
+ * struct drm_gpusvm_pages inline_pages;
+ * struct drm_gpusvm_pages *pages;
+ * };
+ * };
+ *
+ * In the N:1 case the driver allocates the pages array (e.g. with
+ * kmalloc_array(num_pages, ...)), initialises each entry with
+ * drm_gpusvm_init_pages(), and frees each entry with
+ * drm_gpusvm_free_pages() plus the array itself, from its range free
+ * callback. Each drm_gpusvm_pages is mapped independently by their own
+ * drm_device.
+ * Each drm_gpusvm_pages must be initialised with drm_gpusvm_init_pages()
+ * and released with drm_gpusvm_free_pages() in driver range free callback.
+ *
* - Operations:
* Define the interface for driver-specific GPU SVM operations such as
* range allocation, notifier allocation, and invalidations.
@@ -92,7 +131,7 @@
* range RB tree and list, as well as the range's DMA mappings and sequence
* number. GPU SVM manages all necessary locking and unlocking operations,
* except for the recheck range's pages being valid
- * (drm_gpusvm_range_pages_valid) when the driver is committing GPU bindings.
+ * (drm_gpusvm_pages_valid) when the driver is committing GPU bindings.
* This lock corresponds to the ``driver->update`` lock mentioned in
* Documentation/mm/hmm.rst. Future revisions may transition from a GPU SVM
* global lock to a per-notifier lock if finer-grained locking is deemed
@@ -140,15 +179,20 @@
*
* .. code-block:: c
*
- * int driver_bind_range(struct drm_gpusvm *gpusvm, struct drm_gpusvm_range *range)
+ * struct driver_range {
+ * struct drm_gpusvm_range base;
+ * struct drm_gpusvm_pages pages;
+ * };
+ *
+ * int driver_bind_range(struct drm_gpusvm *gpusvm, struct driver_range *drange)
* {
* int err = 0;
*
- * driver_alloc_and_setup_memory_for_bind(gpusvm, range);
+ * driver_alloc_and_setup_memory_for_bind(gpusvm, drange);
*
* drm_gpusvm_notifier_lock(gpusvm);
- * if (drm_gpusvm_range_pages_valid(range))
- * driver_commit_bind(gpusvm, range);
+ * if (drm_gpusvm_pages_valid(gpusvm, &drange->pages))
+ * driver_commit_bind(gpusvm, drange);
* else
* err = -EAGAIN;
* drm_gpusvm_notifier_unlock(gpusvm);
@@ -160,6 +204,8 @@
* unsigned long gpuva_start, unsigned long gpuva_end)
* {
* struct drm_gpusvm_ctx ctx = {};
+ * struct driver_range *drange;
+ * struct drm_gpusvm_range *range;
* int err;
*
* driver_svm_lock();
@@ -174,6 +220,7 @@
* err = PTR_ERR(range);
* goto unlock;
* }
+ * drange = container_of(range, struct driver_range, base);
*
* if (driver_migration_policy(range)) {
* err = drm_pagemap_populate_mm(driver_choose_drm_pagemap(),
@@ -183,7 +230,10 @@
* goto retry;
* }
*
- * err = drm_gpusvm_range_get_pages(gpusvm, range, &ctx);
+ * err = drm_gpusvm_get_pages(gpusvm, &drange->pages,
+ * gpusvm->mm, &range->notifier->notifier,
+ * drm_gpusvm_range_start(range),
+ * drm_gpusvm_range_end(range), &ctx);
* if (err == -EOPNOTSUPP || err == -EFAULT || err == -EPERM) { // CPU mappings changed
* if (err == -EOPNOTSUPP)
* drm_gpusvm_range_evict(gpusvm, range);
@@ -192,7 +242,7 @@
* goto unlock;
* }
*
- * err = driver_bind_range(gpusvm, range);
+ * err = driver_bind_range(gpusvm, drange);
* if (err == -EAGAIN) // CPU mappings changed
* goto retry
*
@@ -205,6 +255,22 @@
*
* .. code-block:: c
*
+ * // The driver owns the drm_gpusvm_pages lifecycle. The natural place
+ * // to release it is the ops->range_free callback, which the framework
+ * // invokes when the range refcount drops to zero inside
+ * // drm_gpusvm_range_remove(). drm_gpusvm_free_pages() unmaps any
+ * // lingering DMA mapping and a no-op if already unmapped and frees the
+ * // dma_addr array.
+ * void driver_range_free(struct drm_gpusvm_range *range)
+ * {
+ * struct driver_range *drange =
+ * container_of(range, struct driver_range, base);
+ *
+ * drm_gpusvm_free_pages(range->gpusvm, &drange->pages,
+ * drm_gpusvm_range_size(range) >> PAGE_SHIFT);
+ * kfree(drange);
+ * }
+ *
* void __driver_garbage_collector(struct drm_gpusvm *gpusvm,
* struct drm_gpusvm_range *range)
* {
@@ -215,6 +281,7 @@
* drm_gpusvm_range_evict(gpusvm, range);
*
* driver_unbind_range(range);
+ * // Pages are released by driver_range_free() (ops->range_free).
* drm_gpusvm_range_remove(gpusvm, range);
* }
*
@@ -236,17 +303,22 @@
* {
* struct drm_gpusvm_ctx ctx = { .in_notifier = true, };
* struct drm_gpusvm_range *range = NULL;
+ * struct driver_range *drange;
*
* driver_invalidate_device_pages(gpusvm, mmu_range->start, mmu_range->end);
*
* drm_gpusvm_for_each_range(range, notifier, mmu_range->start,
* mmu_range->end) {
- * drm_gpusvm_range_unmap_pages(gpusvm, range, &ctx);
+ * drange = container_of(range, struct driver_range, base);
+ *
+ * drm_gpusvm_unmap_pages(gpusvm, &drange->pages,
+ * drm_gpusvm_range_size(range) >> PAGE_SHIFT,
+ * &ctx);
*
* if (mmu_range->event != MMU_NOTIFY_UNMAP)
* continue;
*
- * drm_gpusvm_range_set_unmapped(range, mmu_range);
+ * drm_gpusvm_range_set_unmapped(range, &drange->pages, 1, mmu_range);
* driver_garbage_collector_add(gpusvm, range);
* }
* }
@@ -640,8 +712,6 @@ drm_gpusvm_range_alloc(struct drm_gpusvm *gpusvm,
range->itree.start = ALIGN_DOWN(fault_addr, chunk_size);
range->itree.last = ALIGN(fault_addr + 1, chunk_size) - 1;
INIT_LIST_HEAD(&range->entry);
- range->pages.notifier_seq = LONG_MAX;
- range->pages.drm = gpusvm->drm;
range->flags.migrate_devmem = migrate_devmem ? 1 : 0;
return range;
@@ -930,7 +1000,7 @@ drm_gpusvm_range_chunk_size(struct drm_gpusvm *gpusvm,
* mallocs 16k but the CPU VMA is ~128k which results in 64k SVM
* ranges. When migrating the SVM ranges, some processes fail in
* drm_pagemap_migrate_to_devmem with 'migrate.cpages != npages'
- * and then upon drm_gpusvm_range_get_pages device pages from
+ * and then upon drm_gpusvm_get_pages device pages from
* other processes are collected + faulted in which creates all
* sorts of problems. Unsure exactly how this happening, also
* problem goes away if 'xe_exec_system_allocator --r
@@ -1335,27 +1405,6 @@ bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
}
EXPORT_SYMBOL_GPL(drm_gpusvm_pages_valid);
-/**
- * drm_gpusvm_range_pages_valid() - GPU SVM range pages valid
- * @gpusvm: Pointer to the GPU SVM structure
- * @range: Pointer to the GPU SVM range structure
- *
- * This function determines if a GPU SVM range pages are valid. Expected be
- * called holding gpusvm->notifier_lock and as the last step before committing a
- * GPU binding. This is akin to a notifier seqno check in the HMM documentation
- * but due to wider notifiers (i.e., notifiers which span multiple ranges) this
- * function is required for finer grained checking (i.e., per range) if pages
- * are valid.
- *
- * Return: True if GPU SVM range has valid pages, False otherwise
- */
-bool drm_gpusvm_range_pages_valid(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_range *range)
-{
- return drm_gpusvm_pages_valid(gpusvm, &range->pages);
-}
-EXPORT_SYMBOL_GPL(drm_gpusvm_range_pages_valid);
-
/**
* drm_gpusvm_pages_valid_unlocked() - GPU SVM pages valid unlocked
* @gpusvm: Pointer to the GPU SVM structure
@@ -1636,29 +1685,6 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
}
EXPORT_SYMBOL_GPL(drm_gpusvm_get_pages);
-/**
- * drm_gpusvm_range_get_pages() - Get pages for a GPU SVM range
- * @gpusvm: Pointer to the GPU SVM structure
- * @range: Pointer to the GPU SVM range structure
- * @ctx: GPU SVM context
- *
- * This function gets pages for a GPU SVM range and ensures they are mapped for
- * DMA access.
- *
- * Return: 0 on success, negative error code on failure.
- */
-int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_range *range,
- const struct drm_gpusvm_ctx *ctx)
-{
- return drm_gpusvm_get_pages(gpusvm, &range->pages,
- gpusvm->mm,
- &range->notifier->notifier,
- drm_gpusvm_range_start(range),
- drm_gpusvm_range_end(range), ctx);
-}
-EXPORT_SYMBOL_GPL(drm_gpusvm_range_get_pages);
-
/**
* drm_gpusvm_unmap_pages() - Unmap GPU svm pages
* @gpusvm: Pointer to the GPU SVM structure
@@ -1689,29 +1715,6 @@ void drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
}
EXPORT_SYMBOL_GPL(drm_gpusvm_unmap_pages);
-/**
- * drm_gpusvm_range_unmap_pages() - Unmap pages associated with a GPU SVM range
- * @gpusvm: Pointer to the GPU SVM structure
- * @range: Pointer to the GPU SVM range structure
- * @ctx: GPU SVM context
- *
- * This function unmaps pages associated with a GPU SVM range. If @in_notifier
- * is set, it is assumed that gpusvm->notifier_lock is held in write mode; if it
- * is clear, it acquires gpusvm->notifier_lock in read mode. Must be called on
- * each GPU SVM range attached to notifier in gpusvm->ops->invalidate for IOMMU
- * security model.
- */
-void drm_gpusvm_range_unmap_pages(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_range *range,
- const struct drm_gpusvm_ctx *ctx)
-{
- unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
- drm_gpusvm_range_end(range));
-
- return drm_gpusvm_unmap_pages(gpusvm, &range->pages, npages, ctx);
-}
-EXPORT_SYMBOL_GPL(drm_gpusvm_range_unmap_pages);
-
/**
* drm_gpusvm_range_evict() - Evict GPU SVM range
* @gpusvm: Pointer to the GPU SVM structure
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index e32d3bcb47b..5edfa7d0c36 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -177,7 +177,6 @@ struct drm_gpusvm_range_flags {
* @refcount: Reference count for the range
* @itree: Interval tree node for the range (inserted in GPU SVM notifier)
* @entry: List entry to fast interval tree traversal
- * @pages: The pages for this range.
* @flags: Flags for range see &struct drm_gpusvm_range_flags
*
* This structure represents a GPU SVM range used for tracking memory ranges
@@ -189,7 +188,6 @@ struct drm_gpusvm_range {
struct kref refcount;
struct interval_tree_node itree;
struct list_head entry;
- struct drm_gpusvm_pages pages;
struct drm_gpusvm_range_flags flags;
};
@@ -307,20 +305,9 @@ drm_gpusvm_range_get(struct drm_gpusvm_range *range);
void drm_gpusvm_range_put(struct drm_gpusvm_range *range);
-bool drm_gpusvm_range_pages_valid(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_range *range);
-
bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_pages *svm_pages);
-int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_range *range,
- const struct drm_gpusvm_ctx *ctx);
-
-void drm_gpusvm_range_unmap_pages(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_range *range,
- const struct drm_gpusvm_ctx *ctx);
-
bool drm_gpusvm_has_mapping(struct drm_gpusvm *gpusvm, unsigned long start,
unsigned long end);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range
2026-06-17 10:36 ` [PATCH v2 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range Honglei Huang
@ 2026-06-17 10:52 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-17 10:52 UTC (permalink / raw)
To: Honglei Huang; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Concurrent read-modify-write on `pages[i].flags.unmapped` bitfield causes a data race with lockless readers.
--
commit 48046c418df88520e7f847caa854ce19133469f6
Author: Honglei Huang <honghuan@amd.com>
drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range
This commit abstracts the tracking of physical pages from the MM VA range
in the DRM GPU SVM framework. It removes the embedded pages object from
drm_gpusvm_range, transferring the management of device physical pages
directly to the driver layer using drm_gpusvm_pages helpers.
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 958cb605aedd5..bb186719b17c4 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
[ ... ]
> @@ -1799,20 +1799,28 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_has_mapping);
> /**
> * drm_gpusvm_range_set_unmapped() - Mark a GPU SVM range as unmapped
> * @range: Pointer to the GPU SVM range structure.
> + * @pages: Pointer to the GPU SVM pages structure(s).
> + * @pages_count: Number of GPU SVM pages structure(s) passed in.
> * @mmu_range: Pointer to the MMU notifier range structure.
> *
> * This function marks a GPU SVM range as unmapped and sets the partial_unmap flag
> * if the range partially falls within the provided MMU notifier range.
> */
> void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
> + struct drm_gpusvm_pages *pages,
> + unsigned int pages_count,
> const struct mmu_notifier_range *mmu_range)
> {
> + unsigned int i;
> +
> lockdep_assert_held_write(&range->gpusvm->notifier_lock);
>
> - range->pages.flags.unmapped = true;
> + range->flags.unmapped = true;
> + for (i = 0; i < pages_count; ++i)
> + pages[i].flags.unmapped = true;
[Severity: Medium]
Could this bitfield assignment introduce a regression by causing a data race
with lockless readers?
Since unmapped shares a 16-bit union (__flags) with has_devmem_pages and
has_dma_mapping, the C compiler implements this as a read-modify-write on
the entire 16-bit word.
Meanwhile, lockless readers like xe_svm_range_in_vram() in xe_svm.c read
this word concurrently:
xe_svm_range_in_vram() {
struct drm_gpusvm_pages_flags flags = {
/* Pairs with WRITE_ONCE in drm_gpusvm.c */
.__flags = READ_ONCE(range->pages.flags.__flags),
};
...
}
Does this unmarked RMW conflict with the lockless READ_ONCE(), potentially
allowing compiler load/store tearing? This regression might lead to KCSAN data
race splats or stale reads of has_devmem_pages.
To satisfy the C memory model, should writers use
WRITE_ONCE(pages[i].flags.__flags, new_flags.__flags) as is already done
in drm_gpusvm_get_pages()?
> if (drm_gpusvm_range_start(range) < mmu_range->start ||
> drm_gpusvm_range_end(range) > mmu_range->end)
> - range->pages.flags.partial_unmap = true;
> + range->flags.partial_unmap = true;
> }
> EXPORT_SYMBOL_GPL(drm_gpusvm_range_set_unmapped);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617103602.1491609-1-honglei1.huang@amd.com?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level
2026-06-17 10:35 [PATCH v2 0/5] drm/gpusvm: split MM and device state across gpusvm/range/pages Honglei Huang
` (3 preceding siblings ...)
2026-06-17 10:36 ` [PATCH v2 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range Honglei Huang
@ 2026-06-17 10:36 ` Honglei Huang
4 siblings, 0 replies; 11+ messages in thread
From: Honglei Huang @ 2026-06-17 10:36 UTC (permalink / raw)
To: sima, matthew.brost, rodrigo.vivi, thomas.hellstrom, dakr
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Oak.Zeng, Jenny-Jing.Liu, Philip.Yang, Xiaogang.Chen, Ray.Huang,
Lingshan.Zhu, Junhua.Shen, Yiru.Ma, amd-gfx, dri-devel, intel-xe,
honghuan
From: Honglei Huang <honghuan@amd.com>
The core mechanism of drm_gpusvm is HMM, which is fundamentally an
MM side subsystem. A drm_device, enters the picture on the device side at
DMA mapping / GPU bind.
So move struct drm_device from struct drm_gpusvm in drm_gpusvm. Let
drm_gpusvm keep its core neutral and leave device side decisions to
the driver. Make drm_gpusvm a pure MM level object.
- Move the drm_device from struct drm_gpusvm. drm_device now stored in
drm_gpusvm_pages.
- Drop the drm parameter from drm_gpusvm_init()
- Update the xe call sites in xe_svm_init() and other callers.
drm_device does not disappear from the framework, it is
relocated onto each drm_gpusvm_pages where DMA actually happens.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 8 ++++----
drivers/gpu/drm/xe/xe_svm.c | 4 ++--
drivers/gpu/drm/xe/xe_svm.h | 2 +-
include/drm/drm_gpusvm.h | 4 +---
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index bb186719b17..185e93b564e 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -431,7 +431,6 @@ static const struct mmu_interval_notifier_ops drm_gpusvm_notifier_ops = {
* drm_gpusvm_init() - Initialize the GPU SVM.
* @gpusvm: Pointer to the GPU SVM structure.
* @name: Name of the GPU SVM.
- * @drm: Pointer to the DRM device structure.
* @mm: Pointer to the mm_struct for the address space.
* @mm_start: Start address of GPU SVM.
* @mm_range: Range of the GPU SVM.
@@ -445,7 +444,9 @@ static const struct mmu_interval_notifier_ops drm_gpusvm_notifier_ops = {
* This function initializes the GPU SVM.
*
* Note: If only using the simple drm_gpusvm_pages API (get/unmap/free),
- * then only @gpusvm, @name, and @drm are expected. However, the same base
+ * then only @gpusvm and @name are expected. The @drm drm_device for dma
+ * mappings is bound per-pages via drm_gpusvm_init_pages() before the first
+ * drm_gpusvm_get_pages() call. However, the same base
* @gpusvm can also be used with both modes together in which case the full
* setup is needed, where the core drm_gpusvm_pages API will simply never use
* the other fields.
@@ -453,7 +454,7 @@ static const struct mmu_interval_notifier_ops drm_gpusvm_notifier_ops = {
* Return: 0 on success, a negative error code on failure.
*/
int drm_gpusvm_init(struct drm_gpusvm *gpusvm,
- const char *name, struct drm_device *drm,
+ const char *name,
struct mm_struct *mm,
unsigned long mm_start, unsigned long mm_range,
unsigned long notifier_size,
@@ -471,7 +472,6 @@ int drm_gpusvm_init(struct drm_gpusvm *gpusvm,
}
gpusvm->name = name;
- gpusvm->drm = drm;
gpusvm->mm = mm;
gpusvm->mm_start = mm_start;
gpusvm->mm_range = mm_range;
diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
index 77af0a8de63..7c4e40809e6 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -906,7 +906,7 @@ int xe_svm_init(struct xe_vm *vm)
return err;
}
- err = drm_gpusvm_init(&vm->svm.gpusvm, "Xe SVM", &vm->xe->drm,
+ err = drm_gpusvm_init(&vm->svm.gpusvm, "Xe SVM",
current->mm, 0, vm->size,
xe_modparam.svm_notifier_size * SZ_1M,
&gpusvm_ops, fault_chunk_sizes,
@@ -920,7 +920,7 @@ int xe_svm_init(struct xe_vm *vm)
}
} else {
err = drm_gpusvm_init(&vm->svm.gpusvm, "Xe SVM (simple)",
- &vm->xe->drm, NULL, 0, 0, 0, NULL,
+ NULL, 0, 0, 0, NULL,
NULL, 0);
}
diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
index 1423ab2f1d6..63ea991e2cb 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -235,7 +235,7 @@ static inline
int xe_svm_init(struct xe_vm *vm)
{
#if IS_ENABLED(CONFIG_DRM_GPUSVM)
- return drm_gpusvm_init(&vm->svm.gpusvm, "Xe SVM (simple)", &vm->xe->drm,
+ return drm_gpusvm_init(&vm->svm.gpusvm, "Xe SVM (simple)",
NULL, 0, 0, 0, NULL, NULL, 0);
#else
return 0;
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index 5edfa7d0c36..f40b52e3ae7 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -195,7 +195,6 @@ struct drm_gpusvm_range {
* struct drm_gpusvm - GPU SVM structure
*
* @name: Name of the GPU SVM
- * @drm: Pointer to the DRM device structure
* @mm: Pointer to the mm_struct for the address space
* @mm_start: Start address of GPU SVM
* @mm_range: Range of the GPU SVM
@@ -219,7 +218,6 @@ struct drm_gpusvm_range {
*/
struct drm_gpusvm {
const char *name;
- struct drm_device *drm;
struct mm_struct *mm;
unsigned long mm_start;
unsigned long mm_range;
@@ -271,7 +269,7 @@ struct drm_gpusvm_ctx {
};
int drm_gpusvm_init(struct drm_gpusvm *gpusvm,
- const char *name, struct drm_device *drm,
+ const char *name,
struct mm_struct *mm,
unsigned long mm_start, unsigned long mm_range,
unsigned long notifier_size,
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread