Re: [PATCH v10 0/5] drm/gpusvm: split MM and device state across gpusvm/range/pages

["Huang, Honglei" <honghuan@amd.com>]

On 7/9/2026 6:56 AM, Matthew Brost wrote:
> On Wed, Jul 01, 2026 at 11:34:57AM +0800, Huang, Honglei wrote:
>>
>>
>> On 6/30/2026 10:26 PM, Matthew Brost wrote:
>>> On Tue, Jun 30, 2026 at 06:21:22PM +0800, Honglei Huang wrote:
>>>> 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. It implements the direction Matt suggested in [1]:
>>>> Mirror MR in gitlab: [4]
>>>>
>>>>     - Move struct drm_gpusvm_pages out of struct drm_gpusvm_range.
>>>>     - Embed a struct drm_device in struct drm_gpusvm_pages and drive all
>>>>       DMA through it.
>>>>     - 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.
>>>>     - Drop the range-based helpers (drm_gpusvm_range_pages_valid,
>>>>       drm_gpusvm_range_get_pages, drm_gpusvm_range_unmap_pages) and update
>>>>       drivers to use the drm_gpusvm_pages helpers instead.
>>>>
>>>> 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.
>>>>
>>>> V10:
>>>>     Fix two issues found by the AI review:
>>>>     - patch 1: fix a KCSAN data race in xe_svm_alloc_vram(): read
>>>>       range->base.flags.__flags with READ_ONCE() and assert on a local
>>>>       copy, pairing with the WRITE_ONCE() in
>>>>       drm_gpusvm_range_set_unmapped().
>>>>     - patch 3: remove the DMA unmap in xe_svm_fini(): since
>>>>       drm_gpusvm_range_remove() no longer unmaps synchronously, explicitly
>>>>       drm_gpusvm_unmap_pages() all remaining ranges before
>>>>       drm_gpusvm_fini().
>>>>
>>>> V9:
>>>>     - patch 3: fix the build with CONFIG_DRM_XE_GPUSVM disabled: move
>>>>       pages out of the nested base struct in the stub xe_svm_range and
>>>>       route has_dma_mapping through range->pages. No functional change.
>>>>
>>>> V8:
>>>>     - patch 4: add reviewed-by for Matt's review.
>>>>
>>>> V7:
>>>>     - patch 1: split MM state flags: the AI review found a KCSAN / memory
>>>>       model cleanliness issue. Address it for consistency with
>>>>       drm_gpusvm_pages_flags, set the range flags with WRITE_ONCE() on
>>>>       __flags and read them with READ_ONCE().
>>>>
>>>> V6:
>>>>     - The AI review flagged a potential DMA free issue: the DMA unmap
>>>>       step was moved into the range_free callback, but on the invalidate
>>>>       path a range can be removed from the MMU interval tree while its DMA
>>>>       mappings are still live, so a concurrent unmap event can miss it.
>>>>     - patch 3: have xe_svm_range embed one drm_gpusvm_pages: explicitly
>>>>       call drm_gpusvm_unmap_pages() before drm_gpusvm_range_remove() in the
>>>>       garbage collector, so a range is never off the tree while still DMA
>>>>       mapped, and document this caller contract in drm_gpusvm_range_remove()
>>>>       kernel-doc.
>>>>     - patch 4: move struct drm_gpusvm_pages out: document the
>>>>       unmap before remove contract in the garbage collector example and
>>>>       note that range_free()'s drm_gpusvm_free_pages() as a final fallback.
>>>>     - patch 1: split MM state flags: return -EACCES directly.
>>>>     - Fold in the pre existing IOVA/DMA unmap fixes the AI review found
>>>>       previously sent separately: the uninitialized dma_addr[0].dir on
>>>>       the get_pages() error path, the whole reservation IOVA free for
>>>>       mixed ranges, and the device mapping leak on the get_pages() error
>>>>       path. [6]
>>>>
>>>> V5:
>>>>     - add reviewed-by in patches 1, 2, 3, 5 for Matt's review.
>>>>
>>>> V4:
>>>>     - drm_gpusvm_init_pages(): memset() the pages to zero before recording
>>>>       the owning drm_device.
>>>>     - DOC: overview: recommend a zeroing allocator: kcalloc() for the
>>>>       N:1 pages array.
>>>>     - Rebased onto the latest drm-xe.
>>>>     - The AI review of this series flagged two preexisting issues in the
>>>>       IOVA unmap path that are not introduced by this series; they are
>>>>       fixed in a separate series [5].
>>>>
>>>> V3:
>>>>     - Fix a kernel-doc/Sphinx warning from the kernel test robot: use
>>>>       ".. code-block:: c" for the drm_gpusvm_pages example in DOC: overview.
>>>>     - drm_gpusvm_range_set_unmapped(): use WRITE_ONCE() on the whole
>>>>       pages[i].flags.__flags word to pair with the lockless READ_ONCE()
>>>>       readers and avoid a data race.
>>>>     - xe_userptr_setup(): call drm_gpusvm_init_pages() before
>>>>       mmu_interval_notifier_insert() to avoid exposing uninitialized
>>>>       pages.drm to invalidation callbacks.
>>>>     - Fix per commit build of the set_unmapped() pages.
>>>>
>>>> V2:
>>>>     - Followed in Matt's v0 review fixups [2]:
>>>>        - keep unmapped flag in pages structures.
>>>>        - add pages_count to drm_gpusvm_range_set_unmapped() to set the pages
>>>>          unmapped flag, so the framework can check unmapped status in
>>>>          drm_gpusvm_get_pages().
>>>>     - Add drm_gpusvm_init_pages to init the drm_device and sequence number.
>>>>     - Remove drm_device from drm_gpusvm_get_pages() parameters.
>>>>     - Reworked the DOC: overview and usage examples to describe the new
>>>>       model: struct drm_gpusvm_pages, the 1:1 / N:1 driver layouts, and
>>>>       examples that operate on a driver embedded pages object by the
>>>>       drm_gpusvm_pages helpers and etc.
>>>>     - remove WARN_ON_ONCE in __drm_gpusvm_unmap_pages.
>>>>     - Dropped RFC.
>>>>
>>>> Follow-up (not in this series):
>>>>
>>>>     - modify drm_gpusvm_get_pages() to support one time hmm range fault
>>>>       and multi drm device dma mapping.
>>>>     - Add no dma device support for drm_gpusvm_get_pages().
>>>>
>>>> tests:
>>>> AMDGPU:
>>>>     based on amdgpu adaptation patch in [3], 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): 99% passed.
>>>>                       gfx906 (MI60): 99% passed.
>>>> INTEL XE:
>>>>     CI tests passed in rev5 tests.[7]
>>>
>>>
>>> CI full is in flight, but it seems likely to pass. Once it completes, we
>>> can merge this; we just need to decide which branch to use.
>>>
>>> The series does not apply cleanly to drm-misc-next because it depends on
>>> changes in drm-xe-next that have not yet been merged into drm-misc-next.
>>> It does apply cleanly to drm-xe-next, though.
>>>
>>> If we merge the patches in drm-xe-next and pick them up via a rebase in
>>> the AMD trees, that works. Alternatively, we can ask the drm-misc-next
>>> maintainers to rebase and apply them there. Either option works for me.
>>>
>>
>> Thanks a lot, Matt, for the guidance and the reviews in this series.
>>
>> For the merge, happy to rebase onto whatever base/tag decided, just
>> let me know.
>>
>> And will work on the follow up works.
> 
> This now applies to drm-misc-next, so going to apply there by EOD unless
> I hear an objection.
> 

