* [RFC 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
2026-06-03 6:56 [RFC 0/5] drm/gpusvm: split MM and device state across Honglei Huang
@ 2026-06-03 6:56 ` Honglei Huang
2026-06-10 3:55 ` Matthew Brost
2026-06-03 6:56 ` [RFC 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages Honglei Huang
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Honglei Huang @ 2026-06-03 6:56 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, 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 falgs 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 unmap flages in driver it self
to avoid handling the unmapped pages.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 11 +++--------
drivers/gpu/drm/xe/xe_svm.c | 11 +++++++----
include/drm/drm_gpusvm.h | 28 +++++++++++++++++++++-------
3 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 958cb605aed..6000d587cf2 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;
}
@@ -1470,11 +1470,6 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
drm_gpusvm_notifier_lock(gpusvm);
flags.__flags = svm_pages->flags.__flags;
- if (flags.unmapped) {
- drm_gpusvm_notifier_unlock(gpusvm);
- err = -EFAULT;
- goto err_free;
- }
if (mmu_interval_read_retry(notifier, hmm_range.notifier_seq)) {
drm_gpusvm_notifier_unlock(gpusvm);
@@ -1794,10 +1789,10 @@ void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
{
lockdep_assert_held_write(&range->gpusvm->notifier_lock);
- range->pages.flags.unmapped = true;
+ range->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..3acfddb7c5b 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -166,7 +166,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 +1136,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 +1248,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;
}
@@ -1507,6 +1507,9 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
{
int err = 0;
+ if (READ_ONCE(range->base.flags.unmapped))
+ return -EFAULT;
+
err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx);
if (err == -EOPNOTSUPP) {
range_debug(range, "PAGE FAULT - EVICT PAGES");
@@ -1623,7 +1626,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..3dba4b9516f 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -109,9 +109,6 @@ 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 +116,7 @@ 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 +144,25 @@ 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 {
+ u16 migrate_devmem : 1;
+ u16 unmapped : 1;
+ u16 partial_unmap : 1;
+ };
+ u16 __flags;
+ };
+};
+
/**
* struct drm_gpusvm_range - Structure representing a GPU SVM range
*
@@ -160,6 +172,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 +184,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;
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
2026-06-03 6:56 ` [RFC 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags Honglei Huang
@ 2026-06-10 3:55 ` Matthew Brost
2026-06-10 8:59 ` Huang, Honglei
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Brost @ 2026-06-10 3:55 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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, honghuan
On Wed, Jun 03, 2026 at 02:56:16PM +0800, Honglei Huang wrote:
> 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 falgs 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 unmap flages in driver it self
> to avoid handling the unmapped pages.
>
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 11 +++--------
> drivers/gpu/drm/xe/xe_svm.c | 11 +++++++----
> include/drm/drm_gpusvm.h | 28 +++++++++++++++++++++-------
> 3 files changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 958cb605aed..6000d587cf2 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;
> }
> @@ -1470,11 +1470,6 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> drm_gpusvm_notifier_lock(gpusvm);
>
> flags.__flags = svm_pages->flags.__flags;
> - if (flags.unmapped) {
> - drm_gpusvm_notifier_unlock(gpusvm);
> - err = -EFAULT;
> - goto err_free;
> - }
>
> if (mmu_interval_read_retry(notifier, hmm_range.notifier_seq)) {
> drm_gpusvm_notifier_unlock(gpusvm);
> @@ -1794,10 +1789,10 @@ void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
> {
> lockdep_assert_held_write(&range->gpusvm->notifier_lock);
>
> - range->pages.flags.unmapped = true;
> + range->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..3acfddb7c5b 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -166,7 +166,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 +1136,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 +1248,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;
> }
> @@ -1507,6 +1507,9 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
> {
> int err = 0;
>
> + if (READ_ONCE(range->base.flags.unmapped))
> + return -EFAULT;
> +
This is the compile error I encountered when I pulled the code—READ_ONCE
isn’t valid for bitfields.
Beyond that, there is a reason why unmapped was checked in get_pages()
under the notifier lock: it’s the only way to ensure that the pages
being mapped are valid at the time of mapping, or to determine whether
get_pages() should abort. The HMM locking documentation [1] (sort of)
describes this in some detail.
Since this didn’t compile and exposed the bug, I put together a quick
fix here [2]. The basic idea is to mirror unmapped in the page flags and
update drm_gpusvm_range_set_unmapped() to accept an array of pages,
updating the unmapped flags in those pages as well. This also allows us
to retain the unmapped check in get_pages() under the notifier lock.
I’m not sure how you plan to store the pages in the AMD driver, but if
it’s an array, this approach should work for you. Let me know what you
think.
Matt
[1] https://elixir.bootlin.com/linux/v7.0.11/source/Documentation/mm/hmm.rst#L193
[2] https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/623f6a50c037d9e44f6c9fbe6859a0ba7ad50177
> err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx);
> if (err == -EOPNOTSUPP) {
> range_debug(range, "PAGE FAULT - EVICT PAGES");
> @@ -1623,7 +1626,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..3dba4b9516f 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -109,9 +109,6 @@ 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 +116,7 @@ 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 +144,25 @@ 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 {
> + u16 migrate_devmem : 1;
> + u16 unmapped : 1;
> + u16 partial_unmap : 1;
> + };
> + u16 __flags;
> + };
> +};
> +
> /**
> * struct drm_gpusvm_range - Structure representing a GPU SVM range
> *
> @@ -160,6 +172,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 +184,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;
> };
>
> /**
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
2026-06-10 3:55 ` Matthew Brost
@ 2026-06-10 8:59 ` Huang, Honglei
0 siblings, 0 replies; 18+ messages in thread
From: Huang, Honglei @ 2026-06-10 8:59 UTC (permalink / raw)
To: Matthew Brost
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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,
Honglei Huang
On 6/10/2026 11:55 AM, Matthew Brost wrote:
> On Wed, Jun 03, 2026 at 02:56:16PM +0800, Honglei Huang wrote:
>> 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 falgs 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 unmap flages in driver it self
>> to avoid handling the unmapped pages.
>>
>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>> drivers/gpu/drm/drm_gpusvm.c | 11 +++--------
>> drivers/gpu/drm/xe/xe_svm.c | 11 +++++++----
>> include/drm/drm_gpusvm.h | 28 +++++++++++++++++++++-------
>> 3 files changed, 31 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
>> index 958cb605aed..6000d587cf2 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;
>> }
>> @@ -1470,11 +1470,6 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>> drm_gpusvm_notifier_lock(gpusvm);
>>
>> flags.__flags = svm_pages->flags.__flags;
>> - if (flags.unmapped) {
>> - drm_gpusvm_notifier_unlock(gpusvm);
>> - err = -EFAULT;
>> - goto err_free;
>> - }
>>
>> if (mmu_interval_read_retry(notifier, hmm_range.notifier_seq)) {
>> drm_gpusvm_notifier_unlock(gpusvm);
>> @@ -1794,10 +1789,10 @@ void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
>> {
>> lockdep_assert_held_write(&range->gpusvm->notifier_lock);
>>
>> - range->pages.flags.unmapped = true;
>> + range->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..3acfddb7c5b 100644
>> --- a/drivers/gpu/drm/xe/xe_svm.c
>> +++ b/drivers/gpu/drm/xe/xe_svm.c
>> @@ -166,7 +166,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 +1136,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 +1248,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;
>> }
>> @@ -1507,6 +1507,9 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
>> {
>> int err = 0;
>>
>> + if (READ_ONCE(range->base.flags.unmapped))
>> + return -EFAULT;
>> +
>
> This is the compile error I encountered when I pulled the code—READ_ONCE
> isn’t valid for bitfields.
>
> Beyond that, there is a reason why unmapped was checked in get_pages()
> under the notifier lock: it’s the only way to ensure that the pages
> being mapped are valid at the time of mapping, or to determine whether
> get_pages() should abort. The HMM locking documentation [1] (sort of)
> describes this in some detail.
>
> Since this didn’t compile and exposed the bug, I put together a quick
> fix here [2]. The basic idea is to mirror unmapped in the page flags and
> update drm_gpusvm_range_set_unmapped() to accept an array of pages,
> updating the unmapped flags in those pages as well. This also allows us
> to retain the unmapped check in get_pages() under the notifier lock.
>
> I’m not sure how you plan to store the pages in the AMD driver, but if
> it’s an array, this approach should work for you. Let me know what you
> think.
Oh that is really a bug in my patch, I added the REEAD_ONCE in final
check without compiling check, really sorry about it. Really thanks for
pointing out it.
And will fix this patch according your method in [2]. It should also
works for AMDGPU.
Regards,
Honglei
>
> Matt
>
> [1] https://elixir.bootlin.com/linux/v7.0.11/source/Documentation/mm/hmm.rst#L193
> [2] https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/623f6a50c037d9e44f6c9fbe6859a0ba7ad50177
>
>> err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx);
>> if (err == -EOPNOTSUPP) {
>> range_debug(range, "PAGE FAULT - EVICT PAGES");
>> @@ -1623,7 +1626,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..3dba4b9516f 100644
>> --- a/include/drm/drm_gpusvm.h
>> +++ b/include/drm/drm_gpusvm.h
>> @@ -109,9 +109,6 @@ 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 +116,7 @@ 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 +144,25 @@ 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 {
>> + u16 migrate_devmem : 1;
>> + u16 unmapped : 1;
>> + u16 partial_unmap : 1;
>> + };
>> + u16 __flags;
>> + };
>> +};
>> +
>> /**
>> * struct drm_gpusvm_range - Structure representing a GPU SVM range
>> *
>> @@ -160,6 +172,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 +184,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;
>> };
>>
>> /**
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages
2026-06-03 6:56 [RFC 0/5] drm/gpusvm: split MM and device state across Honglei Huang
2026-06-03 6:56 ` [RFC 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags Honglei Huang
@ 2026-06-03 6:56 ` Honglei Huang
2026-06-10 4:07 ` Matthew Brost
2026-06-03 6:56 ` [RFC 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages Honglei Huang
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Honglei Huang @ 2026-06-03 6:56 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, 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 a matching drm parameter
to drm_gpusvm_get_pages(); the dma device is bound on first use
and immutable for the lifetime of the pages instance.
- Route all DMA in drm_gpusvm_get_pages() / __drm_gpusvm_unmap_pages()
through svm_pages->drm instead of gpusvm->drm.
- Update existing callers (drm_gpusvm_range_get_pages, xe userptr)
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 37 ++++++++++++++++++++++++---------
drivers/gpu/drm/xe/xe_userptr.c | 1 +
include/drm/drm_gpusvm.h | 3 +++
3 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 6000d587cf2..3f076178b2a 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1135,11 +1135,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 (WARN_ON_ONCE(!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,
@@ -1379,6 +1384,7 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
* drm_gpusvm_get_pages() - Get pages and populate GPU SVM pages struct
* @gpusvm: Pointer to the GPU SVM structure
* @svm_pages: The SVM pages to populate. This will contain the dma-addresses
+ * @drm: The DRM device that will own the DMA mappings. Stored into @svm_pages
* @mm: The mm corresponding to the CPU range
* @notifier: The corresponding notifier for the given CPU range
* @pages_start: Start CPU address for the pages
@@ -1392,6 +1398,7 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
*/
int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_pages *svm_pages,
+ struct drm_device *drm,
struct mm_struct *mm,
struct mmu_interval_notifier *notifier,
unsigned long pages_start, unsigned long pages_end,
@@ -1421,6 +1428,15 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
DMA_BIDIRECTIONAL;
struct dma_iova_state *state = &svm_pages->state;
+ if (!drm)
+ return -EINVAL;
+ if (svm_pages->drm) {
+ if (svm_pages->drm != drm)
+ return -EINVAL;
+ } else {
+ svm_pages->drm = drm;
+ }
+
retry:
if (time_after(jiffies, timeout))
return -EBUSY;
@@ -1515,7 +1531,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(drm, !dpagemap)) {
/*
* Raced. This is not supposed to happen
* since hmm_range_fault() should've migrated
@@ -1527,10 +1543,10 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
}
svm_pages->dma_addr[j] =
dpagemap->ops->device_map(dpagemap,
- gpusvm->drm->dev,
+ drm->dev,
page, order,
dma_dir);
- if (dma_mapping_error(gpusvm->drm->dev,
+ if (dma_mapping_error(drm->dev,
svm_pages->dma_addr[j].addr)) {
err = -EFAULT;
goto err_unmap;
@@ -1550,11 +1566,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
}
if (!i)
- dma_iova_try_alloc(gpusvm->drm->dev, state,
+ dma_iova_try_alloc(drm->dev, state,
0, npages * PAGE_SIZE);
if (dma_use_iova(state)) {
- err = dma_iova_link(gpusvm->drm->dev, state,
+ err = dma_iova_link(drm->dev, state,
hmm_pfn_to_phys(pfns[i]),
svm_pages->state_offset,
PAGE_SIZE << order,
@@ -1565,11 +1581,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(drm->dev,
page, 0,
PAGE_SIZE << order,
dma_dir);
- if (dma_mapping_error(gpusvm->drm->dev, addr)) {
+ if (dma_mapping_error(drm->dev, addr)) {
err = -EFAULT;
goto err_unmap;
}
@@ -1585,7 +1601,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(drm->dev, state, 0,
svm_pages->state_offset);
if (err)
goto err_unmap;
@@ -1635,7 +1651,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->drm,
+ 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..7e28f6868ff 100644
--- a/drivers/gpu/drm/xe/xe_userptr.c
+++ b/drivers/gpu/drm/xe/xe_userptr.c
@@ -75,6 +75,7 @@ int xe_vma_userptr_pin_pages(struct xe_userptr_vma *uvma)
return 0;
return drm_gpusvm_get_pages(&vm->svm.gpusvm, &uvma->userptr.pages,
+ &xe->drm,
uvma->userptr.notifier.mm,
&uvma->userptr.notifier,
xe_vma_userptr(vma),
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index 3dba4b9516f..ed228d9ff6b 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -127,6 +127,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.
@@ -136,6 +137,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;
@@ -328,6 +330,7 @@ void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_pages *svm_pages,
+ struct drm_device *drm,
struct mm_struct *mm,
struct mmu_interval_notifier *notifier,
unsigned long pages_start, unsigned long pages_end,
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages
2026-06-03 6:56 ` [RFC 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages Honglei Huang
@ 2026-06-10 4:07 ` Matthew Brost
2026-06-10 9:01 ` Huang, Honglei
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Brost @ 2026-06-10 4:07 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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, honghuan
On Wed, Jun 03, 2026 at 02:56:17PM +0800, Honglei Huang wrote:
> 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 a matching drm parameter
> to drm_gpusvm_get_pages(); the dma device is bound on first use
> and immutable for the lifetime of the pages instance.
> - Route all DMA in drm_gpusvm_get_pages() / __drm_gpusvm_unmap_pages()
> through svm_pages->drm instead of gpusvm->drm.
> - Update existing callers (drm_gpusvm_range_get_pages, xe userptr)
>
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 37 ++++++++++++++++++++++++---------
> drivers/gpu/drm/xe/xe_userptr.c | 1 +
> include/drm/drm_gpusvm.h | 3 +++
> 3 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 6000d587cf2..3f076178b2a 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1135,11 +1135,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 (WARN_ON_ONCE(!svm_pages->drm))
I think it is valid to reach this point without calling get_pages() and
assigning ->drm, so I don’t believe a WARN_ON is required. One example
would be creating a range, attempting to migrate it, and then failing
because the user performs a munmap() on part of the range, resulting in
the range being freed. It’s a weird race, but it’s possible, and I’m
fairly certain Xe SVM tests exercise scenarios like this.
So I would drop the WARN_ON, add a comment like “get_pages() never
called,” and bail out silently. Alternatively, if drm is NULL and
has_dma_mapping is set, then a WARN_ON might make sense, as that should
not be possible.
> + return;
> +
> + dev = svm_pages->drm->dev;
> +
> if (svm_pages->flags.has_dma_mapping) {
> struct drm_gpusvm_pages_flags flags = {
> .__flags = svm_pages->flags.__flags,
> @@ -1379,6 +1384,7 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
> * drm_gpusvm_get_pages() - Get pages and populate GPU SVM pages struct
> * @gpusvm: Pointer to the GPU SVM structure
> * @svm_pages: The SVM pages to populate. This will contain the dma-addresses
> + * @drm: The DRM device that will own the DMA mappings. Stored into @svm_pages
> * @mm: The mm corresponding to the CPU range
> * @notifier: The corresponding notifier for the given CPU range
> * @pages_start: Start CPU address for the pages
> @@ -1392,6 +1398,7 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
> */
> int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> struct drm_gpusvm_pages *svm_pages,
> + struct drm_device *drm,
You could also move drm into a function like drm_gpusvm_init_pages() (as
mentioned in the cover letter). I don’t have a strong preference, but if
we want a helper that calls hmm_range_fault() once and accepts an array
of drm_gpusvm_pages to DMA-map, that might make sense.
> struct mm_struct *mm,
> struct mmu_interval_notifier *notifier,
> unsigned long pages_start, unsigned long pages_end,
> @@ -1421,6 +1428,15 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> DMA_BIDIRECTIONAL;
> struct dma_iova_state *state = &svm_pages->state;
>
> + if (!drm)
> + return -EINVAL;
> + if (svm_pages->drm) {
> + if (svm_pages->drm != drm)
> + return -EINVAL;
> + } else {
> + svm_pages->drm = drm;
> + }
Style nit: If we keep this I'd write this like:
if (!drm || (svm_pages->drm && svm_pages->drm != drm))
return -EINVAL;
svm_pages->drm = drm;
Matt
> +
> retry:
> if (time_after(jiffies, timeout))
> return -EBUSY;
> @@ -1515,7 +1531,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(drm, !dpagemap)) {
> /*
> * Raced. This is not supposed to happen
> * since hmm_range_fault() should've migrated
> @@ -1527,10 +1543,10 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> }
> svm_pages->dma_addr[j] =
> dpagemap->ops->device_map(dpagemap,
> - gpusvm->drm->dev,
> + drm->dev,
> page, order,
> dma_dir);
> - if (dma_mapping_error(gpusvm->drm->dev,
> + if (dma_mapping_error(drm->dev,
> svm_pages->dma_addr[j].addr)) {
> err = -EFAULT;
> goto err_unmap;
> @@ -1550,11 +1566,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> }
>
> if (!i)
> - dma_iova_try_alloc(gpusvm->drm->dev, state,
> + dma_iova_try_alloc(drm->dev, state,
> 0, npages * PAGE_SIZE);
>
> if (dma_use_iova(state)) {
> - err = dma_iova_link(gpusvm->drm->dev, state,
> + err = dma_iova_link(drm->dev, state,
> hmm_pfn_to_phys(pfns[i]),
> svm_pages->state_offset,
> PAGE_SIZE << order,
> @@ -1565,11 +1581,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(drm->dev,
> page, 0,
> PAGE_SIZE << order,
> dma_dir);
> - if (dma_mapping_error(gpusvm->drm->dev, addr)) {
> + if (dma_mapping_error(drm->dev, addr)) {
> err = -EFAULT;
> goto err_unmap;
> }
> @@ -1585,7 +1601,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(drm->dev, state, 0,
> svm_pages->state_offset);
> if (err)
> goto err_unmap;
> @@ -1635,7 +1651,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->drm,
> + 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..7e28f6868ff 100644
> --- a/drivers/gpu/drm/xe/xe_userptr.c
> +++ b/drivers/gpu/drm/xe/xe_userptr.c
> @@ -75,6 +75,7 @@ int xe_vma_userptr_pin_pages(struct xe_userptr_vma *uvma)
> return 0;
>
> return drm_gpusvm_get_pages(&vm->svm.gpusvm, &uvma->userptr.pages,
> + &xe->drm,
> uvma->userptr.notifier.mm,
> &uvma->userptr.notifier,
> xe_vma_userptr(vma),
> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> index 3dba4b9516f..ed228d9ff6b 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -127,6 +127,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.
> @@ -136,6 +137,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;
> @@ -328,6 +330,7 @@ void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
>
> int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> struct drm_gpusvm_pages *svm_pages,
> + struct drm_device *drm,
> struct mm_struct *mm,
> struct mmu_interval_notifier *notifier,
> unsigned long pages_start, unsigned long pages_end,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages
2026-06-10 4:07 ` Matthew Brost
@ 2026-06-10 9:01 ` Huang, Honglei
0 siblings, 0 replies; 18+ messages in thread
From: Huang, Honglei @ 2026-06-10 9:01 UTC (permalink / raw)
To: Matthew Brost
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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,
Honglei Huang
On 6/10/2026 12:07 PM, Matthew Brost wrote:
> On Wed, Jun 03, 2026 at 02:56:17PM +0800, Honglei Huang wrote:
>> 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 a matching drm parameter
>> to drm_gpusvm_get_pages(); the dma device is bound on first use
>> and immutable for the lifetime of the pages instance.
>> - Route all DMA in drm_gpusvm_get_pages() / __drm_gpusvm_unmap_pages()
>> through svm_pages->drm instead of gpusvm->drm.
>> - Update existing callers (drm_gpusvm_range_get_pages, xe userptr)
>>
>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>> drivers/gpu/drm/drm_gpusvm.c | 37 ++++++++++++++++++++++++---------
>> drivers/gpu/drm/xe/xe_userptr.c | 1 +
>> include/drm/drm_gpusvm.h | 3 +++
>> 3 files changed, 31 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
>> index 6000d587cf2..3f076178b2a 100644
>> --- a/drivers/gpu/drm/drm_gpusvm.c
>> +++ b/drivers/gpu/drm/drm_gpusvm.c
>> @@ -1135,11 +1135,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 (WARN_ON_ONCE(!svm_pages->drm))
>
> I think it is valid to reach this point without calling get_pages() and
> assigning ->drm, so I don’t believe a WARN_ON is required. One example
> would be creating a range, attempting to migrate it, and then failing
> because the user performs a munmap() on part of the range, resulting in
> the range being freed. It’s a weird race, but it’s possible, and I’m
> fairly certain Xe SVM tests exercise scenarios like this.
>
> So I would drop the WARN_ON, add a comment like “get_pages() never
> called,” and bail out silently. Alternatively, if drm is NULL and
> has_dma_mapping is set, then a WARN_ON might make sense, as that should
> not be possible.
Got it, will drop the WARN_ON.
>
>> + return;
>> +
>> + dev = svm_pages->drm->dev;
>> +
>> if (svm_pages->flags.has_dma_mapping) {
>> struct drm_gpusvm_pages_flags flags = {
>> .__flags = svm_pages->flags.__flags,
>> @@ -1379,6 +1384,7 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
>> * drm_gpusvm_get_pages() - Get pages and populate GPU SVM pages struct
>> * @gpusvm: Pointer to the GPU SVM structure
>> * @svm_pages: The SVM pages to populate. This will contain the dma-addresses
>> + * @drm: The DRM device that will own the DMA mappings. Stored into @svm_pages
>> * @mm: The mm corresponding to the CPU range
>> * @notifier: The corresponding notifier for the given CPU range
>> * @pages_start: Start CPU address for the pages
>> @@ -1392,6 +1398,7 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
>> */
>> int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>> struct drm_gpusvm_pages *svm_pages,
>> + struct drm_device *drm,
>
> You could also move drm into a function like drm_gpusvm_init_pages() (as
> mentioned in the cover letter). I don’t have a strong preference, but if
> we want a helper that calls hmm_range_fault() once and accepts an array
> of drm_gpusvm_pages to DMA-map, that might make sense.
Got it, will init the drm device in drm_gpusvm_init_pages.
>
>> struct mm_struct *mm,
>> struct mmu_interval_notifier *notifier,
>> unsigned long pages_start, unsigned long pages_end,
>> @@ -1421,6 +1428,15 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>> DMA_BIDIRECTIONAL;
>> struct dma_iova_state *state = &svm_pages->state;
>>
>> + if (!drm)
>> + return -EINVAL;
>> + if (svm_pages->drm) {
>> + if (svm_pages->drm != drm)
>> + return -EINVAL;
>> + } else {
>> + svm_pages->drm = drm;
>> + }
>
> Style nit: If we keep this I'd write this like:
>
> if (!drm || (svm_pages->drm && svm_pages->drm != drm))
> return -EINVAL;
>
> svm_pages->drm = drm;
Got it will modify in next version.
Regards,
Honglei
>
> Matt
>
>> +
>> retry:
>> if (time_after(jiffies, timeout))
>> return -EBUSY;
>> @@ -1515,7 +1531,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(drm, !dpagemap)) {
>> /*
>> * Raced. This is not supposed to happen
>> * since hmm_range_fault() should've migrated
>> @@ -1527,10 +1543,10 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>> }
>> svm_pages->dma_addr[j] =
>> dpagemap->ops->device_map(dpagemap,
>> - gpusvm->drm->dev,
>> + drm->dev,
>> page, order,
>> dma_dir);
>> - if (dma_mapping_error(gpusvm->drm->dev,
>> + if (dma_mapping_error(drm->dev,
>> svm_pages->dma_addr[j].addr)) {
>> err = -EFAULT;
>> goto err_unmap;
>> @@ -1550,11 +1566,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>> }
>>
>> if (!i)
>> - dma_iova_try_alloc(gpusvm->drm->dev, state,
>> + dma_iova_try_alloc(drm->dev, state,
>> 0, npages * PAGE_SIZE);
>>
>> if (dma_use_iova(state)) {
>> - err = dma_iova_link(gpusvm->drm->dev, state,
>> + err = dma_iova_link(drm->dev, state,
>> hmm_pfn_to_phys(pfns[i]),
>> svm_pages->state_offset,
>> PAGE_SIZE << order,
>> @@ -1565,11 +1581,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(drm->dev,
>> page, 0,
>> PAGE_SIZE << order,
>> dma_dir);
>> - if (dma_mapping_error(gpusvm->drm->dev, addr)) {
>> + if (dma_mapping_error(drm->dev, addr)) {
>> err = -EFAULT;
>> goto err_unmap;
>> }
>> @@ -1585,7 +1601,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(drm->dev, state, 0,
>> svm_pages->state_offset);
>> if (err)
>> goto err_unmap;
>> @@ -1635,7 +1651,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->drm,
>> + 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..7e28f6868ff 100644
>> --- a/drivers/gpu/drm/xe/xe_userptr.c
>> +++ b/drivers/gpu/drm/xe/xe_userptr.c
>> @@ -75,6 +75,7 @@ int xe_vma_userptr_pin_pages(struct xe_userptr_vma *uvma)
>> return 0;
>>
>> return drm_gpusvm_get_pages(&vm->svm.gpusvm, &uvma->userptr.pages,
>> + &xe->drm,
>> uvma->userptr.notifier.mm,
>> &uvma->userptr.notifier,
>> xe_vma_userptr(vma),
>> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
>> index 3dba4b9516f..ed228d9ff6b 100644
>> --- a/include/drm/drm_gpusvm.h
>> +++ b/include/drm/drm_gpusvm.h
>> @@ -127,6 +127,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.
>> @@ -136,6 +137,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;
>> @@ -328,6 +330,7 @@ void drm_gpusvm_range_set_unmapped(struct drm_gpusvm_range *range,
>>
>> int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>> struct drm_gpusvm_pages *svm_pages,
>> + struct drm_device *drm,
>> struct mm_struct *mm,
>> struct mmu_interval_notifier *notifier,
>> unsigned long pages_start, unsigned long pages_end,
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages
2026-06-03 6:56 [RFC 0/5] drm/gpusvm: split MM and device state across Honglei Huang
2026-06-03 6:56 ` [RFC 1/5] drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags Honglei Huang
2026-06-03 6:56 ` [RFC 2/5] drm/gpusvm: embed struct drm_device into drm_gpusvm_pages Honglei Huang
@ 2026-06-03 6:56 ` Honglei Huang
2026-06-10 4:14 ` Matthew Brost
2026-06-03 6:56 ` [RFC 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range Honglei Huang
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Honglei Huang @ 2026-06-03 6:56 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, 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.
- Take over the page lifecycle: xe_svm_range_get_pages() calls
drm_gpusvm_get_pages() directly with &xe->drm; the notifier
event_end and xe_svm_range_free() paths drive unmap/free on
the embedded pages object.
- Switch xe_svm_range_pages_valid() to drm_gpusvm_pages_valid().
Framework side (drm_gpusvm):
- Export drm_gpusvm_pages_valid() to let driver owned pages
can query mapping state without going through a range.
- Contract change: drm_gpusvm_range_remove() no longer unmaps or
frees pages; drivers that own a drm_gpusvm_pages instance must
do that themselves.
Side effect / contract: drivers that own a drm_gpusvm_pages
are now responsible for its lifecycle, in particular for calling
drm_gpusvm_unmap_pages() and drm_gpusvm_free_pages() at the
appropriate points.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 9 +++------
drivers/gpu/drm/xe/xe_pt.c | 2 +-
drivers/gpu/drm/xe/xe_svm.c | 22 +++++++++++++++-------
drivers/gpu/drm/xe/xe_svm.h | 9 +++++++--
include/drm/drm_gpusvm.h | 3 +++
5 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 3f076178b2a..a4b56cefeb2 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1231,8 +1231,6 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_free_pages);
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);
@@ -1244,8 +1242,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);
@@ -1324,13 +1320,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 3acfddb7c5b..33c26df5111 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);
+ range->pages.notifier_seq = LONG_MAX;
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
@@ -208,7 +211,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);
@@ -952,7 +956,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,
@@ -1017,7 +1021,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);
@@ -1510,7 +1514,11 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
if (READ_ONCE(range->base.flags.unmapped))
return -EFAULT;
- err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx);
+ err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages,
+ &vm->xe->drm, 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..ea73241d3d9 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -31,6 +31,11 @@ struct xe_vram_region;
struct xe_svm_range {
/** @base: base drm_gpusvm_range */
struct drm_gpusvm_range base;
+ /**
+ * @pages: Per-device DMA mapping state; single instance since
+ * xe svm is 1 svm : 1 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 +79,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 +137,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/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index ed228d9ff6b..21baf91ec7e 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -306,6 +306,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);
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages
2026-06-03 6:56 ` [RFC 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages Honglei Huang
@ 2026-06-10 4:14 ` Matthew Brost
2026-06-10 9:03 ` Huang, Honglei
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Brost @ 2026-06-10 4:14 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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, honghuan
On Wed, Jun 03, 2026 at 02:56:18PM +0800, Honglei Huang wrote:
> 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.
> - Take over the page lifecycle: xe_svm_range_get_pages() calls
> drm_gpusvm_get_pages() directly with &xe->drm; the notifier
> event_end and xe_svm_range_free() paths drive unmap/free on
> the embedded pages object.
> - Switch xe_svm_range_pages_valid() to drm_gpusvm_pages_valid().
>
> Framework side (drm_gpusvm):
>
> - Export drm_gpusvm_pages_valid() to let driver owned pages
> can query mapping state without going through a range.
> - Contract change: drm_gpusvm_range_remove() no longer unmaps or
> frees pages; drivers that own a drm_gpusvm_pages instance must
> do that themselves.
>
> Side effect / contract: drivers that own a drm_gpusvm_pages
> are now responsible for its lifecycle, in particular for calling
> drm_gpusvm_unmap_pages() and drm_gpusvm_free_pages() at the
> appropriate points.
>
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 9 +++------
> drivers/gpu/drm/xe/xe_pt.c | 2 +-
> drivers/gpu/drm/xe/xe_svm.c | 22 +++++++++++++++-------
> drivers/gpu/drm/xe/xe_svm.h | 9 +++++++--
> include/drm/drm_gpusvm.h | 3 +++
> 5 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 3f076178b2a..a4b56cefeb2 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1231,8 +1231,6 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_free_pages);
> 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);
> @@ -1244,8 +1242,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);
>
> @@ -1324,13 +1320,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 3acfddb7c5b..33c26df5111 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);
> + range->pages.notifier_seq = LONG_MAX;
As discussed in the cover-letter let's do a drm_gpusvm_init_pages()
function to set the notifier_seq.
If we want to include 'drm' in the init function as discussed in patch
#2, to fish this out in Xe you can do '&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
> @@ -208,7 +211,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);
> @@ -952,7 +956,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,
> @@ -1017,7 +1021,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);
>
> @@ -1510,7 +1514,11 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
> if (READ_ONCE(range->base.flags.unmapped))
> return -EFAULT;
>
> - err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx);
> + err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages,
> + &vm->xe->drm, 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..ea73241d3d9 100644
> --- a/drivers/gpu/drm/xe/xe_svm.h
> +++ b/drivers/gpu/drm/xe/xe_svm.h
> @@ -31,6 +31,11 @@ struct xe_vram_region;
> struct xe_svm_range {
> /** @base: base drm_gpusvm_range */
> struct drm_gpusvm_range base;
> + /**
> + * @pages: Per-device DMA mapping state; single instance since
> + * xe svm is 1 svm : 1 drm_device.
s/xe/Xe
Matt
> + */
> + 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 +79,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 +137,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/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> index ed228d9ff6b..21baf91ec7e 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -306,6 +306,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);
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages
2026-06-10 4:14 ` Matthew Brost
@ 2026-06-10 9:03 ` Huang, Honglei
0 siblings, 0 replies; 18+ messages in thread
From: Huang, Honglei @ 2026-06-10 9:03 UTC (permalink / raw)
To: Matthew Brost, Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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
On 6/10/2026 12:14 PM, Matthew Brost wrote:
> On Wed, Jun 03, 2026 at 02:56:18PM +0800, Honglei Huang wrote:
>> 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.
>> - Take over the page lifecycle: xe_svm_range_get_pages() calls
>> drm_gpusvm_get_pages() directly with &xe->drm; the notifier
>> event_end and xe_svm_range_free() paths drive unmap/free on
>> the embedded pages object.
>> - Switch xe_svm_range_pages_valid() to drm_gpusvm_pages_valid().
>>
>> Framework side (drm_gpusvm):
>>
>> - Export drm_gpusvm_pages_valid() to let driver owned pages
>> can query mapping state without going through a range.
>> - Contract change: drm_gpusvm_range_remove() no longer unmaps or
>> frees pages; drivers that own a drm_gpusvm_pages instance must
>> do that themselves.
>>
>> Side effect / contract: drivers that own a drm_gpusvm_pages
>> are now responsible for its lifecycle, in particular for calling
>> drm_gpusvm_unmap_pages() and drm_gpusvm_free_pages() at the
>> appropriate points.
>>
>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>> drivers/gpu/drm/drm_gpusvm.c | 9 +++------
>> drivers/gpu/drm/xe/xe_pt.c | 2 +-
>> drivers/gpu/drm/xe/xe_svm.c | 22 +++++++++++++++-------
>> drivers/gpu/drm/xe/xe_svm.h | 9 +++++++--
>> include/drm/drm_gpusvm.h | 3 +++
>> 5 files changed, 29 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
>> index 3f076178b2a..a4b56cefeb2 100644
>> --- a/drivers/gpu/drm/drm_gpusvm.c
>> +++ b/drivers/gpu/drm/drm_gpusvm.c
>> @@ -1231,8 +1231,6 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_free_pages);
>> 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);
>> @@ -1244,8 +1242,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);
>>
>> @@ -1324,13 +1320,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 3acfddb7c5b..33c26df5111 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);
>> + range->pages.notifier_seq = LONG_MAX;
>
> As discussed in the cover-letter let's do a drm_gpusvm_init_pages()
> function to set the notifier_seq.
>
> If we want to include 'drm' in the init function as discussed in patch
> #2, to fish this out in Xe you can do '&gpusvm_to_vm(gpusvm)->xe->drm'.
Got it, will init the seq in drm_gpusvm_init_pages, and will aplly the
init in Xe according to your suggestion.
>
>> 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
>> @@ -208,7 +211,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);
>> @@ -952,7 +956,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,
>> @@ -1017,7 +1021,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);
>>
>> @@ -1510,7 +1514,11 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
>> if (READ_ONCE(range->base.flags.unmapped))
>> return -EFAULT;
>>
>> - err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx);
>> + err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages,
>> + &vm->xe->drm, 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..ea73241d3d9 100644
>> --- a/drivers/gpu/drm/xe/xe_svm.h
>> +++ b/drivers/gpu/drm/xe/xe_svm.h
>> @@ -31,6 +31,11 @@ struct xe_vram_region;
>> struct xe_svm_range {
>> /** @base: base drm_gpusvm_range */
>> struct drm_gpusvm_range base;
>> + /**
>> + * @pages: Per-device DMA mapping state; single instance since
>> + * xe svm is 1 svm : 1 drm_device.
>
> s/xe/Xe
Got it, will fix.
Regards,
Honglei
>
> Matt
>
>> + */
>> + 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 +79,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 +137,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/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
>> index ed228d9ff6b..21baf91ec7e 100644
>> --- a/include/drm/drm_gpusvm.h
>> +++ b/include/drm/drm_gpusvm.h
>> @@ -306,6 +306,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);
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range
2026-06-03 6:56 [RFC 0/5] drm/gpusvm: split MM and device state across Honglei Huang
` (2 preceding siblings ...)
2026-06-03 6:56 ` [RFC 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages Honglei Huang
@ 2026-06-03 6:56 ` Honglei Huang
2026-06-10 4:17 ` Matthew Brost
2026-06-03 6:56 ` [RFC 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level Honglei Huang
2026-06-10 3:44 ` [RFC 0/5] drm/gpusvm: split MM and device state across Matthew Brost
5 siblings, 1 reply; 18+ messages in thread
From: Honglei Huang @ 2026-06-03 6:56 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, 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.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 68 ------------------------------------
include/drm/drm_gpusvm.h | 13 -------
2 files changed, 81 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index a4b56cefeb2..55515390c53 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -640,7 +640,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->flags.migrate_devmem = migrate_devmem ? 1 : 0;
return range;
@@ -1329,27 +1328,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
@@ -1633,29 +1611,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->drm,
- 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
@@ -1686,29 +1641,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 21baf91ec7e..250c59f0930 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -173,7 +173,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
@@ -185,7 +184,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;
};
@@ -303,20 +301,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] 18+ messages in thread* Re: [RFC 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range
2026-06-03 6:56 ` [RFC 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range Honglei Huang
@ 2026-06-10 4:17 ` Matthew Brost
2026-06-10 9:04 ` Huang, Honglei
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Brost @ 2026-06-10 4:17 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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, honghuan
On Wed, Jun 03, 2026 at 02:56:19PM +0800, Honglei Huang wrote:
> 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.
>
The patch looks good, but I think the kernel documentation at the top of
drm_gpusvm.c should be updated—particularly the examples. It may also be
worth updating the section explaining how pages are embedded in
driver-side ranges, including the options for one-to-one or many-to-one
mappings and the implications of each choice.
Matt
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 68 ------------------------------------
> include/drm/drm_gpusvm.h | 13 -------
> 2 files changed, 81 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index a4b56cefeb2..55515390c53 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -640,7 +640,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->flags.migrate_devmem = migrate_devmem ? 1 : 0;
>
> return range;
> @@ -1329,27 +1328,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
> @@ -1633,29 +1611,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->drm,
> - 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
> @@ -1686,29 +1641,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 21baf91ec7e..250c59f0930 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -173,7 +173,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
> @@ -185,7 +184,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;
> };
>
> @@ -303,20 +301,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 [flat|nested] 18+ messages in thread* Re: [RFC 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range
2026-06-10 4:17 ` Matthew Brost
@ 2026-06-10 9:04 ` Huang, Honglei
0 siblings, 0 replies; 18+ messages in thread
From: Huang, Honglei @ 2026-06-10 9:04 UTC (permalink / raw)
To: Matthew Brost
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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,
Honglei Huang
On 6/10/2026 12:17 PM, Matthew Brost wrote:
> On Wed, Jun 03, 2026 at 02:56:19PM +0800, Honglei Huang wrote:
>> 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.
>>
>
> The patch looks good, but I think the kernel documentation at the top of
> drm_gpusvm.c should be updated—particularly the examples. It may also be
> worth updating the section explaining how pages are embedded in
> driver-side ranges, including the options for one-to-one or many-to-one
> mappings and the implications of each choice.
>
Got it, will update the kernel documents in drm_gpusvm.c for both 1:1
and 1:n mappings.
Regards,
Honglei
> Matt
>
>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>> drivers/gpu/drm/drm_gpusvm.c | 68 ------------------------------------
>> include/drm/drm_gpusvm.h | 13 -------
>> 2 files changed, 81 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
>> index a4b56cefeb2..55515390c53 100644
>> --- a/drivers/gpu/drm/drm_gpusvm.c
>> +++ b/drivers/gpu/drm/drm_gpusvm.c
>> @@ -640,7 +640,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->flags.migrate_devmem = migrate_devmem ? 1 : 0;
>>
>> return range;
>> @@ -1329,27 +1328,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
>> @@ -1633,29 +1611,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->drm,
>> - 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
>> @@ -1686,29 +1641,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 21baf91ec7e..250c59f0930 100644
>> --- a/include/drm/drm_gpusvm.h
>> +++ b/include/drm/drm_gpusvm.h
>> @@ -173,7 +173,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
>> @@ -185,7 +184,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;
>> };
>>
>> @@ -303,20 +301,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 [flat|nested] 18+ messages in thread
* [RFC 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level
2026-06-03 6:56 [RFC 0/5] drm/gpusvm: split MM and device state across Honglei Huang
` (3 preceding siblings ...)
2026-06-03 6:56 ` [RFC 4/5] drm/gpusvm: move struct drm_gpusvm_pages out of struct drm_gpusvm_range Honglei Huang
@ 2026-06-03 6:56 ` Honglei Huang
2026-06-10 4:19 ` Matthew Brost
2026-06-10 3:44 ` [RFC 0/5] drm/gpusvm: split MM and device state across Matthew Brost
5 siblings, 1 reply; 18+ messages in thread
From: Honglei Huang @ 2026-06-03 6:56 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, 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 drop struct drm_device from struct 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.
- Drop the drm from struct drm_gpusvm
- Drop the drm parameter from drm_gpusvm_init()
- Update the xe call sites in xe_svm_init() and other callers.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 7 +++----
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, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 55515390c53..5cade46234c 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -359,7 +359,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.
@@ -373,7 +372,8 @@ 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 struct @drm for dma
+ * mappings is now required in drm_gpusvm_get_pages(). 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.
@@ -381,7 +381,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,
@@ -399,7 +399,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 33c26df5111..b0b737234ee 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -905,7 +905,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,
@@ -919,7 +919,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 ea73241d3d9..1c5195f5495 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -238,7 +238,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 250c59f0930..2bea47ee171 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -191,7 +191,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
@@ -215,7 +214,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;
@@ -267,7 +265,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] 18+ messages in thread* Re: [RFC 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level
2026-06-03 6:56 ` [RFC 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level Honglei Huang
@ 2026-06-10 4:19 ` Matthew Brost
2026-06-10 9:10 ` Huang, Honglei
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Brost @ 2026-06-10 4:19 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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, honghuan
On Wed, Jun 03, 2026 at 02:56:20PM +0800, Honglei Huang wrote:
> 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 drop struct drm_device from struct 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.
>
> - Drop the drm from struct drm_gpusvm
> - Drop the drm parameter from drm_gpusvm_init()
> - Update the xe call sites in xe_svm_init() and other callers.
>
I'd mention somewhere that drm_device is now stored in the pages.
Otherwise LGTM.
Matt
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 7 +++----
> 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, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 55515390c53..5cade46234c 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -359,7 +359,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.
> @@ -373,7 +372,8 @@ 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 struct @drm for dma
> + * mappings is now required in drm_gpusvm_get_pages(). 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.
> @@ -381,7 +381,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,
> @@ -399,7 +399,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 33c26df5111..b0b737234ee 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -905,7 +905,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,
> @@ -919,7 +919,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 ea73241d3d9..1c5195f5495 100644
> --- a/drivers/gpu/drm/xe/xe_svm.h
> +++ b/drivers/gpu/drm/xe/xe_svm.h
> @@ -238,7 +238,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 250c59f0930..2bea47ee171 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -191,7 +191,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
> @@ -215,7 +214,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;
> @@ -267,7 +265,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 [flat|nested] 18+ messages in thread* Re: [RFC 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level
2026-06-10 4:19 ` Matthew Brost
@ 2026-06-10 9:10 ` Huang, Honglei
0 siblings, 0 replies; 18+ messages in thread
From: Huang, Honglei @ 2026-06-10 9:10 UTC (permalink / raw)
To: Matthew Brost
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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,
Honglei Huang
On 6/10/2026 12:19 PM, Matthew Brost wrote:
> On Wed, Jun 03, 2026 at 02:56:20PM +0800, Honglei Huang wrote:
>> 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 drop struct drm_device from struct 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.
>>
>> - Drop the drm from struct drm_gpusvm
>> - Drop the drm parameter from drm_gpusvm_init()
>> - Update the xe call sites in xe_svm_init() and other callers.
>>
>
> I'd mention somewhere that drm_device is now stored in the pages.
>
> Otherwise LGTM.
Got it, will add text for the drm_device store location.
And thanks a lot for the review and your quick fix for my bugs in this
series!
will make fixes to the opinions you have provided. And will send the
patches to the Xe mailing list to trigger the CI test according the link
in cover letter.
Regards,
Honglei
>
> Matt
>
>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>> drivers/gpu/drm/drm_gpusvm.c | 7 +++----
>> 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, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
>> index 55515390c53..5cade46234c 100644
>> --- a/drivers/gpu/drm/drm_gpusvm.c
>> +++ b/drivers/gpu/drm/drm_gpusvm.c
>> @@ -359,7 +359,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.
>> @@ -373,7 +372,8 @@ 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 struct @drm for dma
>> + * mappings is now required in drm_gpusvm_get_pages(). 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.
>> @@ -381,7 +381,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,
>> @@ -399,7 +399,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 33c26df5111..b0b737234ee 100644
>> --- a/drivers/gpu/drm/xe/xe_svm.c
>> +++ b/drivers/gpu/drm/xe/xe_svm.c
>> @@ -905,7 +905,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,
>> @@ -919,7 +919,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 ea73241d3d9..1c5195f5495 100644
>> --- a/drivers/gpu/drm/xe/xe_svm.h
>> +++ b/drivers/gpu/drm/xe/xe_svm.h
>> @@ -238,7 +238,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 250c59f0930..2bea47ee171 100644
>> --- a/include/drm/drm_gpusvm.h
>> +++ b/include/drm/drm_gpusvm.h
>> @@ -191,7 +191,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
>> @@ -215,7 +214,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;
>> @@ -267,7 +265,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 [flat|nested] 18+ messages in thread
* Re: [RFC 0/5] drm/gpusvm: split MM and device state across
2026-06-03 6:56 [RFC 0/5] drm/gpusvm: split MM and device state across Honglei Huang
` (4 preceding siblings ...)
2026-06-03 6:56 ` [RFC 5/5] drm/gpusvm: let the drm_gpusvm core context purely MM level Honglei Huang
@ 2026-06-10 3:44 ` Matthew Brost
2026-06-10 8:49 ` Huang, Honglei
5 siblings, 1 reply; 18+ messages in thread
From: Matthew Brost @ 2026-06-10 3:44 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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, honghuan
On Wed, Jun 03, 2026 at 02:56:15PM +0800, Honglei Huang wrote:
> From: Honglei Huang <honghuan@amd.com>
>
> The intent of this series is to make drm_gpusvm more flexible and
> give drivers more freedom over how they assemble the MM related and device
> side operations.
>
> This RFC implements the direction Matt suggested in [1]:
>
> - Move struct drm_gpusvm_pages out of struct drm_gpusvm_range.
> - Embed either a struct device or a struct drm_device in struct
> drm_gpusvm_pages.
> - Drop struct drm_device from struct drm_gpusvm.
> - Have the driver's range structure embed one or more struct
> drm_gpusvm_pages in addition to struct drm_gpusvm_range.
> - Refactor a few range-based helpers (drm_gpusvm_range_pages_valid,
> drm_gpusvm_range_get_pages, drm_gpusvm_range_unmap_pages), or
> simply drop them entirely and update drivers to use the
> drm_gpusvm_pages helpers instead.
>
Overall this looks good - thanks doing this.
> In essence the series does only two abstractions, plus the xe
> adaptation that follows from them:
>
> - range vs pages: split drm_gpusvm_range (MM / VA range state) from
> drm_gpusvm_pages (device physical related), so the two
> sides can have independent lifetimes and ownership.
> - drm_gpusvm vs drm_device: make drm_gpusvm pure MM level and push
> the device side down onto drm_gpusvm_pages, which is where DMA
> actually happens.
> - xe is updated to fit the modifications, no functional change intended.
>
> If such changes are acceptable in terms of direction, I have a few questions:
>
> - Drivers now own drm_gpusvm_pages unmap / free and notifier_seq init.
> OK to push this fully to drivers, or should some new mechanisms need to add
> to ensure functions can be completed by the framework?
I'm looking at the diff of xe_svm.c before / after and I see
drm_gpusvm_free_pages moved to xe_svm_range_free. That looks fine to me.
I see in xe_svm_range_alloc() this:
range->pages.notifier_seq = LONG_MAX;
Can we make help like drm_gpusvm_init_pages which does this? I think it
is better to encapsulate the pages init into normalized helper even
though it is very simple. Maybe an inline since this just a single line
of code?
> - This series drops the three drm_gpusvm_range_* helpers and changes
> drm_gpusvm_get_pages() / drm_gpusvm_init() signatures.
> Do we need to keep thin wrappers for backward compatibility.
It should be safe to drop these helpers.
> - drm_gpusvm_get_pages() mixes HMM fault and device DMA map. Multi device under
> one SVM calls would repeat the HMM fault. Does it need to modified to Split
> into MM level fault + per pages DMA map?
>
Hmm, this might get a little tricky because of how the allocation/retry
loop is implemented in get_pages(). Maybe we could change the function
to accept an array of pages plus a count? I’m not sure what the best
approach is here, but I’m open to ideas. That said, I’d rather avoid
having the driver open-code a retry loop if it could live in common
code.
Side note: another modification we need in get_pages() is to make the
DMA-mapping step optional. I suggested that AMDXDNA use GPU SVM for
userptr, and I don’t believe that device requires DMA mapping.
> Patch overview:
>
> 1/5 gpusvm: split MM state flags onto drm_gpusvm_range_flags.
> 2/5 gpusvm: embed drm_device into drm_gpusvm_pages; DMA goes
> through it.
> 3/5 xe: xe_svm_range owns its drm_gpusvm_pages and its lifecycle.
> 4/5 gpusvm: drop pages from drm_gpusvm_range and the range-level
> wrappers.
> 5/5 gpusvm: drop drm_device from drm_gpusvm.
>
> tests:
> AMDGPU:
> based on amdgpu adaptation patch in [2], but still SVM:DRM = 1:1,
> 1:n is on going needs many modifications and testings.
>
> Tested on gfx943 (MI300X) and gfx906 (MI60) with XNACK on/off:
> - KFD test: 95%+ passed.
> - ROCR test: all passed.
> - HIP catch test: gfx943 (MI300X): 96% passed.
> gfx906 (MI60): 99% passed.
> INTEL XE:
> TODO: We bought some Intel Arc A380, but it seems like this cards
> don't support hardware fault / SVM, waiting for the new
> cards B580/B570 to arrive.
>
Please send patches that modify GPU SVM or Xe to the Xe mailing list. We
have public CI, which I believe can be triggered by any AMD email
address.
I just pulled the code, encountered a compile error, and noticed a bug
around unmapping related to that error. I put together some quick fixes
on top of the series here [3], and locally all of our tests seem to be
passing.
I’ll reply in detail to the patches shortly, explaining some of the
reasoning behind these changes.
Matt
[3] https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/623f6a50c037d9e44f6c9fbe6859a0ba7ad50177
> links:
> [1] https://lore.kernel.org/amd-gfx/acRgr7QwdULsn6G2@gsse-cloud1/#:~:text=I%20think%20roughly,drm_gpusvm_pages%0A%20%20helpers%20instead.
> [2] https://lore.kernel.org/amd-gfx/20260603065030.2554403-1-honglei1.huang@amd.com/
> Honglei Huang (5):
> drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
> drm/gpusvm: embed struct drm_device into drm_gpusvm_pages
> drm/xe: have xe_svm_range embed one drm_gpusvm_pages
> drm/gpusvm: move struct drm_gpusvm_pages out of struct
> drm_gpusvm_range
> drm/gpusvm: let the drm_gpusvm core context purely MM level
>
> drivers/gpu/drm/drm_gpusvm.c | 128 +++++++++-----------------------
> drivers/gpu/drm/xe/xe_pt.c | 2 +-
> drivers/gpu/drm/xe/xe_svm.c | 37 +++++----
> drivers/gpu/drm/xe/xe_svm.h | 11 ++-
> drivers/gpu/drm/xe/xe_userptr.c | 1 +
> include/drm/drm_gpusvm.h | 49 ++++++------
> 6 files changed, 95 insertions(+), 133 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 0/5] drm/gpusvm: split MM and device state across
2026-06-10 3:44 ` [RFC 0/5] drm/gpusvm: split MM and device state across Matthew Brost
@ 2026-06-10 8:49 ` Huang, Honglei
0 siblings, 0 replies; 18+ messages in thread
From: Huang, Honglei @ 2026-06-10 8:49 UTC (permalink / raw)
To: Matthew Brost
Cc: sima, rodrigo.vivi, thomas.hellstrom, dakr, 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,
Honglei Huang
On 6/10/2026 11:44 AM, Matthew Brost wrote:
> On Wed, Jun 03, 2026 at 02:56:15PM +0800, Honglei Huang wrote:
>> From: Honglei Huang <honghuan@amd.com>
>>
>> The intent of this series is to make drm_gpusvm more flexible and
>> give drivers more freedom over how they assemble the MM related and device
>> side operations.
>>
>> This RFC implements the direction Matt suggested in [1]:
>>
>> - Move struct drm_gpusvm_pages out of struct drm_gpusvm_range.
>> - Embed either a struct device or a struct drm_device in struct
>> drm_gpusvm_pages.
>> - Drop struct drm_device from struct drm_gpusvm.
>> - Have the driver's range structure embed one or more struct
>> drm_gpusvm_pages in addition to struct drm_gpusvm_range.
>> - Refactor a few range-based helpers (drm_gpusvm_range_pages_valid,
>> drm_gpusvm_range_get_pages, drm_gpusvm_range_unmap_pages), or
>> simply drop them entirely and update drivers to use the
>> drm_gpusvm_pages helpers instead.
>>
>
> Overall this looks good - thanks doing this.
>
>> In essence the series does only two abstractions, plus the xe
>> adaptation that follows from them:
>>
>> - range vs pages: split drm_gpusvm_range (MM / VA range state) from
>> drm_gpusvm_pages (device physical related), so the two
>> sides can have independent lifetimes and ownership.
>> - drm_gpusvm vs drm_device: make drm_gpusvm pure MM level and push
>> the device side down onto drm_gpusvm_pages, which is where DMA
>> actually happens.
>> - xe is updated to fit the modifications, no functional change intended.
>>
>> If such changes are acceptable in terms of direction, I have a few questions:
>>
>> - Drivers now own drm_gpusvm_pages unmap / free and notifier_seq init.
>> OK to push this fully to drivers, or should some new mechanisms need to add
>> to ensure functions can be completed by the framework?
>
> I'm looking at the diff of xe_svm.c before / after and I see
> drm_gpusvm_free_pages moved to xe_svm_range_free. That looks fine to me.
>
> I see in xe_svm_range_alloc() this:
>
> range->pages.notifier_seq = LONG_MAX;
>
> Can we make help like drm_gpusvm_init_pages which does this? I think it
> is better to encapsulate the pages init into normalized helper even
> though it is very simple. Maybe an inline since this just a single line
> of code?
Got it will add helper drm_gpusvm_init_pages in next version.
>
>> - This series drops the three drm_gpusvm_range_* helpers and changes
>> drm_gpusvm_get_pages() / drm_gpusvm_init() signatures.
>> Do we need to keep thin wrappers for backward compatibility.
>
> It should be safe to drop these helpers.
Got it.
>
>> - drm_gpusvm_get_pages() mixes HMM fault and device DMA map. Multi device under
>> one SVM calls would repeat the HMM fault. Does it need to modified to Split
>> into MM level fault + per pages DMA map?
>>
>
> Hmm, this might get a little tricky because of how the allocation/retry
> loop is implemented in get_pages(). Maybe we could change the function
> to accept an array of pages plus a count? I’m not sure what the best
> approach is here, but I’m open to ideas. That said, I’d rather avoid
> having the driver open-code a retry loop if it could live in common
> code.
>
> Side note: another modification we need in get_pages() is to make the
> DMA-mapping step optional. I suggested that AMDXDNA use GPU SVM for
> userptr, and I don’t believe that device requires DMA mapping.
Got it, will try to change the get_pages() to accept array of pages and
a count, and see if it will work well.
For the no DMA mapping condition, maybe a flag like no_dma_mapping can
be added in drm_gpusvm_ctx or someplace else.
Regards,
Honglei
>
>> Patch overview:
>>
>> 1/5 gpusvm: split MM state flags onto drm_gpusvm_range_flags.
>> 2/5 gpusvm: embed drm_device into drm_gpusvm_pages; DMA goes
>> through it.
>> 3/5 xe: xe_svm_range owns its drm_gpusvm_pages and its lifecycle.
>> 4/5 gpusvm: drop pages from drm_gpusvm_range and the range-level
>> wrappers.
>> 5/5 gpusvm: drop drm_device from drm_gpusvm.
>>
>> tests:
>> AMDGPU:
>> based on amdgpu adaptation patch in [2], but still SVM:DRM = 1:1,
>> 1:n is on going needs many modifications and testings.
>>
>> Tested on gfx943 (MI300X) and gfx906 (MI60) with XNACK on/off:
>> - KFD test: 95%+ passed.
>> - ROCR test: all passed.
>> - HIP catch test: gfx943 (MI300X): 96% passed.
>> gfx906 (MI60): 99% passed.
>> INTEL XE:
>> TODO: We bought some Intel Arc A380, but it seems like this cards
>> don't support hardware fault / SVM, waiting for the new
>> cards B580/B570 to arrive.
>>
>
> Please send patches that modify GPU SVM or Xe to the Xe mailing list. We
> have public CI, which I believe can be triggered by any AMD email
> address.
>
> I just pulled the code, encountered a compile error, and noticed a bug
> around unmapping related to that error. I put together some quick fixes
> on top of the series here [3], and locally all of our tests seem to be
> passing.
>
> I’ll reply in detail to the patches shortly, explaining some of the
> reasoning behind these changes.
>
> Matt
>
> [3] https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/623f6a50c037d9e44f6c9fbe6859a0ba7ad50177
>
>> links:
>> [1] https://lore.kernel.org/amd-gfx/acRgr7QwdULsn6G2@gsse-cloud1/#:~:text=I%20think%20roughly,drm_gpusvm_pages%0A%20%20helpers%20instead.
>> [2] https://lore.kernel.org/amd-gfx/20260603065030.2554403-1-honglei1.huang@amd.com/
>> Honglei Huang (5):
>> drm/gpusvm: split MM state flags out of drm_gpusvm_pages_flags
>> drm/gpusvm: embed struct drm_device into drm_gpusvm_pages
>> drm/xe: have xe_svm_range embed one drm_gpusvm_pages
>> drm/gpusvm: move struct drm_gpusvm_pages out of struct
>> drm_gpusvm_range
>> drm/gpusvm: let the drm_gpusvm core context purely MM level
>>
>> drivers/gpu/drm/drm_gpusvm.c | 128 +++++++++-----------------------
>> drivers/gpu/drm/xe/xe_pt.c | 2 +-
>> drivers/gpu/drm/xe/xe_svm.c | 37 +++++----
>> drivers/gpu/drm/xe/xe_svm.h | 11 ++-
>> drivers/gpu/drm/xe/xe_userptr.c | 1 +
>> include/drm/drm_gpusvm.h | 49 ++++++------
>> 6 files changed, 95 insertions(+), 133 deletions(-)
>>
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 18+ messages in thread