On Wed, Jun 03, 2026 at 02:56:16PM +0800, Honglei Huang wrote: > From: Honglei Huang <[email protected]> > > 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 <[email protected]> > Signed-off-by: Honglei Huang <[email protected]> > --- > 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 >