Thanks Matt!

No objection from my side, drm-misc-next works for me. Thanks again
for all the reviews and for handling the merge.

I'll rebase the amdgpu SVM on top of drm-misc-next once this lands.

Regards,

> Matt
> 
>>
>> Regards,
>> Honglei
>>
>>> Matt
>>>
>>>>
>>>> links:
>>>> [1] https://lore.kernel.org/amd-gfx/acRgr7QwdULsn6G2@gsse-cloud1/#:~:text=I%20think%20roughly,drm_gpusvm_pages%0A%20%20helpers%20instead.
>>>> [2] https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/623f6a50c037d9e44f6c9fbe6859a0ba7ad50177
>>>> [3] https://lore.kernel.org/amd-gfx/20260603065030.2554403-1-honglei1.huang@amd.com/
>>>> [4] https://gitlab.freedesktop.org/drm/xe/kernel/-/merge_requests/360
>>>> [5] https://lore.kernel.org/all/20260627033325.3795298-1-honglei1.huang@amd.com/
>>>> [6] https://lore.kernel.org/all/20260628061757.4093701-1-honglei1.huang@amd.com/
>>>> [7] https://patchwork.freedesktop.org/series/169384/#rev5
>>>>
>>>> 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    | 243 +++++++++++++++++++-------------
>>>>    drivers/gpu/drm/xe/xe_pt.c      |   2 +-
>>>>    drivers/gpu/drm/xe/xe_svm.c     |  66 +++++++--
>>>>    drivers/gpu/drm/xe/xe_svm.h     |  14 +-
>>>>    drivers/gpu/drm/xe/xe_userptr.c |   5 +-
>>>>    include/drm/drm_gpusvm.h        |  67 ++++++---
>>>>    6 files changed, 255 insertions(+), 142 deletions(-)
>>>>
>>>> -- 
>>>> 2.34.1
>>>>
>>