* [PATCH v4 0/6] drm/gpusvm: share one HMM fault and keep single mappings inline
@ 2026-09-05 13:31 Honglei Huang
2026-09-05 13:31 ` [PATCH v4 1/6] drm/gpusvm: move dma_addr allocation before the notifier lock Honglei Huang
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Honglei Huang @ 2026-09-05 13:31 UTC (permalink / raw)
To: matthew.brost, sima, rodrigo.vivi, thomas.hellstrom,
himal.prasad.ghimiray, dakr, intel-xe, dri-devel
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Ray.Huang, Junhua.Shen, amd-gfx, honghuan
Patches 1 to 4 are the get_pages() split already posted as v2. Patches 5
and 6 implement the dma_addr storage optimization Matt suggested in [6].
They build on the DMA paths v2 reworks, so they are sent in the same
series; I can split them out if that is preferred.
drm_gpusvm_get_pages() does two things at once: the MM level HMM fault
of the CPU range, and the device DMA mapping of the faulted pages. When
one CPU range is mirrored on several devices, every device has to call
get_pages() and redo the HMM fault. Matt suggested [2] passing an array
of drm_gpusvm_pages plus a count so the fault is taken once and shared,
while the notifier retry loop stays in common code so drivers never open
code it.
For patches 5 and 6, the focus is on optimizing the storage of DMA
addresses for THP pages and IOVA mapped ranges. get_pages() sizes
dma_addr for the worst case of one entry per page, but the mapping loop
advances by page order, so a 2 MiB THP holds an 8 KiB array with 16
bytes of address in it. Patch 5 keeps that single mapping inline. Patch
6 extends the optimization to IOVA mapped ranges, where contiguous
device addresses allow for a similar inline approach.
A range folds only when one drm_pagemap_addr describes all of it, which
leaves three shapes not included:
- Mixed device and system pages: the device addresses come from
device_map(), outside the IOVA reservation, so they are not
contiguous. Not included for inline storage.
- Mixed orders: contiguous under IOVA and foldable in principle, but
one entry carries one order; expressing two needs an entry count or
a segment iterator. Needs additional handling for mixed orders.
So not included for this inline storage change.
- Several huge entries: needs a chunk larger than PMD size, which no
consumer configures today.
Patches overview:
- patch 1 moves the dma_addr allocation out of the notifier locked
section, so the mapping step becomes self contained.
- patch 2 pulls the per-device mapping loop into
drm_gpusvm_dma_map_pages(). Code motion only.
- patch 3 makes get_pages() take an array of drm_gpusvm_pages plus a
count: fault once, then DMA map each instance, one per owning
drm_device, under a single notifier retry gate. Instances that are
already mapped are skipped, so an -EAGAIN retry does not redo them.
The common 1:1 case passes count == 1 and is unchanged.
- patch 4 adds a no_dma_map context flag so a driver that only needs
the CPU pages faulted in can skip the device DMA mapping.
- patch 5: keeps a single mapping inline for THP.
- patch 6: extends that to IOVA mapped ranges.
v2:
- Rebased on drm-tip.
- Dropped v1 patch 1 ("drm/gpusvm: extract drm_gpusvm_hmm_fault()
helper"), it is part of [3] now.
- patch 3: drm_gpusvm_pages_valid_unlocked() takes the array and the
count itself, instead of get_pages() open coding an all_valid loop.
- patch 3: fixed the N:1 doc example, it used the wrong union member
when the count is 1. Added a driver_pages() accessor.
- patch 3: documented that on error the instances mapped before the
failing one stay mapped, the caller must unmap and free all of them.
- patch 4: reject no_dma_map together with devmem_only, without the
DMA mapping step there is no page type check to enforce it.
- patch 4: documented that no_dma_map only returns a snapshot, the
caller must recheck mmu_interval_read_retry() itself.
V3:
- Patch 3: check svm_pages[0].flags.unmapped instead of walking every
instance, per Matt's review. drm_gpusvm_range_set_unmapped() flags
the whole array in one go under the write lock, so any instance
answers for all of them.
- Patch 3: reject a zero page count instead of relying on the caller.
- Patch 5: new, keeps a single mapping inline for THP.
- Patch 6: new, extends that to IOVA mapped range.
- Add Reviewed-by Matt in patches 1, 2 and 4.
V4:
- Patch 5: add xe_svm_range_first_dma(), with a stub for the
CONFIG_DRM_XE_GPUSVM disabled build, so xe_pt_stage_bind() type
checks in both configurations. Reported by Intel CI [7].
- Patch 6: the wrapper hands out the contiguous flag too, and the stub
clears it.
- Add Reviewed-by Matt in patch 3.
tests:
AMDGPU:
SVM:DRM N:1 multi device support is work in progress on top of this
series. The single device (1:1) path was tested with the amdgpu SVM
adaptation on top. Based on amdgpu SVM [4].
Tested on gfx906 (MI60) with XNACK on, in three configurations:
THP ON + IOVA ON, THP ON + IOVA OFF, NO THP + NO IOVA.
- KFD test: SVM all passed except the get attr refactor.
- ROCR test: all passed.
- HIP catch test: gfx943 (MI300X): 99% passed.
gfx906 (MI60): 99% passed.
links:
[1] drm_gpusvm_pages decoupling series:
https://lore.kernel.org/amd-gfx/20260630102127.392396-1-honghuan@amd.com/
[2] Matt's suggested direction:
https://lore.kernel.org/amd-gfx/aijdg7RWwrEDEMxC@gsse-cloud1.jf.intel.com/
[3] drm/gpusvm: use hmm_range_fault_unlocked_timeout() for range faults:
https://lore.kernel.org/20260723-hmm-v10-v11-8-c55b003a4b61@gmail.com
[4] amdgpu SVM:
https://lore.kernel.org/amd-gfx/20260804094246.1719318-1-ray.huang@amd.com/
[5] v2 of this series:
https://lore.kernel.org/amd-gfx/20260901090100.2024933-1-honghuan@amd.com/
[6] Matt on keeping the dma address inline:
https://lore.kernel.org/amd-gfx/apcp%2FXJpPdG3jzPd@gsse-cloud1.jf.intel.com/
[7] Intel CI build report on v3:
https://patchwork.freedesktop.org/series/173405/
Honglei Huang (6):
drm/gpusvm: move dma_addr allocation before the notifier lock
drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper
drm/gpusvm: let drm_gpusvm_get_pages() map an array of pages
drm/gpusvm: make the DMA mapping step in get_pages() optional
drm/gpusvm: keep a single DMA mapping inline for THP
drm/gpusvm: keep an IOVA mapped range dma address inline
drivers/gpu/drm/drm_gpusvm.c | 406 +++++++++++++++++++++--------
drivers/gpu/drm/xe/xe_pt.c | 22 +-
drivers/gpu/drm/xe/xe_res_cursor.h | 5 +-
drivers/gpu/drm/xe/xe_svm.c | 2 +-
drivers/gpu/drm/xe/xe_svm.h | 20 ++
drivers/gpu/drm/xe/xe_userptr.c | 2 +-
include/drm/drm_gpusvm.h | 71 ++++-
7 files changed, 400 insertions(+), 128 deletions(-)
base-commit: 313da1cc22491f07075c7ae36372343aa235d11e
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 1/6] drm/gpusvm: move dma_addr allocation before the notifier lock
2026-09-05 13:31 [PATCH v4 0/6] drm/gpusvm: share one HMM fault and keep single mappings inline Honglei Huang
@ 2026-09-05 13:31 ` Honglei Huang
2026-09-05 13:31 ` [PATCH v4 2/6] drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper Honglei Huang
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Honglei Huang @ 2026-09-05 13:31 UTC (permalink / raw)
To: matthew.brost, sima, rodrigo.vivi, thomas.hellstrom,
himal.prasad.ghimiray, dakr, intel-xe, dri-devel
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Ray.Huang, Junhua.Shen, amd-gfx, honghuan
The dma_addr allocation was in a lazy allocation flow, it needs unlock
and goto map_pages. The allocation only needs npages, so just do it
before taking the lock. Drop the map_pages label and the relock flow, so
the sequence becomes fault, allocate, then lock, validate, map and
unlock. No functional change intended.
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index a93eee7ddb9..b507de539e6 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1516,10 +1516,18 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
if (err)
goto err_free;
+ if (!svm_pages->dma_addr) {
+ svm_pages->dma_addr =
+ kvzalloc_objs(*svm_pages->dma_addr, npages);
+ if (!svm_pages->dma_addr) {
+ err = -ENOMEM;
+ goto err_free;
+ }
+ }
+
*state = (struct dma_iova_state){};
svm_pages->state_offset = 0;
-map_pages:
/*
* Perform all dma mappings under the notifier lock to not
* access freed pages. A notifier will either block on
@@ -1540,18 +1548,6 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
goto retry;
}
- if (!svm_pages->dma_addr) {
- /* Unlock and restart mapping to allocate memory. */
- drm_gpusvm_notifier_unlock(gpusvm);
- svm_pages->dma_addr =
- kvzalloc_objs(*svm_pages->dma_addr, npages);
- if (!svm_pages->dma_addr) {
- err = -ENOMEM;
- goto err_free;
- }
- goto map_pages;
- }
-
zdd = NULL;
pagemap = NULL;
num_dma_mapped = 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 2/6] drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper
2026-09-05 13:31 [PATCH v4 0/6] drm/gpusvm: share one HMM fault and keep single mappings inline Honglei Huang
2026-09-05 13:31 ` [PATCH v4 1/6] drm/gpusvm: move dma_addr allocation before the notifier lock Honglei Huang
@ 2026-09-05 13:31 ` Honglei Huang
2026-09-05 13:31 ` [PATCH v4 3/6] drm/gpusvm: let drm_gpusvm_get_pages() map an array of pages Honglei Huang
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Honglei Huang @ 2026-09-05 13:31 UTC (permalink / raw)
To: matthew.brost, sima, rodrigo.vivi, thomas.hellstrom,
himal.prasad.ghimiray, dakr, intel-xe, dri-devel
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Ray.Huang, Junhua.Shen, amd-gfx, honghuan
Move the per-device DMA mapping loop of drm_gpusvm_get_pages() into a
helper drm_gpusvm_dma_map_pages(). The mapping logic is only moved, not
changed, so there is no functional change.
The helper maps the already-faulted pfns into one drm_gpusvm_pages
instance under the notifier lock and unwinds its own partial mapping on
error. The HMM fault and the notifier retry loop stay in get_pages()
common code rather than being pushed down to drivers, so no driver has
to reimplement the subtle fault and retry logic.
With the mapping isolated per instance, get_pages() can later fault once
and DMA map an array of drm_gpusvm_pages plus a count, one per owning
drm_device.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 225 ++++++++++++++++++++---------------
1 file changed, 129 insertions(+), 96 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index b507de539e6..89c3061d8ef 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1442,115 +1442,41 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
}
/**
- * drm_gpusvm_get_pages() - Get pages and populate GPU SVM pages struct
+ * drm_gpusvm_dma_map_pages() - DMA map one drm_gpusvm_pages instance
* @gpusvm: Pointer to the GPU SVM structure
- * @svm_pages: The SVM pages to populate. This will contain the dma-addresses
- * @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
- * @pages_end: End CPU address for the pages (exclusive)
+ * @svm_pages: The SVM pages instance to populate with dma-addresses
+ * @pfns: The already-faulted pfn array (size @npages)
+ * @npages: Number of pages in the CPU range
* @ctx: GPU SVM context
+ * @dma_dir: DMA data direction for the mappings
*
- * This function gets and maps pages for CPU range and ensures they are
- * mapped for DMA access.
+ * Map the faulted @pfns into @svm_pages for DMA access through its owning
+ * drm_device. Must be called under the notifier lock. On failure this unwinds
+ * the partial mapping of this instance before returning.
*
* Return: 0 on success, negative error code on failure.
*/
-int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_pages *svm_pages,
- struct mm_struct *mm,
- struct mmu_interval_notifier *notifier,
- unsigned long pages_start, unsigned long pages_end,
- const struct drm_gpusvm_ctx *ctx)
+static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
+ struct drm_gpusvm_pages *svm_pages,
+ unsigned long *pfns,
+ unsigned long npages,
+ const struct drm_gpusvm_ctx *ctx,
+ enum dma_data_direction dma_dir)
{
- struct hmm_range hmm_range = {
- .default_flags = HMM_PFN_REQ_FAULT | (ctx->read_only ? 0 :
- HMM_PFN_REQ_WRITE),
- .notifier = notifier,
- .start = pages_start,
- .end = pages_end,
- .dev_private_owner = ctx->device_private_page_owner,
- };
- void *zdd;
- unsigned long timeout =
- jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
- unsigned long remaining;
+ void *zdd = NULL;
unsigned long i, j;
- unsigned long npages = npages_in_range(pages_start, pages_end);
- unsigned long num_dma_mapped;
+ unsigned long num_dma_mapped = 0;
unsigned int order = 0;
- unsigned long *pfns;
int err = 0;
- struct dev_pagemap *pagemap;
+ struct dev_pagemap *pagemap = NULL;
struct drm_pagemap *dpagemap;
struct drm_gpusvm_pages_flags flags;
- enum dma_data_direction dma_dir = ctx->read_only ? DMA_TO_DEVICE :
- DMA_BIDIRECTIONAL;
struct dma_iova_state *state = &svm_pages->state;
- if (!svm_pages->drm)
- return -EINVAL;
-
-retry:
- remaining = timeout - jiffies;
-
- if (time_after_eq(jiffies, timeout))
- return -EBUSY;
-
- hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
- if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages))
- goto set_seqno;
-
- pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
- if (!pfns)
- return -ENOMEM;
-
- if (!mmget_not_zero(mm)) {
- err = -EFAULT;
- goto err_free;
- }
-
- hmm_range.hmm_pfns = pfns;
- err = hmm_range_fault_unlocked_timeout(&hmm_range, remaining);
- mmput(mm);
- if (err)
- goto err_free;
-
- if (!svm_pages->dma_addr) {
- svm_pages->dma_addr =
- kvzalloc_objs(*svm_pages->dma_addr, npages);
- if (!svm_pages->dma_addr) {
- err = -ENOMEM;
- goto err_free;
- }
- }
-
- *state = (struct dma_iova_state){};
- svm_pages->state_offset = 0;
-
- /*
- * Perform all dma mappings under the notifier lock to not
- * access freed pages. A notifier will either block on
- * the notifier lock or unmap dma.
- */
- drm_gpusvm_notifier_lock(gpusvm);
+ lockdep_assert_held(&gpusvm->notifier_lock);
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);
- kvfree(pfns);
- goto retry;
- }
- zdd = NULL;
- pagemap = NULL;
- num_dma_mapped = 0;
for (i = 0, j = 0; i < npages; ++j) {
struct page *page = hmm_pfn_to_page(pfns[i]);
@@ -1666,17 +1592,124 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
/* WRITE_ONCE pairs with READ_ONCE for opportunistic checks */
WRITE_ONCE(svm_pages->flags.__flags, flags.__flags);
+ return 0;
+
+err_unmap:
+ svm_pages->flags.has_dma_mapping = true;
+ __drm_gpusvm_unmap_pages(gpusvm, svm_pages, num_dma_mapped);
+ return err;
+}
+
+/**
+ * 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
+ * @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
+ * @pages_end: End CPU address for the pages (exclusive)
+ * @ctx: GPU SVM context
+ *
+ * This function gets and maps pages for CPU range and ensures they are
+ * mapped for DMA access.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
+ struct drm_gpusvm_pages *svm_pages,
+ struct mm_struct *mm,
+ struct mmu_interval_notifier *notifier,
+ unsigned long pages_start, unsigned long pages_end,
+ const struct drm_gpusvm_ctx *ctx)
+{
+ struct hmm_range hmm_range = {
+ .default_flags = HMM_PFN_REQ_FAULT | (ctx->read_only ? 0 :
+ HMM_PFN_REQ_WRITE),
+ .notifier = notifier,
+ .start = pages_start,
+ .end = pages_end,
+ .dev_private_owner = ctx->device_private_page_owner,
+ };
+ unsigned long timeout =
+ jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+ unsigned long remaining;
+ unsigned long npages = npages_in_range(pages_start, pages_end);
+ unsigned long *pfns;
+ int err = 0;
+ enum dma_data_direction dma_dir = ctx->read_only ? DMA_TO_DEVICE :
+ DMA_BIDIRECTIONAL;
+
+ if (!svm_pages->drm)
+ return -EINVAL;
+
+retry:
+ remaining = timeout - jiffies;
+
+ if (time_after_eq(jiffies, timeout))
+ return -EBUSY;
+
+ hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
+ if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages))
+ goto set_seqno;
+
+ pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
+ if (!pfns)
+ return -ENOMEM;
+
+ if (!mmget_not_zero(mm)) {
+ err = -EFAULT;
+ goto err_free;
+ }
+
+ hmm_range.hmm_pfns = pfns;
+ err = hmm_range_fault_unlocked_timeout(&hmm_range, remaining);
+ mmput(mm);
+ if (err)
+ goto err_free;
+
+ if (!svm_pages->dma_addr) {
+ svm_pages->dma_addr =
+ kvzalloc_objs(*svm_pages->dma_addr, npages);
+ if (!svm_pages->dma_addr) {
+ err = -ENOMEM;
+ goto err_free;
+ }
+ }
+
+ svm_pages->state = (struct dma_iova_state){};
+ svm_pages->state_offset = 0;
+
+ /*
+ * Perform all dma mappings under the notifier lock to not
+ * access freed pages. A notifier will either block on
+ * the notifier lock or unmap dma.
+ */
+ drm_gpusvm_notifier_lock(gpusvm);
+
+ if (svm_pages->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);
+ kvfree(pfns);
+ goto retry;
+ }
+
+ err = drm_gpusvm_dma_map_pages(gpusvm, svm_pages, pfns, npages, ctx,
+ dma_dir);
drm_gpusvm_notifier_unlock(gpusvm);
+ if (err)
+ goto err_free;
+
kvfree(pfns);
set_seqno:
svm_pages->notifier_seq = hmm_range.notifier_seq;
return 0;
-err_unmap:
- svm_pages->flags.has_dma_mapping = true;
- __drm_gpusvm_unmap_pages(gpusvm, svm_pages, num_dma_mapped);
- drm_gpusvm_notifier_unlock(gpusvm);
err_free:
kvfree(pfns);
if (err == -EAGAIN)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 3/6] drm/gpusvm: let drm_gpusvm_get_pages() map an array of pages
2026-09-05 13:31 [PATCH v4 0/6] drm/gpusvm: share one HMM fault and keep single mappings inline Honglei Huang
2026-09-05 13:31 ` [PATCH v4 1/6] drm/gpusvm: move dma_addr allocation before the notifier lock Honglei Huang
2026-09-05 13:31 ` [PATCH v4 2/6] drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper Honglei Huang
@ 2026-09-05 13:31 ` Honglei Huang
2026-09-05 13:31 ` [PATCH v4 4/6] drm/gpusvm: make the DMA mapping step in get_pages() optional Honglei Huang
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Honglei Huang @ 2026-09-05 13:31 UTC (permalink / raw)
To: matthew.brost, sima, rodrigo.vivi, thomas.hellstrom,
himal.prasad.ghimiray, dakr, intel-xe, dri-devel
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Ray.Huang, Junhua.Shen, amd-gfx, honghuan
With the N:1 drm_gpusvm_pages layout, one CPU range mirrored on several
drm_devices, the caller had to invoke get_pages() once per device and
repeat the HMM fault every time.
Make get_pages() take a contiguous array of drm_gpusvm_pages plus a
count: fault once, then DMA map each instance by
drm_gpusvm_dma_map_pages() under a single read_retry gate. xe range and
userptr callers are updated.
Document the N:1 array usage in the Overview, showing how get_pages()
and drm_gpusvm_range_set_unmapped() take the whole array and its count
while the unmap and free paths stay per-instance.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 140 ++++++++++++++++++++++++--------
drivers/gpu/drm/xe/xe_svm.c | 2 +-
drivers/gpu/drm/xe/xe_userptr.c | 2 +-
include/drm/drm_gpusvm.h | 1 +
4 files changed, 110 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 89c3061d8ef..d8ee0e5f2ae 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -80,6 +80,13 @@
* };
* };
*
+ * static struct drm_gpusvm_pages *
+ * driver_pages(struct driver_range *drange)
+ * {
+ * return drange->num_pages == 1 ? &drange->inline_pages :
+ * drange->pages;
+ * }
+ *
* In the N:1 case the driver allocates the pages array with a zeroing
* allocator (e.g. kcalloc(num_pages, ...)), initialises each entry with
* drm_gpusvm_init_pages(), and frees each entry with
@@ -89,6 +96,28 @@
* Each drm_gpusvm_pages must be zero-initialised and initialised with
* drm_gpusvm_init_pages(), called once per entry.
*
+ * The 1:1 examples below pass @num_pages == 1 and &drange->pages. In the
+ * N:1 case the driver instead passes the whole array and its count, so a
+ * single call faults the CPU range once and DMA maps it for every owning
+ * drm_device, e.g.:
+ *
+ * .. code-block:: c
+ *
+ * // GPU fault handler: one fault, one DMA mapping per device
+ * err = drm_gpusvm_get_pages(gpusvm, driver_pages(drange),
+ * drange->num_pages, gpusvm->mm,
+ * &range->notifier->notifier,
+ * drm_gpusvm_range_start(range),
+ * drm_gpusvm_range_end(range), &ctx);
+ *
+ * // Notifier callback: mark every instance unmapped in one call
+ * drm_gpusvm_range_set_unmapped(range, driver_pages(drange),
+ * drange->num_pages, mmu_range);
+ *
+ * The unmap and free paths stay per-instance: iterate @num_pages over
+ * driver_pages(drange) and call drm_gpusvm_unmap_pages() /
+ * drm_gpusvm_free_pages() for each entry.
+ *
* - Operations:
* Define the interface for driver-specific GPU SVM operations such as
* range allocation, notifier allocation, and invalidations.
@@ -232,7 +261,7 @@
* goto retry;
* }
*
- * err = drm_gpusvm_get_pages(gpusvm, &drange->pages,
+ * err = drm_gpusvm_get_pages(gpusvm, &drange->pages, 1,
* gpusvm->mm, &range->notifier->notifier,
* drm_gpusvm_range_start(range),
* drm_gpusvm_range_end(range), &ctx);
@@ -1417,25 +1446,35 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_pages_valid);
/**
* drm_gpusvm_pages_valid_unlocked() - GPU SVM pages valid unlocked
* @gpusvm: Pointer to the GPU SVM structure
- * @svm_pages: Pointer to the GPU SVM pages structure
+ * @svm_pages: Array of GPU SVM pages structures
+ * @num_pages: Number of drm_gpusvm_pages instances in @svm_pages
*
- * This function determines if a GPU SVM pages are valid. Expected be called
- * without holding gpusvm->notifier_lock.
+ * This function determines if every GPU SVM pages instance is valid, resetting
+ * every instance which is not so that get_pages() maps it afresh. It therefore
+ * has to walk them all. Expected be called without holding
+ * gpusvm->notifier_lock.
*
- * Return: True if GPU SVM pages are valid, False otherwise
+ * Return: True if all GPU SVM pages are valid, False otherwise
*/
static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_pages *svm_pages)
+ struct drm_gpusvm_pages *svm_pages,
+ unsigned int num_pages)
{
- bool pages_valid;
+ bool pages_valid = true;
+ unsigned int p;
- if (!svm_pages->dma_addr)
- return false;
+ for (p = 0; p < num_pages; ++p) {
+ if (!svm_pages[p].dma_addr)
+ return false;
+ }
drm_gpusvm_notifier_lock(gpusvm);
- pages_valid = drm_gpusvm_pages_valid(gpusvm, svm_pages);
- if (!pages_valid)
- __drm_gpusvm_free_pages(gpusvm, svm_pages);
+ for (p = 0; p < num_pages; ++p) {
+ if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
+ continue;
+ __drm_gpusvm_free_pages(gpusvm, &svm_pages[p]);
+ pages_valid = false;
+ }
drm_gpusvm_notifier_unlock(gpusvm);
return pages_valid;
@@ -1451,8 +1490,9 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
* @dma_dir: DMA data direction for the mappings
*
* Map the faulted @pfns into @svm_pages for DMA access through its owning
- * drm_device. Must be called under the notifier lock. On failure this unwinds
- * the partial mapping of this instance before returning.
+ * drm_device. Must be called under the notifier lock and only for an instance
+ * without a live mapping. On failure this unwinds the partial mapping of this
+ * instance before returning.
*
* Return: 0 on success, negative error code on failure.
*/
@@ -1475,6 +1515,9 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
lockdep_assert_held(&gpusvm->notifier_lock);
+ *state = (struct dma_iova_state){};
+ svm_pages->state_offset = 0;
+
flags.__flags = svm_pages->flags.__flags;
for (i = 0, j = 0; i < npages; ++j) {
@@ -1603,20 +1646,28 @@ static int drm_gpusvm_dma_map_pages(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
+ * @svm_pages: Array of SVM pages instances to populate with dma addresses
+ * @num_pages: Number of drm_gpusvm_pages instances in @svm_pages, must not be 0
* @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
* @pages_end: End CPU address for the pages (exclusive)
* @ctx: GPU SVM context
*
- * This function gets and maps pages for CPU range and ensures they are
- * mapped for DMA access.
+ * This function gets and maps pages for a CPU range and ensures they are
+ * mapped for DMA access. The HMM fault for the CPU range is performed once,
+ * the DMA mapping by drm_gpusvm_dma_map_pages() is then done per instance,
+ * one per owning drm_device. The retry against notifier races is kept here
+ * in common code so drivers never open code it.
+ *
+ * On error the instances mapped before the failing one stay mapped, so the
+ * caller must unmap and free every instance regardless of the return value.
*
* Return: 0 on success, negative error code on failure.
*/
int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_pages *svm_pages,
+ unsigned int num_pages,
struct mm_struct *mm,
struct mmu_interval_notifier *notifier,
unsigned long pages_start, unsigned long pages_end,
@@ -1638,10 +1689,15 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
int err = 0;
enum dma_data_direction dma_dir = ctx->read_only ? DMA_TO_DEVICE :
DMA_BIDIRECTIONAL;
+ unsigned int p;
- if (!svm_pages->drm)
+ if (!num_pages)
return -EINVAL;
+ for (p = 0; p < num_pages; ++p)
+ if (!svm_pages[p].drm)
+ return -EINVAL;
+
retry:
remaining = timeout - jiffies;
@@ -1649,7 +1705,8 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
return -EBUSY;
hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
- if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages))
+
+ if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages, num_pages))
goto set_seqno;
pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
@@ -1667,18 +1724,17 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
if (err)
goto err_free;
- if (!svm_pages->dma_addr) {
- svm_pages->dma_addr =
- kvzalloc_objs(*svm_pages->dma_addr, npages);
- if (!svm_pages->dma_addr) {
+ for (p = 0; p < num_pages; ++p) {
+ if (svm_pages[p].dma_addr)
+ continue;
+ svm_pages[p].dma_addr =
+ kvzalloc_objs(*svm_pages[p].dma_addr, npages);
+ if (!svm_pages[p].dma_addr) {
err = -ENOMEM;
goto err_free;
}
}
- svm_pages->state = (struct dma_iova_state){};
- svm_pages->state_offset = 0;
-
/*
* Perform all dma mappings under the notifier lock to not
* access freed pages. A notifier will either block on
@@ -1686,7 +1742,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
*/
drm_gpusvm_notifier_lock(gpusvm);
- if (svm_pages->flags.unmapped) {
+ /*
+ * drm_gpusvm_range_set_unmapped() flags the whole array in one go under
+ * the write lock, so any instance answers for all of them here.
+ */
+ if (svm_pages[0].flags.unmapped) {
drm_gpusvm_notifier_unlock(gpusvm);
err = -EFAULT;
goto err_free;
@@ -1698,15 +1758,29 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
goto retry;
}
- err = drm_gpusvm_dma_map_pages(gpusvm, svm_pages, pfns, npages, ctx,
- dma_dir);
- drm_gpusvm_notifier_unlock(gpusvm);
- if (err)
- goto err_free;
+ for (p = 0; p < num_pages; ++p) {
+ if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
+ continue;
+ err = drm_gpusvm_dma_map_pages(gpusvm, &svm_pages[p], pfns,
+ npages, ctx, dma_dir);
+ if (err) {
+ /*
+ * The failing instance was unwound by the helper. Keep
+ * the ones mapped earlier: the -EAGAIN retry reuses
+ * them, and the driver unmaps every instance with the
+ * range on the other error paths.
+ */
+ drm_gpusvm_notifier_unlock(gpusvm);
+ goto err_free;
+ }
+ }
+
+ drm_gpusvm_notifier_unlock(gpusvm);
kvfree(pfns);
set_seqno:
- svm_pages->notifier_seq = hmm_range.notifier_seq;
+ for (p = 0; p < num_pages; ++p)
+ svm_pages[p].notifier_seq = hmm_range.notifier_seq;
return 0;
diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
index 627a741293d..1c7793d8caa 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -1598,7 +1598,7 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
lockdep_assert_held(&range->lock);
- err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages,
+ err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages, 1,
vm->svm.gpusvm.mm,
&range->base.notifier->notifier,
drm_gpusvm_range_start(&range->base),
diff --git a/drivers/gpu/drm/xe/xe_userptr.c b/drivers/gpu/drm/xe/xe_userptr.c
index 90ac141fc12..9c1dac0fce6 100644
--- a/drivers/gpu/drm/xe/xe_userptr.c
+++ b/drivers/gpu/drm/xe/xe_userptr.c
@@ -91,7 +91,7 @@ int xe_vma_userptr_pin_pages(struct xe_userptr_vma *uvma)
if (vma->gpuva.flags & XE_VMA_DESTROYED)
return 0;
- return drm_gpusvm_get_pages(&vm->svm.gpusvm, &uvma->userptr.pages,
+ return drm_gpusvm_get_pages(&vm->svm.gpusvm, &uvma->userptr.pages, 1,
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 b7d987bf76a..d2b6f3d2b84 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -324,6 +324,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,
+ unsigned int num_pages,
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] 11+ messages in thread
* [PATCH v4 4/6] drm/gpusvm: make the DMA mapping step in get_pages() optional
2026-09-05 13:31 [PATCH v4 0/6] drm/gpusvm: share one HMM fault and keep single mappings inline Honglei Huang
` (2 preceding siblings ...)
2026-09-05 13:31 ` [PATCH v4 3/6] drm/gpusvm: let drm_gpusvm_get_pages() map an array of pages Honglei Huang
@ 2026-09-05 13:31 ` Honglei Huang
2026-09-05 13:31 ` [PATCH v4 5/6] drm/gpusvm: keep a single DMA mapping inline for THP Honglei Huang
2026-09-05 13:31 ` [PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline Honglei Huang
5 siblings, 0 replies; 11+ messages in thread
From: Honglei Huang @ 2026-09-05 13:31 UTC (permalink / raw)
To: matthew.brost, sima, rodrigo.vivi, thomas.hellstrom,
himal.prasad.ghimiray, dakr, intel-xe, dri-devel
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Ray.Huang, Junhua.Shen, amd-gfx, honghuan
Some drivers (e.g. AMDXDNA) only need the CPU pages faulted in and tracked
by the notifier, no need DMA mapping.
Add a drm_gpusvm_ctx::no_dma_map flag. When set, get_pages() does the
shared HMM fault and records notifier_seq, but skips svm_pages->drm
validation, the dma_addr allocation and drm_gpusvm_dma_map_pages().
With no mapping state to check, the fault is redone on every call. The
default (no_dma_map == 0) is unchanged.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 44 ++++++++++++++++++++++++++----------
include/drm/drm_gpusvm.h | 9 ++++++++
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index d8ee0e5f2ae..7efc35507f1 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1663,6 +1663,12 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
* On error the instances mapped before the failing one stay mapped, so the
* caller must unmap and free every instance regardless of the return value.
*
+ * With &drm_gpusvm_ctx.no_dma_map no mapping state is recorded, so
+ * drm_gpusvm_pages_valid() never returns true and success is only a snapshot:
+ * the caller must recheck mmu_interval_read_retry() against the recorded
+ * &drm_gpusvm_pages.notifier_seq under the notifier lock, and hold it until
+ * its work is visible to invalidation.
+ *
* Return: 0 on success, negative error code on failure.
*/
int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
@@ -1689,14 +1695,21 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
int err = 0;
enum dma_data_direction dma_dir = ctx->read_only ? DMA_TO_DEVICE :
DMA_BIDIRECTIONAL;
+ const bool map_dma = !ctx->no_dma_map;
unsigned int p;
if (!num_pages)
return -EINVAL;
- for (p = 0; p < num_pages; ++p)
- if (!svm_pages[p].drm)
- return -EINVAL;
+ if (ctx->no_dma_map && ctx->devmem_only)
+ return -EINVAL;
+
+ if (map_dma) {
+ for (p = 0; p < num_pages; ++p) {
+ if (!svm_pages[p].drm)
+ return -EINVAL;
+ }
+ }
retry:
remaining = timeout - jiffies;
@@ -1706,7 +1719,8 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
- if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages, num_pages))
+ if (map_dma &&
+ drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages, num_pages))
goto set_seqno;
pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
@@ -1724,14 +1738,16 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
if (err)
goto err_free;
- for (p = 0; p < num_pages; ++p) {
- if (svm_pages[p].dma_addr)
- continue;
- svm_pages[p].dma_addr =
- kvzalloc_objs(*svm_pages[p].dma_addr, npages);
- if (!svm_pages[p].dma_addr) {
- err = -ENOMEM;
- goto err_free;
+ if (map_dma) {
+ for (p = 0; p < num_pages; ++p) {
+ if (svm_pages[p].dma_addr)
+ continue;
+ svm_pages[p].dma_addr =
+ kvzalloc_objs(*svm_pages[p].dma_addr, npages);
+ if (!svm_pages[p].dma_addr) {
+ err = -ENOMEM;
+ goto err_free;
+ }
}
}
@@ -1758,6 +1774,9 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
goto retry;
}
+ if (!map_dma)
+ goto done_mapping;
+
for (p = 0; p < num_pages; ++p) {
if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
continue;
@@ -1776,6 +1795,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
}
}
+done_mapping:
drm_gpusvm_notifier_unlock(gpusvm);
kvfree(pfns);
set_seqno:
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index d2b6f3d2b84..ec7b81957b1 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -254,6 +254,14 @@ struct drm_gpusvm {
* @allow_mixed: Allow mixed mappings in get pages. Mixing between system and
* single dpagemap is supported, mixing between multiple dpagemap
* is unsupported.
+ * @no_dma_map: Only fault the CPU pages for the range; skip the device DMA
+ * mapping step. Used by drivers that consume the faulted pages
+ * without needing a DMA mapping. In this mode @drm on the
+ * drm_gpusvm_pages is not required, no mapping state is recorded
+ * and drm_gpusvm_pages_valid() therefore never reports these
+ * pages as valid; the caller revalidates the snapshot itself, see
+ * drm_gpusvm_get_pages(). @devmem_only is rejected and no page
+ * type check is performed, so @allow_mixed has no effect.
*
* Context that is DRM GPUSVM is operating in (i.e. user arguments).
*/
@@ -266,6 +274,7 @@ struct drm_gpusvm_ctx {
unsigned int devmem_possible :1;
unsigned int devmem_only :1;
unsigned int allow_mixed :1;
+ unsigned int no_dma_map :1;
};
int drm_gpusvm_init(struct drm_gpusvm *gpusvm,
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 5/6] drm/gpusvm: keep a single DMA mapping inline for THP
2026-09-05 13:31 [PATCH v4 0/6] drm/gpusvm: share one HMM fault and keep single mappings inline Honglei Huang
` (3 preceding siblings ...)
2026-09-05 13:31 ` [PATCH v4 4/6] drm/gpusvm: make the DMA mapping step in get_pages() optional Honglei Huang
@ 2026-09-05 13:31 ` Honglei Huang
2026-09-08 3:12 ` Matthew Brost
2026-09-05 13:31 ` [PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline Honglei Huang
5 siblings, 1 reply; 11+ messages in thread
From: Honglei Huang @ 2026-09-05 13:31 UTC (permalink / raw)
To: matthew.brost, sima, rodrigo.vivi, thomas.hellstrom,
himal.prasad.ghimiray, dakr, intel-xe, dri-devel
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Ray.Huang, Junhua.Shen, amd-gfx, honghuan
drm_gpusvm_get_pages() sizes the dma_addr array for one drm_pagemap_addr
per page, but the mapping loop advances by page order, so a range backed
by one huge page needs a single entry. For a 2 MiB THP that is an 8 KiB
array holding 16 bytes of address.
Union that entry with the array pointer, discriminated by a new
inline_dma_mapping flag. When drm_gpusvm_dma_map_pages() ends up with one
entry it stores it inline and frees the array, after the last error
unwind, which still walks the array form. An unchecked dma_addr read is
now type confusion rather than a compile error, so reads go through the
new drm_gpusvm_pages_first_dma() accessor, including the two
xe_pt_stage_bind() paths.
Only get_pages() and the free path write the union, never the notifier,
and both run under the driver lock that every address reader already
holds. The unlocked short circuit in drm_gpusvm_pages_valid_unlocked()
goes for the same reason: it cannot resolve the union, and every instance
it rejects has to be reset before the allocation loop reuses it.
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 48 ++++++++++++++++++++++++++++------
drivers/gpu/drm/xe/xe_pt.c | 7 ++---
drivers/gpu/drm/xe/xe_svm.h | 18 +++++++++++++
include/drm/drm_gpusvm.h | 50 +++++++++++++++++++++++++++++++++---
4 files changed, 109 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 7efc35507f1..2c7c4c89dc4 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1241,6 +1241,8 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_pages_flags flags = {
.__flags = svm_pages->flags.__flags,
};
+ const struct drm_pagemap_addr *addrs =
+ drm_gpusvm_pages_first_dma(svm_pages);
bool use_iova = dma_use_iova(&svm_pages->state);
/*
@@ -1253,12 +1255,12 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
if (svm_pages->state_offset)
dma_iova_unlink(dev, &svm_pages->state, 0,
svm_pages->state_offset,
- svm_pages->dma_addr[0].dir, 0);
+ addrs[0].dir, 0);
dma_iova_free(dev, &svm_pages->state);
}
for (i = 0, j = 0; i < npages; j++) {
- struct drm_pagemap_addr *addr = &svm_pages->dma_addr[j];
+ const struct drm_pagemap_addr *addr = &addrs[j];
if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
/*
@@ -1299,6 +1301,18 @@ static void __drm_gpusvm_free_pages(struct drm_gpusvm *gpusvm,
{
lockdep_assert_held(&gpusvm->notifier_lock);
+ if (svm_pages->flags.inline_dma_mapping) {
+ struct drm_gpusvm_pages_flags flags = {
+ .__flags = svm_pages->flags.__flags,
+ };
+
+ svm_pages->inline_addr = (struct drm_pagemap_addr){};
+ flags.inline_dma_mapping = false;
+ /* WRITE_ONCE pairs with READ_ONCE for opportunistic checks */
+ WRITE_ONCE(svm_pages->flags.__flags, flags.__flags);
+ return;
+ }
+
if (svm_pages->dma_addr) {
kvfree(svm_pages->dma_addr);
svm_pages->dma_addr = NULL;
@@ -1463,11 +1477,6 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
bool pages_valid = true;
unsigned int p;
- for (p = 0; p < num_pages; ++p) {
- if (!svm_pages[p].dma_addr)
- return false;
- }
-
drm_gpusvm_notifier_lock(gpusvm);
for (p = 0; p < num_pages; ++p) {
if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
@@ -1480,6 +1489,21 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
return pages_valid;
}
+/**
+ * drm_gpusvm_pages_inlinable() - Whether the dma address can be inlined
+ * @nentries: Number of entries the mapping loop produced
+ *
+ * A THP maps as one huge page, so the whole range needs a single device
+ * address: the dma_addr array can be freed and the address kept inline,
+ * which is where the memory saving comes from.
+ *
+ * Return: True if the mapping fits in a single drm_pagemap_addr.
+ */
+static bool drm_gpusvm_pages_inlinable(unsigned long nentries)
+{
+ return nentries == 1;
+}
+
/**
* drm_gpusvm_dma_map_pages() - DMA map one drm_gpusvm_pages instance
* @gpusvm: Pointer to the GPU SVM structure
@@ -1632,6 +1656,14 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
if (pagemap)
flags.has_devmem_pages = true;
+ if (drm_gpusvm_pages_inlinable(j)) {
+ struct drm_pagemap_addr addr = svm_pages->dma_addr[0];
+
+ kvfree(svm_pages->dma_addr);
+ svm_pages->inline_addr = addr;
+ flags.inline_dma_mapping = true;
+ }
+
/* WRITE_ONCE pairs with READ_ONCE for opportunistic checks */
WRITE_ONCE(svm_pages->flags.__flags, flags.__flags);
@@ -1740,7 +1772,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
if (map_dma) {
for (p = 0; p < num_pages; ++p) {
- if (svm_pages[p].dma_addr)
+ if (drm_gpusvm_pages_first_dma(&svm_pages[p]))
continue;
svm_pages[p].dma_addr =
kvzalloc_objs(*svm_pages[p].dma_addr, npages);
diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index 5d990c1c374..fa4b29da0b6 100644
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -831,7 +831,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->pages.dma_addr, 0,
+ xe_res_first_dma(xe_svm_range_first_dma(range), 0,
xe_svm_range_size(range),
&curs);
xe_svm_range_debug(range, "BIND PREPARE - MIXED");
@@ -866,8 +866,9 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
if (!xe_vma_is_null(vma) && !range && !is_purged) {
if (xe_vma_is_userptr(vma))
- xe_res_first_dma(to_userptr_vma(vma)->userptr.pages.dma_addr, 0,
- xe_vma_size(vma), &curs);
+ xe_res_first_dma(drm_gpusvm_pages_first_dma
+ (&to_userptr_vma(vma)->userptr.pages),
+ 0, xe_vma_size(vma), &curs);
else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
xe_vma_size(vma), &curs);
diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
index 2a0dc0d125c..7eb80d4d6db 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -220,6 +220,18 @@ static inline unsigned long xe_svm_range_size(struct xe_svm_range *range)
return drm_gpusvm_range_size(&range->base);
}
+/**
+ * xe_svm_range_first_dma() - Resolve the device address array of a SVM range
+ * @range: SVM range
+ *
+ * Return: Pointer to the first device address, NULL if none is populated.
+ */
+static inline const struct drm_pagemap_addr *
+xe_svm_range_first_dma(struct xe_svm_range *range)
+{
+ return drm_gpusvm_pages_first_dma(&range->pages);
+}
+
void xe_svm_flush(struct xe_vm *vm);
int xe_pagemap_shrinker_create(struct xe_device *xe);
@@ -436,6 +448,12 @@ static inline bool xe_svm_range_is_removed(struct xe_svm_range *range)
return false;
}
+static inline const struct drm_pagemap_addr *
+xe_svm_range_first_dma(struct xe_svm_range *range)
+{
+ return NULL;
+}
+
#define xe_svm_range_has_dma_mapping(...) false
#endif /* CONFIG_DRM_XE_GPUSVM */
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index ec7b81957b1..aaad5c9b510 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -10,6 +10,7 @@
#include <linux/kref.h>
#include <linux/interval_tree.h>
#include <linux/mmu_notifier.h>
+#include <drm/drm_pagemap.h>
struct dev_pagemap_ops;
struct drm_device;
@@ -18,7 +19,6 @@ struct drm_gpusvm_notifier;
struct drm_gpusvm_ops;
struct drm_gpusvm_range;
struct drm_pagemap;
-struct drm_pagemap_addr;
/**
* struct drm_gpusvm_ops - Operations structure for GPU SVM
@@ -112,6 +112,7 @@ struct drm_gpusvm_notifier {
* @unmapped: Flag indicating if the pages has been unmapped
* @has_devmem_pages: Flag indicating if the pages has devmem pages
* @has_dma_mapping: Flag indicating if the pages has a DMA mapping
+ * @inline_dma_mapping: Flag indicating if the pages have an inline DMA mapping
* @__flags: Flags for pages in u16 form (used for READ_ONCE)
*/
struct drm_gpusvm_pages_flags {
@@ -121,6 +122,7 @@ struct drm_gpusvm_pages_flags {
u16 unmapped : 1;
u16 has_devmem_pages : 1;
u16 has_dma_mapping : 1;
+ u16 inline_dma_mapping : 1;
};
u16 __flags;
};
@@ -130,17 +132,27 @@ 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
+ * @dma_addr: Device address array, valid while @flags.inline_dma_mapping is
+ * not set
+ * @inline_addr: Device address inline address, valid while
+ * @flags.inline_dma_mapping is set
* @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.
* @state: DMA IOVA state for mapping.
* @state_offset: DMA IOVA offset for mapping.
* @notifier_seq: Notifier sequence number of the range's pages
* @flags: Flags for the range; see &struct drm_gpusvm_pages_flags
+ *
+ * @dma_addr and @inline_addr share storage, discriminated by
+ * @flags.inline_dma_mapping. Driver should use drm_gpusvm_pages_first_dma()
+ * to access the correct DMA address.
*/
struct drm_gpusvm_pages {
struct drm_device *drm;
- struct drm_pagemap_addr *dma_addr;
+ union {
+ struct drm_pagemap_addr *dma_addr;
+ struct drm_pagemap_addr inline_addr;
+ };
struct drm_pagemap *dpagemap;
struct dma_iova_state state;
unsigned long state_offset;
@@ -365,6 +377,38 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
svm_pages->notifier_seq = LONG_MAX;
}
+/**
+ * drm_gpusvm_pages_first_dma() - Resolve the device address array
+ * @svm_pages: Pointer to the drm_gpusvm_pages.
+ *
+ * drm_gpusvm_pages use unions to optimize the storage of DMA addresses,
+ * this function abstracts the access to the first device address. The driver
+ * should use this helper instead of reading dma_addr directly to prevent
+ * array out of bounds access.
+ *
+ * Only get_pages() and the free path switch between the two union members.
+ * Both hold the notifier lock for read, so taking that lock does not stop
+ * them; callers need the driver lock that does, which every reader of the
+ * addresses holds anyway. The notifier never touches the union, so the
+ * pointer returned here stays good and can then be used under the notifier
+ * lock.
+ *
+ * Return: Pointer to the first device address, NULL if none is populated.
+ */
+static inline const struct drm_pagemap_addr *
+drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages)
+{
+ struct drm_gpusvm_pages_flags flags = {
+ /* READ_ONCE pairs with the WRITE_ONCE of the flag writers */
+ .__flags = READ_ONCE(svm_pages->flags.__flags),
+ };
+
+ if (flags.inline_dma_mapping)
+ return &svm_pages->inline_addr;
+
+ return READ_ONCE(svm_pages->dma_addr);
+}
+
/**
* enum drm_gpusvm_scan_result - Scan result from the drm_gpusvm_scan_mm() function.
* @DRM_GPUSVM_SCAN_UNPOPULATED: At least one page was not present or inaccessible.
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline
2026-09-05 13:31 [PATCH v4 0/6] drm/gpusvm: share one HMM fault and keep single mappings inline Honglei Huang
` (4 preceding siblings ...)
2026-09-05 13:31 ` [PATCH v4 5/6] drm/gpusvm: keep a single DMA mapping inline for THP Honglei Huang
@ 2026-09-05 13:31 ` Honglei Huang
2026-09-08 3:18 ` Matthew Brost
5 siblings, 1 reply; 11+ messages in thread
From: Honglei Huang @ 2026-09-05 13:31 UTC (permalink / raw)
To: matthew.brost, sima, rodrigo.vivi, thomas.hellstrom,
himal.prasad.ghimiray, dakr, intel-xe, dri-devel
Cc: aliceryhl, Alexander.Deucher, Felix.Kuehling, Christian.Koenig,
Ray.Huang, Junhua.Shen, amd-gfx, honghuan
dma_iova_try_alloc() reserves one contiguous IOVA for the whole range and
links each page at the next offset, so the device addresses run
contiguously from entry 0 and one entry describes them all. A 2 MiB range
of 4 KiB pages then drops the same 8 KiB array as a THP backed one.
Fold only when state_offset covers the full range, which proves no device
page was mapped in between, and only single page entries, so the order
kept is 0 and stays true. Widening it instead would tell a consumer to use
a huge page for npages separate CPU pages, which hangs Vega20 on amdgpu.
The kept entry no longer bounds the segment, so skip the unmap walk when
it has nothing to do, keyed off dpagemap rather than the flags, which are
not published yet on the error unwind. Consumers need the same
distinction, so drm_gpusvm_pages_first_dma() returns it alongside the
array from one read of the flags; xe passes it to xe_res_first_dma().
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/drm_gpusvm.c | 41 +++++++++++++++++++++++-------
drivers/gpu/drm/xe/xe_pt.c | 23 +++++++++++------
drivers/gpu/drm/xe/xe_res_cursor.h | 5 ++--
drivers/gpu/drm/xe/xe_svm.h | 8 +++---
include/drm/drm_gpusvm.h | 13 +++++++++-
5 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 2c7c4c89dc4..b6c9d3a07dc 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1242,7 +1242,7 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
.__flags = svm_pages->flags.__flags,
};
const struct drm_pagemap_addr *addrs =
- drm_gpusvm_pages_first_dma(svm_pages);
+ drm_gpusvm_pages_first_dma(svm_pages, NULL);
bool use_iova = dma_use_iova(&svm_pages->state);
/*
@@ -1259,7 +1259,15 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
dma_iova_free(dev, &svm_pages->state);
}
- for (i = 0, j = 0; i < npages; j++) {
+ /*
+ * With IOVA and no device page the unlink above tore every
+ * entry down, and that is also when the range may be folded
+ * to one entry, which must not be walked per entry. dpagemap
+ * is set before the first device_map(), so it is also right
+ * on the error path, where the flags are not published yet.
+ */
+ for (i = 0, j = 0;
+ (!use_iova || dpagemap) && i < npages; j++) {
const struct drm_pagemap_addr *addr = &addrs[j];
if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
@@ -1491,17 +1499,32 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
/**
* drm_gpusvm_pages_inlinable() - Whether the dma address can be inlined
+ * @svm_pages: The SVM pages instance that was just mapped
* @nentries: Number of entries the mapping loop produced
+ * @npages: Number of pages in the CPU range
*
- * A THP maps as one huge page, so the whole range needs a single device
- * address: the dma_addr array can be freed and the address kept inline,
- * which is where the memory saving comes from.
+ * A THP maps as one huge page, and an IOVA reservation links every page of
+ * the range at the next offset, so the device addresses run contiguously from
+ * entry 0. Either way one entry describes the whole range, so the dma_addr
+ * array can be freed and the address kept inline.
+ *
+ * state_offset advances only on the IOVA branch, so reaching the full range
+ * length proves no device page was mapped in between. Only single page
+ * entries fold, so the order kept is 0 and describes the range truthfully.
+ * Larger chunks, several huge pages among them, stay an array that is
+ * already short and that a consumer places with one PTE each.
*
* Return: True if the mapping fits in a single drm_pagemap_addr.
*/
-static bool drm_gpusvm_pages_inlinable(unsigned long nentries)
+static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
+ unsigned long nentries,
+ unsigned long npages)
{
- return nentries == 1;
+ if (nentries == 1)
+ return true;
+
+ return nentries == npages && dma_use_iova(&svm_pages->state) &&
+ svm_pages->state_offset == npages * PAGE_SIZE;
}
/**
@@ -1656,7 +1679,7 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
if (pagemap)
flags.has_devmem_pages = true;
- if (drm_gpusvm_pages_inlinable(j)) {
+ if (drm_gpusvm_pages_inlinable(svm_pages, j, npages)) {
struct drm_pagemap_addr addr = svm_pages->dma_addr[0];
kvfree(svm_pages->dma_addr);
@@ -1772,7 +1795,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
if (map_dma) {
for (p = 0; p < num_pages; ++p) {
- if (drm_gpusvm_pages_first_dma(&svm_pages[p]))
+ if (drm_gpusvm_pages_first_dma(&svm_pages[p], NULL))
continue;
svm_pages[p].dma_addr =
kvzalloc_objs(*svm_pages[p].dma_addr, npages);
diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index fa4b29da0b6..7fb2fe1f826 100644
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -831,9 +831,12 @@ 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(xe_svm_range_first_dma(range), 0,
- xe_svm_range_size(range),
- &curs);
+ const struct drm_pagemap_addr *addr;
+ bool contiguous;
+
+ addr = xe_svm_range_first_dma(range, &contiguous);
+ xe_res_first_dma(addr, 0, xe_svm_range_size(range),
+ contiguous, &curs);
xe_svm_range_debug(range, "BIND PREPARE - MIXED");
} else {
xe_assert(xe, false);
@@ -865,11 +868,15 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
xe_bo_assert_held(bo);
if (!xe_vma_is_null(vma) && !range && !is_purged) {
- if (xe_vma_is_userptr(vma))
- xe_res_first_dma(drm_gpusvm_pages_first_dma
- (&to_userptr_vma(vma)->userptr.pages),
- 0, xe_vma_size(vma), &curs);
- else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
+ if (xe_vma_is_userptr(vma)) {
+ const struct drm_pagemap_addr *addr;
+ bool contiguous;
+
+ addr = drm_gpusvm_pages_first_dma(&to_userptr_vma(vma)->userptr.pages,
+ &contiguous);
+ xe_res_first_dma(addr, 0, xe_vma_size(vma), contiguous,
+ &curs);
+ } else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
xe_vma_size(vma), &curs);
else
diff --git a/drivers/gpu/drm/xe/xe_res_cursor.h b/drivers/gpu/drm/xe/xe_res_cursor.h
index 0522caafd89..c3a037e5f34 100644
--- a/drivers/gpu/drm/xe/xe_res_cursor.h
+++ b/drivers/gpu/drm/xe/xe_res_cursor.h
@@ -233,12 +233,13 @@ static inline void xe_res_first_sg(const struct sg_table *sg,
* @dma_addr: struct drm_pagemap_addr array to walk
* @start: Start of the range
* @size: Size of the range
+ * @contiguous: Whether one entry describes the whole range
* @cur: cursor object to initialize
*
* Start walking over the range of allocations between @start and @size.
*/
static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
- u64 start, u64 size,
+ u64 start, u64 size, bool contiguous,
struct xe_res_cursor *cur)
{
XE_WARN_ON(!dma_addr);
@@ -248,7 +249,7 @@ static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
cur->node = NULL;
cur->start = start;
cur->remaining = size;
- cur->dma_seg_size = PAGE_SIZE << dma_addr->order;
+ cur->dma_seg_size = contiguous ? start + size : PAGE_SIZE << dma_addr->order;
cur->dma_start = 0;
cur->size = 0;
cur->dma_addr = dma_addr;
diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
index 7eb80d4d6db..2ef4ef026cc 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -223,13 +223,14 @@ static inline unsigned long xe_svm_range_size(struct xe_svm_range *range)
/**
* xe_svm_range_first_dma() - Resolve the device address array of a SVM range
* @range: SVM range
+ * @contiguous: Where to store whether one entry spans the whole range
*
* Return: Pointer to the first device address, NULL if none is populated.
*/
static inline const struct drm_pagemap_addr *
-xe_svm_range_first_dma(struct xe_svm_range *range)
+xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
{
- return drm_gpusvm_pages_first_dma(&range->pages);
+ return drm_gpusvm_pages_first_dma(&range->pages, contiguous);
}
void xe_svm_flush(struct xe_vm *vm);
@@ -449,8 +450,9 @@ static inline bool xe_svm_range_is_removed(struct xe_svm_range *range)
}
static inline const struct drm_pagemap_addr *
-xe_svm_range_first_dma(struct xe_svm_range *range)
+xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
{
+ *contiguous = false;
return NULL;
}
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index aaad5c9b510..9e35584812f 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -380,12 +380,19 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
/**
* drm_gpusvm_pages_first_dma() - Resolve the device address array
* @svm_pages: Pointer to the drm_gpusvm_pages.
+ * @contiguous: Where to store whether one entry spans the whole range, or NULL
*
* drm_gpusvm_pages use unions to optimize the storage of DMA addresses,
* this function abstracts the access to the first device address. The driver
* should use this helper instead of reading dma_addr directly to prevent
* array out of bounds access.
*
+ * @contiguous comes from the same read of the flags as the array itself, so a
+ * caller cannot see the two disagree and walk past that single entry into the
+ * fields behind it. When it is set the length comes from the range rather than
+ * from the order. The order still states what one PTE may cover: the range
+ * length for a huge page, PAGE_SIZE for an IOVA mapped range of single pages.
+ *
* Only get_pages() and the free path switch between the two union members.
* Both hold the notifier lock for read, so taking that lock does not stop
* them; callers need the driver lock that does, which every reader of the
@@ -396,13 +403,17 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
* Return: Pointer to the first device address, NULL if none is populated.
*/
static inline const struct drm_pagemap_addr *
-drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages)
+drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages,
+ bool *contiguous)
{
struct drm_gpusvm_pages_flags flags = {
/* READ_ONCE pairs with the WRITE_ONCE of the flag writers */
.__flags = READ_ONCE(svm_pages->flags.__flags),
};
+ if (contiguous)
+ *contiguous = flags.inline_dma_mapping;
+
if (flags.inline_dma_mapping)
return &svm_pages->inline_addr;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 5/6] drm/gpusvm: keep a single DMA mapping inline for THP
2026-09-05 13:31 ` [PATCH v4 5/6] drm/gpusvm: keep a single DMA mapping inline for THP Honglei Huang
@ 2026-09-08 3:12 ` Matthew Brost
0 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2026-09-08 3:12 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, himal.prasad.ghimiray, dakr,
intel-xe, dri-devel, aliceryhl, Alexander.Deucher, Felix.Kuehling,
Christian.Koenig, Ray.Huang, Junhua.Shen, amd-gfx
On Sat, Sep 05, 2026 at 09:31:41PM +0800, Honglei Huang wrote:
> drm_gpusvm_get_pages() sizes the dma_addr array for one drm_pagemap_addr
> per page, but the mapping loop advances by page order, so a range backed
> by one huge page needs a single entry. For a 2 MiB THP that is an 8 KiB
> array holding 16 bytes of address.
>
> Union that entry with the array pointer, discriminated by a new
> inline_dma_mapping flag. When drm_gpusvm_dma_map_pages() ends up with one
> entry it stores it inline and frees the array, after the last error
> unwind, which still walks the array form. An unchecked dma_addr read is
> now type confusion rather than a compile error, so reads go through the
> new drm_gpusvm_pages_first_dma() accessor, including the two
> xe_pt_stage_bind() paths.
>
> Only get_pages() and the free path write the union, never the notifier,
> and both run under the driver lock that every address reader already
> holds. The unlocked short circuit in drm_gpusvm_pages_valid_unlocked()
> goes for the same reason: it cannot resolve the union, and every instance
> it rejects has to be reset before the allocation loop reuses it.
>
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 48 ++++++++++++++++++++++++++++------
> drivers/gpu/drm/xe/xe_pt.c | 7 ++---
> drivers/gpu/drm/xe/xe_svm.h | 18 +++++++++++++
> include/drm/drm_gpusvm.h | 50 +++++++++++++++++++++++++++++++++---
> 4 files changed, 109 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 7efc35507f1..2c7c4c89dc4 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1241,6 +1241,8 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> struct drm_gpusvm_pages_flags flags = {
> .__flags = svm_pages->flags.__flags,
> };
> + const struct drm_pagemap_addr *addrs =
> + drm_gpusvm_pages_first_dma(svm_pages);
> bool use_iova = dma_use_iova(&svm_pages->state);
>
> /*
> @@ -1253,12 +1255,12 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> if (svm_pages->state_offset)
> dma_iova_unlink(dev, &svm_pages->state, 0,
> svm_pages->state_offset,
> - svm_pages->dma_addr[0].dir, 0);
> + addrs[0].dir, 0);
> dma_iova_free(dev, &svm_pages->state);
> }
>
> for (i = 0, j = 0; i < npages; j++) {
> - struct drm_pagemap_addr *addr = &svm_pages->dma_addr[j];
> + const struct drm_pagemap_addr *addr = &addrs[j];
>
> if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
> /*
> @@ -1299,6 +1301,18 @@ static void __drm_gpusvm_free_pages(struct drm_gpusvm *gpusvm,
> {
> lockdep_assert_held(&gpusvm->notifier_lock);
>
> + if (svm_pages->flags.inline_dma_mapping) {
> + struct drm_gpusvm_pages_flags flags = {
> + .__flags = svm_pages->flags.__flags,
> + };
> +
> + svm_pages->inline_addr = (struct drm_pagemap_addr){};
> + flags.inline_dma_mapping = false;
> + /* WRITE_ONCE pairs with READ_ONCE for opportunistic checks */
> + WRITE_ONCE(svm_pages->flags.__flags, flags.__flags);
> + return;
> + }
> +
> if (svm_pages->dma_addr) {
> kvfree(svm_pages->dma_addr);
> svm_pages->dma_addr = NULL;
> @@ -1463,11 +1477,6 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
> bool pages_valid = true;
> unsigned int p;
>
> - for (p = 0; p < num_pages; ++p) {
> - if (!svm_pages[p].dma_addr)
> - return false;
> - }
> -
> drm_gpusvm_notifier_lock(gpusvm);
> for (p = 0; p < num_pages; ++p) {
> if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
> @@ -1480,6 +1489,21 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
> return pages_valid;
> }
>
> +/**
> + * drm_gpusvm_pages_inlinable() - Whether the dma address can be inlined
> + * @nentries: Number of entries the mapping loop produced
> + *
> + * A THP maps as one huge page, so the whole range needs a single device
> + * address: the dma_addr array can be freed and the address kept inline,
> + * which is where the memory saving comes from.
> + *
> + * Return: True if the mapping fits in a single drm_pagemap_addr.
> + */
> +static bool drm_gpusvm_pages_inlinable(unsigned long nentries)
> +{
> + return nentries == 1;
> +}
> +
> /**
> * drm_gpusvm_dma_map_pages() - DMA map one drm_gpusvm_pages instance
> * @gpusvm: Pointer to the GPU SVM structure
> @@ -1632,6 +1656,14 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
> if (pagemap)
> flags.has_devmem_pages = true;
>
> + if (drm_gpusvm_pages_inlinable(j)) {
> + struct drm_pagemap_addr addr = svm_pages->dma_addr[0];
> +
> + kvfree(svm_pages->dma_addr);
> + svm_pages->inline_addr = addr;
> + flags.inline_dma_mapping = true;
> + }
> +
> /* WRITE_ONCE pairs with READ_ONCE for opportunistic checks */
> WRITE_ONCE(svm_pages->flags.__flags, flags.__flags);
>
> @@ -1740,7 +1772,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>
> if (map_dma) {
> for (p = 0; p < num_pages; ++p) {
> - if (svm_pages[p].dma_addr)
> + if (drm_gpusvm_pages_first_dma(&svm_pages[p]))
> continue;
> svm_pages[p].dma_addr =
> kvzalloc_objs(*svm_pages[p].dma_addr, npages);
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 5d990c1c374..fa4b29da0b6 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -831,7 +831,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->pages.dma_addr, 0,
> + xe_res_first_dma(xe_svm_range_first_dma(range), 0,
> xe_svm_range_size(range),
> &curs);
> xe_svm_range_debug(range, "BIND PREPARE - MIXED");
> @@ -866,8 +866,9 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
>
> if (!xe_vma_is_null(vma) && !range && !is_purged) {
> if (xe_vma_is_userptr(vma))
> - xe_res_first_dma(to_userptr_vma(vma)->userptr.pages.dma_addr, 0,
> - xe_vma_size(vma), &curs);
> + xe_res_first_dma(drm_gpusvm_pages_first_dma
> + (&to_userptr_vma(vma)->userptr.pages),
> + 0, xe_vma_size(vma), &curs);
> else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
> xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
> xe_vma_size(vma), &curs);
> diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
> index 2a0dc0d125c..7eb80d4d6db 100644
> --- a/drivers/gpu/drm/xe/xe_svm.h
> +++ b/drivers/gpu/drm/xe/xe_svm.h
> @@ -220,6 +220,18 @@ static inline unsigned long xe_svm_range_size(struct xe_svm_range *range)
> return drm_gpusvm_range_size(&range->base);
> }
>
> +/**
> + * xe_svm_range_first_dma() - Resolve the device address array of a SVM range
> + * @range: SVM range
> + *
> + * Return: Pointer to the first device address, NULL if none is populated.
> + */
> +static inline const struct drm_pagemap_addr *
> +xe_svm_range_first_dma(struct xe_svm_range *range)
> +{
> + return drm_gpusvm_pages_first_dma(&range->pages);
> +}
> +
> void xe_svm_flush(struct xe_vm *vm);
>
> int xe_pagemap_shrinker_create(struct xe_device *xe);
> @@ -436,6 +448,12 @@ static inline bool xe_svm_range_is_removed(struct xe_svm_range *range)
> return false;
> }
>
> +static inline const struct drm_pagemap_addr *
> +xe_svm_range_first_dma(struct xe_svm_range *range)
> +{
> + return NULL;
> +}
> +
> #define xe_svm_range_has_dma_mapping(...) false
> #endif /* CONFIG_DRM_XE_GPUSVM */
>
> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> index ec7b81957b1..aaad5c9b510 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -10,6 +10,7 @@
> #include <linux/kref.h>
> #include <linux/interval_tree.h>
> #include <linux/mmu_notifier.h>
> +#include <drm/drm_pagemap.h>
>
> struct dev_pagemap_ops;
> struct drm_device;
> @@ -18,7 +19,6 @@ struct drm_gpusvm_notifier;
> struct drm_gpusvm_ops;
> struct drm_gpusvm_range;
> struct drm_pagemap;
> -struct drm_pagemap_addr;
>
> /**
> * struct drm_gpusvm_ops - Operations structure for GPU SVM
> @@ -112,6 +112,7 @@ struct drm_gpusvm_notifier {
> * @unmapped: Flag indicating if the pages has been unmapped
> * @has_devmem_pages: Flag indicating if the pages has devmem pages
> * @has_dma_mapping: Flag indicating if the pages has a DMA mapping
> + * @inline_dma_mapping: Flag indicating if the pages have an inline DMA mapping
> * @__flags: Flags for pages in u16 form (used for READ_ONCE)
> */
> struct drm_gpusvm_pages_flags {
> @@ -121,6 +122,7 @@ struct drm_gpusvm_pages_flags {
> u16 unmapped : 1;
> u16 has_devmem_pages : 1;
> u16 has_dma_mapping : 1;
> + u16 inline_dma_mapping : 1;
> };
> u16 __flags;
> };
> @@ -130,17 +132,27 @@ 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
> + * @dma_addr: Device address array, valid while @flags.inline_dma_mapping is
> + * not set
> + * @inline_addr: Device address inline address, valid while
> + * @flags.inline_dma_mapping is set
> * @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.
> * @state: DMA IOVA state for mapping.
> * @state_offset: DMA IOVA offset for mapping.
> * @notifier_seq: Notifier sequence number of the range's pages
> * @flags: Flags for the range; see &struct drm_gpusvm_pages_flags
> + *
> + * @dma_addr and @inline_addr share storage, discriminated by
> + * @flags.inline_dma_mapping. Driver should use drm_gpusvm_pages_first_dma()
> + * to access the correct DMA address.
> */
> struct drm_gpusvm_pages {
> struct drm_device *drm;
> - struct drm_pagemap_addr *dma_addr;
> + union {
> + struct drm_pagemap_addr *dma_addr;
> + struct drm_pagemap_addr inline_addr;
> + };
> struct drm_pagemap *dpagemap;
> struct dma_iova_state state;
> unsigned long state_offset;
> @@ -365,6 +377,38 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
> svm_pages->notifier_seq = LONG_MAX;
> }
>
> +/**
> + * drm_gpusvm_pages_first_dma() - Resolve the device address array
> + * @svm_pages: Pointer to the drm_gpusvm_pages.
> + *
> + * drm_gpusvm_pages use unions to optimize the storage of DMA addresses,
> + * this function abstracts the access to the first device address. The driver
> + * should use this helper instead of reading dma_addr directly to prevent
> + * array out of bounds access.
> + *
> + * Only get_pages() and the free path switch between the two union members.
> + * Both hold the notifier lock for read, so taking that lock does not stop
> + * them; callers need the driver lock that does, which every reader of the
> + * addresses holds anyway. The notifier never touches the union, so the
> + * pointer returned here stays good and can then be used under the notifier
> + * lock.
> + *
> + * Return: Pointer to the first device address, NULL if none is populated.
> + */
> +static inline const struct drm_pagemap_addr *
> +drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages)
> +{
> + struct drm_gpusvm_pages_flags flags = {
> + /* READ_ONCE pairs with the WRITE_ONCE of the flag writers */
> + .__flags = READ_ONCE(svm_pages->flags.__flags),
> + };
> +
> + if (flags.inline_dma_mapping)
> + return &svm_pages->inline_addr;
> +
> + return READ_ONCE(svm_pages->dma_addr);
> +}
> +
> /**
> * enum drm_gpusvm_scan_result - Scan result from the drm_gpusvm_scan_mm() function.
> * @DRM_GPUSVM_SCAN_UNPOPULATED: At least one page was not present or inaccessible.
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline
2026-09-05 13:31 ` [PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline Honglei Huang
@ 2026-09-08 3:18 ` Matthew Brost
2026-09-08 3:49 ` Matthew Brost
0 siblings, 1 reply; 11+ messages in thread
From: Matthew Brost @ 2026-09-08 3:18 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, himal.prasad.ghimiray, dakr,
intel-xe, dri-devel, aliceryhl, Alexander.Deucher, Felix.Kuehling,
Christian.Koenig, Ray.Huang, Junhua.Shen, amd-gfx
On Sat, Sep 05, 2026 at 09:31:42PM +0800, Honglei Huang wrote:
> dma_iova_try_alloc() reserves one contiguous IOVA for the whole range and
> links each page at the next offset, so the device addresses run
> contiguously from entry 0 and one entry describes them all. A 2 MiB range
> of 4 KiB pages then drops the same 8 KiB array as a THP backed one.
>
> Fold only when state_offset covers the full range, which proves no device
> page was mapped in between, and only single page entries, so the order
> kept is 0 and stays true. Widening it instead would tell a consumer to use
> a huge page for npages separate CPU pages, which hangs Vega20 on amdgpu.
>
> The kept entry no longer bounds the segment, so skip the unmap walk when
> it has nothing to do, keyed off dpagemap rather than the flags, which are
> not published yet on the error unwind. Consumers need the same
> distinction, so drm_gpusvm_pages_first_dma() returns it alongside the
> array from one read of the flags; xe passes it to xe_res_first_dma().
>
Nice trick for IOVA allocs and variabled sized mappings like userptr.
Looks good.
Going to megre entire series to drm-misc-next shortly. Thanks!
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 41 +++++++++++++++++++++++-------
> drivers/gpu/drm/xe/xe_pt.c | 23 +++++++++++------
> drivers/gpu/drm/xe/xe_res_cursor.h | 5 ++--
> drivers/gpu/drm/xe/xe_svm.h | 8 +++---
> include/drm/drm_gpusvm.h | 13 +++++++++-
> 5 files changed, 67 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 2c7c4c89dc4..b6c9d3a07dc 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1242,7 +1242,7 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> .__flags = svm_pages->flags.__flags,
> };
> const struct drm_pagemap_addr *addrs =
> - drm_gpusvm_pages_first_dma(svm_pages);
> + drm_gpusvm_pages_first_dma(svm_pages, NULL);
> bool use_iova = dma_use_iova(&svm_pages->state);
>
> /*
> @@ -1259,7 +1259,15 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> dma_iova_free(dev, &svm_pages->state);
> }
>
> - for (i = 0, j = 0; i < npages; j++) {
> + /*
> + * With IOVA and no device page the unlink above tore every
> + * entry down, and that is also when the range may be folded
> + * to one entry, which must not be walked per entry. dpagemap
> + * is set before the first device_map(), so it is also right
> + * on the error path, where the flags are not published yet.
> + */
> + for (i = 0, j = 0;
> + (!use_iova || dpagemap) && i < npages; j++) {
> const struct drm_pagemap_addr *addr = &addrs[j];
>
> if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
> @@ -1491,17 +1499,32 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
>
> /**
> * drm_gpusvm_pages_inlinable() - Whether the dma address can be inlined
> + * @svm_pages: The SVM pages instance that was just mapped
> * @nentries: Number of entries the mapping loop produced
> + * @npages: Number of pages in the CPU range
> *
> - * A THP maps as one huge page, so the whole range needs a single device
> - * address: the dma_addr array can be freed and the address kept inline,
> - * which is where the memory saving comes from.
> + * A THP maps as one huge page, and an IOVA reservation links every page of
> + * the range at the next offset, so the device addresses run contiguously from
> + * entry 0. Either way one entry describes the whole range, so the dma_addr
> + * array can be freed and the address kept inline.
> + *
> + * state_offset advances only on the IOVA branch, so reaching the full range
> + * length proves no device page was mapped in between. Only single page
> + * entries fold, so the order kept is 0 and describes the range truthfully.
> + * Larger chunks, several huge pages among them, stay an array that is
> + * already short and that a consumer places with one PTE each.
> *
> * Return: True if the mapping fits in a single drm_pagemap_addr.
> */
> -static bool drm_gpusvm_pages_inlinable(unsigned long nentries)
> +static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
> + unsigned long nentries,
> + unsigned long npages)
> {
> - return nentries == 1;
> + if (nentries == 1)
> + return true;
> +
> + return nentries == npages && dma_use_iova(&svm_pages->state) &&
> + svm_pages->state_offset == npages * PAGE_SIZE;
> }
>
> /**
> @@ -1656,7 +1679,7 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
> if (pagemap)
> flags.has_devmem_pages = true;
>
> - if (drm_gpusvm_pages_inlinable(j)) {
> + if (drm_gpusvm_pages_inlinable(svm_pages, j, npages)) {
> struct drm_pagemap_addr addr = svm_pages->dma_addr[0];
>
> kvfree(svm_pages->dma_addr);
> @@ -1772,7 +1795,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>
> if (map_dma) {
> for (p = 0; p < num_pages; ++p) {
> - if (drm_gpusvm_pages_first_dma(&svm_pages[p]))
> + if (drm_gpusvm_pages_first_dma(&svm_pages[p], NULL))
> continue;
> svm_pages[p].dma_addr =
> kvzalloc_objs(*svm_pages[p].dma_addr, npages);
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index fa4b29da0b6..7fb2fe1f826 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -831,9 +831,12 @@ 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(xe_svm_range_first_dma(range), 0,
> - xe_svm_range_size(range),
> - &curs);
> + const struct drm_pagemap_addr *addr;
> + bool contiguous;
> +
> + addr = xe_svm_range_first_dma(range, &contiguous);
> + xe_res_first_dma(addr, 0, xe_svm_range_size(range),
> + contiguous, &curs);
> xe_svm_range_debug(range, "BIND PREPARE - MIXED");
> } else {
> xe_assert(xe, false);
> @@ -865,11 +868,15 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
> xe_bo_assert_held(bo);
>
> if (!xe_vma_is_null(vma) && !range && !is_purged) {
> - if (xe_vma_is_userptr(vma))
> - xe_res_first_dma(drm_gpusvm_pages_first_dma
> - (&to_userptr_vma(vma)->userptr.pages),
> - 0, xe_vma_size(vma), &curs);
> - else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
> + if (xe_vma_is_userptr(vma)) {
> + const struct drm_pagemap_addr *addr;
> + bool contiguous;
> +
> + addr = drm_gpusvm_pages_first_dma(&to_userptr_vma(vma)->userptr.pages,
> + &contiguous);
> + xe_res_first_dma(addr, 0, xe_vma_size(vma), contiguous,
> + &curs);
> + } else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
> xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
> xe_vma_size(vma), &curs);
> else
> diff --git a/drivers/gpu/drm/xe/xe_res_cursor.h b/drivers/gpu/drm/xe/xe_res_cursor.h
> index 0522caafd89..c3a037e5f34 100644
> --- a/drivers/gpu/drm/xe/xe_res_cursor.h
> +++ b/drivers/gpu/drm/xe/xe_res_cursor.h
> @@ -233,12 +233,13 @@ static inline void xe_res_first_sg(const struct sg_table *sg,
> * @dma_addr: struct drm_pagemap_addr array to walk
> * @start: Start of the range
> * @size: Size of the range
> + * @contiguous: Whether one entry describes the whole range
> * @cur: cursor object to initialize
> *
> * Start walking over the range of allocations between @start and @size.
> */
> static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
> - u64 start, u64 size,
> + u64 start, u64 size, bool contiguous,
> struct xe_res_cursor *cur)
> {
> XE_WARN_ON(!dma_addr);
> @@ -248,7 +249,7 @@ static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
> cur->node = NULL;
> cur->start = start;
> cur->remaining = size;
> - cur->dma_seg_size = PAGE_SIZE << dma_addr->order;
> + cur->dma_seg_size = contiguous ? start + size : PAGE_SIZE << dma_addr->order;
> cur->dma_start = 0;
> cur->size = 0;
> cur->dma_addr = dma_addr;
> diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
> index 7eb80d4d6db..2ef4ef026cc 100644
> --- a/drivers/gpu/drm/xe/xe_svm.h
> +++ b/drivers/gpu/drm/xe/xe_svm.h
> @@ -223,13 +223,14 @@ static inline unsigned long xe_svm_range_size(struct xe_svm_range *range)
> /**
> * xe_svm_range_first_dma() - Resolve the device address array of a SVM range
> * @range: SVM range
> + * @contiguous: Where to store whether one entry spans the whole range
> *
> * Return: Pointer to the first device address, NULL if none is populated.
> */
> static inline const struct drm_pagemap_addr *
> -xe_svm_range_first_dma(struct xe_svm_range *range)
> +xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
> {
> - return drm_gpusvm_pages_first_dma(&range->pages);
> + return drm_gpusvm_pages_first_dma(&range->pages, contiguous);
> }
>
> void xe_svm_flush(struct xe_vm *vm);
> @@ -449,8 +450,9 @@ static inline bool xe_svm_range_is_removed(struct xe_svm_range *range)
> }
>
> static inline const struct drm_pagemap_addr *
> -xe_svm_range_first_dma(struct xe_svm_range *range)
> +xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
> {
> + *contiguous = false;
> return NULL;
> }
>
> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> index aaad5c9b510..9e35584812f 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -380,12 +380,19 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
> /**
> * drm_gpusvm_pages_first_dma() - Resolve the device address array
> * @svm_pages: Pointer to the drm_gpusvm_pages.
> + * @contiguous: Where to store whether one entry spans the whole range, or NULL
> *
> * drm_gpusvm_pages use unions to optimize the storage of DMA addresses,
> * this function abstracts the access to the first device address. The driver
> * should use this helper instead of reading dma_addr directly to prevent
> * array out of bounds access.
> *
> + * @contiguous comes from the same read of the flags as the array itself, so a
> + * caller cannot see the two disagree and walk past that single entry into the
> + * fields behind it. When it is set the length comes from the range rather than
> + * from the order. The order still states what one PTE may cover: the range
> + * length for a huge page, PAGE_SIZE for an IOVA mapped range of single pages.
> + *
> * Only get_pages() and the free path switch between the two union members.
> * Both hold the notifier lock for read, so taking that lock does not stop
> * them; callers need the driver lock that does, which every reader of the
> @@ -396,13 +403,17 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
> * Return: Pointer to the first device address, NULL if none is populated.
> */
> static inline const struct drm_pagemap_addr *
> -drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages)
> +drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages,
> + bool *contiguous)
> {
> struct drm_gpusvm_pages_flags flags = {
> /* READ_ONCE pairs with the WRITE_ONCE of the flag writers */
> .__flags = READ_ONCE(svm_pages->flags.__flags),
> };
>
> + if (contiguous)
> + *contiguous = flags.inline_dma_mapping;
> +
> if (flags.inline_dma_mapping)
> return &svm_pages->inline_addr;
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline
2026-09-08 3:18 ` Matthew Brost
@ 2026-09-08 3:49 ` Matthew Brost
2026-09-08 5:40 ` Huang, Honglei
0 siblings, 1 reply; 11+ messages in thread
From: Matthew Brost @ 2026-09-08 3:49 UTC (permalink / raw)
To: Honglei Huang
Cc: sima, rodrigo.vivi, thomas.hellstrom, himal.prasad.ghimiray, dakr,
intel-xe, dri-devel, aliceryhl, Alexander.Deucher, Felix.Kuehling,
Christian.Koenig, Ray.Huang, Junhua.Shen, amd-gfx
On Mon, Sep 07, 2026 at 08:18:08PM -0700, Matthew Brost wrote:
> On Sat, Sep 05, 2026 at 09:31:42PM +0800, Honglei Huang wrote:
> > dma_iova_try_alloc() reserves one contiguous IOVA for the whole range and
> > links each page at the next offset, so the device addresses run
> > contiguously from entry 0 and one entry describes them all. A 2 MiB range
> > of 4 KiB pages then drops the same 8 KiB array as a THP backed one.
> >
> > Fold only when state_offset covers the full range, which proves no device
> > page was mapped in between, and only single page entries, so the order
> > kept is 0 and stays true. Widening it instead would tell a consumer to use
> > a huge page for npages separate CPU pages, which hangs Vega20 on amdgpu.
> >
> > The kept entry no longer bounds the segment, so skip the unmap walk when
> > it has nothing to do, keyed off dpagemap rather than the flags, which are
> > not published yet on the error unwind. Consumers need the same
> > distinction, so drm_gpusvm_pages_first_dma() returns it alongside the
> > array from one read of the flags; xe passes it to xe_res_first_dma().
> >
>
> Nice trick for IOVA allocs and variabled sized mappings like userptr.
> Looks good.
>
> Going to megre entire series to drm-misc-next shortly. Thanks!
>
So small snafu merging - this series won't cleanly apply drm-misc-next
and won't until 7.4 because of patches in Xe targeted for 7.4. The
series will cleanly apply to drm-xe-next, so if AMD is ok with this, I
can merge there. Or I can work through the (minor) conflicts and merge
this into drm-misc-next (ofc I'll need confirm with the drm-misc
maintainers that this ok too).
Matt
> > Suggested-by: Matthew Brost <matthew.brost@intel.com>
>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>
> > Signed-off-by: Honglei Huang <honghuan@amd.com>
> > ---
> > drivers/gpu/drm/drm_gpusvm.c | 41 +++++++++++++++++++++++-------
> > drivers/gpu/drm/xe/xe_pt.c | 23 +++++++++++------
> > drivers/gpu/drm/xe/xe_res_cursor.h | 5 ++--
> > drivers/gpu/drm/xe/xe_svm.h | 8 +++---
> > include/drm/drm_gpusvm.h | 13 +++++++++-
> > 5 files changed, 67 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > index 2c7c4c89dc4..b6c9d3a07dc 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > @@ -1242,7 +1242,7 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> > .__flags = svm_pages->flags.__flags,
> > };
> > const struct drm_pagemap_addr *addrs =
> > - drm_gpusvm_pages_first_dma(svm_pages);
> > + drm_gpusvm_pages_first_dma(svm_pages, NULL);
> > bool use_iova = dma_use_iova(&svm_pages->state);
> >
> > /*
> > @@ -1259,7 +1259,15 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> > dma_iova_free(dev, &svm_pages->state);
> > }
> >
> > - for (i = 0, j = 0; i < npages; j++) {
> > + /*
> > + * With IOVA and no device page the unlink above tore every
> > + * entry down, and that is also when the range may be folded
> > + * to one entry, which must not be walked per entry. dpagemap
> > + * is set before the first device_map(), so it is also right
> > + * on the error path, where the flags are not published yet.
> > + */
> > + for (i = 0, j = 0;
> > + (!use_iova || dpagemap) && i < npages; j++) {
> > const struct drm_pagemap_addr *addr = &addrs[j];
> >
> > if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
> > @@ -1491,17 +1499,32 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
> >
> > /**
> > * drm_gpusvm_pages_inlinable() - Whether the dma address can be inlined
> > + * @svm_pages: The SVM pages instance that was just mapped
> > * @nentries: Number of entries the mapping loop produced
> > + * @npages: Number of pages in the CPU range
> > *
> > - * A THP maps as one huge page, so the whole range needs a single device
> > - * address: the dma_addr array can be freed and the address kept inline,
> > - * which is where the memory saving comes from.
> > + * A THP maps as one huge page, and an IOVA reservation links every page of
> > + * the range at the next offset, so the device addresses run contiguously from
> > + * entry 0. Either way one entry describes the whole range, so the dma_addr
> > + * array can be freed and the address kept inline.
> > + *
> > + * state_offset advances only on the IOVA branch, so reaching the full range
> > + * length proves no device page was mapped in between. Only single page
> > + * entries fold, so the order kept is 0 and describes the range truthfully.
> > + * Larger chunks, several huge pages among them, stay an array that is
> > + * already short and that a consumer places with one PTE each.
> > *
> > * Return: True if the mapping fits in a single drm_pagemap_addr.
> > */
> > -static bool drm_gpusvm_pages_inlinable(unsigned long nentries)
> > +static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
> > + unsigned long nentries,
> > + unsigned long npages)
> > {
> > - return nentries == 1;
> > + if (nentries == 1)
> > + return true;
> > +
> > + return nentries == npages && dma_use_iova(&svm_pages->state) &&
> > + svm_pages->state_offset == npages * PAGE_SIZE;
> > }
> >
> > /**
> > @@ -1656,7 +1679,7 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
> > if (pagemap)
> > flags.has_devmem_pages = true;
> >
> > - if (drm_gpusvm_pages_inlinable(j)) {
> > + if (drm_gpusvm_pages_inlinable(svm_pages, j, npages)) {
> > struct drm_pagemap_addr addr = svm_pages->dma_addr[0];
> >
> > kvfree(svm_pages->dma_addr);
> > @@ -1772,7 +1795,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> >
> > if (map_dma) {
> > for (p = 0; p < num_pages; ++p) {
> > - if (drm_gpusvm_pages_first_dma(&svm_pages[p]))
> > + if (drm_gpusvm_pages_first_dma(&svm_pages[p], NULL))
> > continue;
> > svm_pages[p].dma_addr =
> > kvzalloc_objs(*svm_pages[p].dma_addr, npages);
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index fa4b29da0b6..7fb2fe1f826 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -831,9 +831,12 @@ 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(xe_svm_range_first_dma(range), 0,
> > - xe_svm_range_size(range),
> > - &curs);
> > + const struct drm_pagemap_addr *addr;
> > + bool contiguous;
> > +
> > + addr = xe_svm_range_first_dma(range, &contiguous);
> > + xe_res_first_dma(addr, 0, xe_svm_range_size(range),
> > + contiguous, &curs);
> > xe_svm_range_debug(range, "BIND PREPARE - MIXED");
> > } else {
> > xe_assert(xe, false);
> > @@ -865,11 +868,15 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
> > xe_bo_assert_held(bo);
> >
> > if (!xe_vma_is_null(vma) && !range && !is_purged) {
> > - if (xe_vma_is_userptr(vma))
> > - xe_res_first_dma(drm_gpusvm_pages_first_dma
> > - (&to_userptr_vma(vma)->userptr.pages),
> > - 0, xe_vma_size(vma), &curs);
> > - else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
> > + if (xe_vma_is_userptr(vma)) {
> > + const struct drm_pagemap_addr *addr;
> > + bool contiguous;
> > +
> > + addr = drm_gpusvm_pages_first_dma(&to_userptr_vma(vma)->userptr.pages,
> > + &contiguous);
> > + xe_res_first_dma(addr, 0, xe_vma_size(vma), contiguous,
> > + &curs);
> > + } else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
> > xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
> > xe_vma_size(vma), &curs);
> > else
> > diff --git a/drivers/gpu/drm/xe/xe_res_cursor.h b/drivers/gpu/drm/xe/xe_res_cursor.h
> > index 0522caafd89..c3a037e5f34 100644
> > --- a/drivers/gpu/drm/xe/xe_res_cursor.h
> > +++ b/drivers/gpu/drm/xe/xe_res_cursor.h
> > @@ -233,12 +233,13 @@ static inline void xe_res_first_sg(const struct sg_table *sg,
> > * @dma_addr: struct drm_pagemap_addr array to walk
> > * @start: Start of the range
> > * @size: Size of the range
> > + * @contiguous: Whether one entry describes the whole range
> > * @cur: cursor object to initialize
> > *
> > * Start walking over the range of allocations between @start and @size.
> > */
> > static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
> > - u64 start, u64 size,
> > + u64 start, u64 size, bool contiguous,
> > struct xe_res_cursor *cur)
> > {
> > XE_WARN_ON(!dma_addr);
> > @@ -248,7 +249,7 @@ static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
> > cur->node = NULL;
> > cur->start = start;
> > cur->remaining = size;
> > - cur->dma_seg_size = PAGE_SIZE << dma_addr->order;
> > + cur->dma_seg_size = contiguous ? start + size : PAGE_SIZE << dma_addr->order;
> > cur->dma_start = 0;
> > cur->size = 0;
> > cur->dma_addr = dma_addr;
> > diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
> > index 7eb80d4d6db..2ef4ef026cc 100644
> > --- a/drivers/gpu/drm/xe/xe_svm.h
> > +++ b/drivers/gpu/drm/xe/xe_svm.h
> > @@ -223,13 +223,14 @@ static inline unsigned long xe_svm_range_size(struct xe_svm_range *range)
> > /**
> > * xe_svm_range_first_dma() - Resolve the device address array of a SVM range
> > * @range: SVM range
> > + * @contiguous: Where to store whether one entry spans the whole range
> > *
> > * Return: Pointer to the first device address, NULL if none is populated.
> > */
> > static inline const struct drm_pagemap_addr *
> > -xe_svm_range_first_dma(struct xe_svm_range *range)
> > +xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
> > {
> > - return drm_gpusvm_pages_first_dma(&range->pages);
> > + return drm_gpusvm_pages_first_dma(&range->pages, contiguous);
> > }
> >
> > void xe_svm_flush(struct xe_vm *vm);
> > @@ -449,8 +450,9 @@ static inline bool xe_svm_range_is_removed(struct xe_svm_range *range)
> > }
> >
> > static inline const struct drm_pagemap_addr *
> > -xe_svm_range_first_dma(struct xe_svm_range *range)
> > +xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
> > {
> > + *contiguous = false;
> > return NULL;
> > }
> >
> > diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> > index aaad5c9b510..9e35584812f 100644
> > --- a/include/drm/drm_gpusvm.h
> > +++ b/include/drm/drm_gpusvm.h
> > @@ -380,12 +380,19 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
> > /**
> > * drm_gpusvm_pages_first_dma() - Resolve the device address array
> > * @svm_pages: Pointer to the drm_gpusvm_pages.
> > + * @contiguous: Where to store whether one entry spans the whole range, or NULL
> > *
> > * drm_gpusvm_pages use unions to optimize the storage of DMA addresses,
> > * this function abstracts the access to the first device address. The driver
> > * should use this helper instead of reading dma_addr directly to prevent
> > * array out of bounds access.
> > *
> > + * @contiguous comes from the same read of the flags as the array itself, so a
> > + * caller cannot see the two disagree and walk past that single entry into the
> > + * fields behind it. When it is set the length comes from the range rather than
> > + * from the order. The order still states what one PTE may cover: the range
> > + * length for a huge page, PAGE_SIZE for an IOVA mapped range of single pages.
> > + *
> > * Only get_pages() and the free path switch between the two union members.
> > * Both hold the notifier lock for read, so taking that lock does not stop
> > * them; callers need the driver lock that does, which every reader of the
> > @@ -396,13 +403,17 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
> > * Return: Pointer to the first device address, NULL if none is populated.
> > */
> > static inline const struct drm_pagemap_addr *
> > -drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages)
> > +drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages,
> > + bool *contiguous)
> > {
> > struct drm_gpusvm_pages_flags flags = {
> > /* READ_ONCE pairs with the WRITE_ONCE of the flag writers */
> > .__flags = READ_ONCE(svm_pages->flags.__flags),
> > };
> >
> > + if (contiguous)
> > + *contiguous = flags.inline_dma_mapping;
> > +
> > if (flags.inline_dma_mapping)
> > return &svm_pages->inline_addr;
> >
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline
2026-09-08 3:49 ` Matthew Brost
@ 2026-09-08 5:40 ` Huang, Honglei
0 siblings, 0 replies; 11+ messages in thread
From: Huang, Honglei @ 2026-09-08 5:40 UTC (permalink / raw)
To: Matthew Brost
Cc: sima, rodrigo.vivi, thomas.hellstrom, himal.prasad.ghimiray, dakr,
intel-xe, dri-devel, aliceryhl, Alexander.Deucher, Felix.Kuehling,
Christian.Koenig, Ray.Huang, Junhua.Shen, amd-gfx
On 9/8/2026 11:49 AM, Matthew Brost wrote:
> On Mon, Sep 07, 2026 at 08:18:08PM -0700, Matthew Brost wrote:
>> On Sat, Sep 05, 2026 at 09:31:42PM +0800, Honglei Huang wrote:
>>> dma_iova_try_alloc() reserves one contiguous IOVA for the whole range and
>>> links each page at the next offset, so the device addresses run
>>> contiguously from entry 0 and one entry describes them all. A 2 MiB range
>>> of 4 KiB pages then drops the same 8 KiB array as a THP backed one.
>>>
>>> Fold only when state_offset covers the full range, which proves no device
>>> page was mapped in between, and only single page entries, so the order
>>> kept is 0 and stays true. Widening it instead would tell a consumer to use
>>> a huge page for npages separate CPU pages, which hangs Vega20 on amdgpu.
>>>
>>> The kept entry no longer bounds the segment, so skip the unmap walk when
>>> it has nothing to do, keyed off dpagemap rather than the flags, which are
>>> not published yet on the error unwind. Consumers need the same
>>> distinction, so drm_gpusvm_pages_first_dma() returns it alongside the
>>> array from one read of the flags; xe passes it to xe_res_first_dma().
>>>
>>
>> Nice trick for IOVA allocs and variabled sized mappings like userptr.
>> Looks good.
>>
>> Going to megre entire series to drm-misc-next shortly. Thanks!
>>
>
> So small snafu merging - this series won't cleanly apply drm-misc-next
> and won't until 7.4 because of patches in Xe targeted for 7.4. The
> series will cleanly apply to drm-xe-next, so if AMD is ok with this, I
> can merge there. Or I can work through the (minor) conflicts and merge
> this into drm-misc-next (ofc I'll need confirm with the drm-misc
> maintainers that this ok too).
Merging to drm-xe-next is fine for us. Please proceed with the merge there.
And thanks for the review and merging!
Regards,
Honglei
>
> Matt
>
>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>
>> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>>
>>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>>> ---
>>> drivers/gpu/drm/drm_gpusvm.c | 41 +++++++++++++++++++++++-------
>>> drivers/gpu/drm/xe/xe_pt.c | 23 +++++++++++------
>>> drivers/gpu/drm/xe/xe_res_cursor.h | 5 ++--
>>> drivers/gpu/drm/xe/xe_svm.h | 8 +++---
>>> include/drm/drm_gpusvm.h | 13 +++++++++-
>>> 5 files changed, 67 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
>>> index 2c7c4c89dc4..b6c9d3a07dc 100644
>>> --- a/drivers/gpu/drm/drm_gpusvm.c
>>> +++ b/drivers/gpu/drm/drm_gpusvm.c
>>> @@ -1242,7 +1242,7 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
>>> .__flags = svm_pages->flags.__flags,
>>> };
>>> const struct drm_pagemap_addr *addrs =
>>> - drm_gpusvm_pages_first_dma(svm_pages);
>>> + drm_gpusvm_pages_first_dma(svm_pages, NULL);
>>> bool use_iova = dma_use_iova(&svm_pages->state);
>>>
>>> /*
>>> @@ -1259,7 +1259,15 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
>>> dma_iova_free(dev, &svm_pages->state);
>>> }
>>>
>>> - for (i = 0, j = 0; i < npages; j++) {
>>> + /*
>>> + * With IOVA and no device page the unlink above tore every
>>> + * entry down, and that is also when the range may be folded
>>> + * to one entry, which must not be walked per entry. dpagemap
>>> + * is set before the first device_map(), so it is also right
>>> + * on the error path, where the flags are not published yet.
>>> + */
>>> + for (i = 0, j = 0;
>>> + (!use_iova || dpagemap) && i < npages; j++) {
>>> const struct drm_pagemap_addr *addr = &addrs[j];
>>>
>>> if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
>>> @@ -1491,17 +1499,32 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
>>>
>>> /**
>>> * drm_gpusvm_pages_inlinable() - Whether the dma address can be inlined
>>> + * @svm_pages: The SVM pages instance that was just mapped
>>> * @nentries: Number of entries the mapping loop produced
>>> + * @npages: Number of pages in the CPU range
>>> *
>>> - * A THP maps as one huge page, so the whole range needs a single device
>>> - * address: the dma_addr array can be freed and the address kept inline,
>>> - * which is where the memory saving comes from.
>>> + * A THP maps as one huge page, and an IOVA reservation links every page of
>>> + * the range at the next offset, so the device addresses run contiguously from
>>> + * entry 0. Either way one entry describes the whole range, so the dma_addr
>>> + * array can be freed and the address kept inline.
>>> + *
>>> + * state_offset advances only on the IOVA branch, so reaching the full range
>>> + * length proves no device page was mapped in between. Only single page
>>> + * entries fold, so the order kept is 0 and describes the range truthfully.
>>> + * Larger chunks, several huge pages among them, stay an array that is
>>> + * already short and that a consumer places with one PTE each.
>>> *
>>> * Return: True if the mapping fits in a single drm_pagemap_addr.
>>> */
>>> -static bool drm_gpusvm_pages_inlinable(unsigned long nentries)
>>> +static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
>>> + unsigned long nentries,
>>> + unsigned long npages)
>>> {
>>> - return nentries == 1;
>>> + if (nentries == 1)
>>> + return true;
>>> +
>>> + return nentries == npages && dma_use_iova(&svm_pages->state) &&
>>> + svm_pages->state_offset == npages * PAGE_SIZE;
>>> }
>>>
>>> /**
>>> @@ -1656,7 +1679,7 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
>>> if (pagemap)
>>> flags.has_devmem_pages = true;
>>>
>>> - if (drm_gpusvm_pages_inlinable(j)) {
>>> + if (drm_gpusvm_pages_inlinable(svm_pages, j, npages)) {
>>> struct drm_pagemap_addr addr = svm_pages->dma_addr[0];
>>>
>>> kvfree(svm_pages->dma_addr);
>>> @@ -1772,7 +1795,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>>>
>>> if (map_dma) {
>>> for (p = 0; p < num_pages; ++p) {
>>> - if (drm_gpusvm_pages_first_dma(&svm_pages[p]))
>>> + if (drm_gpusvm_pages_first_dma(&svm_pages[p], NULL))
>>> continue;
>>> svm_pages[p].dma_addr =
>>> kvzalloc_objs(*svm_pages[p].dma_addr, npages);
>>> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
>>> index fa4b29da0b6..7fb2fe1f826 100644
>>> --- a/drivers/gpu/drm/xe/xe_pt.c
>>> +++ b/drivers/gpu/drm/xe/xe_pt.c
>>> @@ -831,9 +831,12 @@ 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(xe_svm_range_first_dma(range), 0,
>>> - xe_svm_range_size(range),
>>> - &curs);
>>> + const struct drm_pagemap_addr *addr;
>>> + bool contiguous;
>>> +
>>> + addr = xe_svm_range_first_dma(range, &contiguous);
>>> + xe_res_first_dma(addr, 0, xe_svm_range_size(range),
>>> + contiguous, &curs);
>>> xe_svm_range_debug(range, "BIND PREPARE - MIXED");
>>> } else {
>>> xe_assert(xe, false);
>>> @@ -865,11 +868,15 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
>>> xe_bo_assert_held(bo);
>>>
>>> if (!xe_vma_is_null(vma) && !range && !is_purged) {
>>> - if (xe_vma_is_userptr(vma))
>>> - xe_res_first_dma(drm_gpusvm_pages_first_dma
>>> - (&to_userptr_vma(vma)->userptr.pages),
>>> - 0, xe_vma_size(vma), &curs);
>>> - else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
>>> + if (xe_vma_is_userptr(vma)) {
>>> + const struct drm_pagemap_addr *addr;
>>> + bool contiguous;
>>> +
>>> + addr = drm_gpusvm_pages_first_dma(&to_userptr_vma(vma)->userptr.pages,
>>> + &contiguous);
>>> + xe_res_first_dma(addr, 0, xe_vma_size(vma), contiguous,
>>> + &curs);
>>> + } else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
>>> xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
>>> xe_vma_size(vma), &curs);
>>> else
>>> diff --git a/drivers/gpu/drm/xe/xe_res_cursor.h b/drivers/gpu/drm/xe/xe_res_cursor.h
>>> index 0522caafd89..c3a037e5f34 100644
>>> --- a/drivers/gpu/drm/xe/xe_res_cursor.h
>>> +++ b/drivers/gpu/drm/xe/xe_res_cursor.h
>>> @@ -233,12 +233,13 @@ static inline void xe_res_first_sg(const struct sg_table *sg,
>>> * @dma_addr: struct drm_pagemap_addr array to walk
>>> * @start: Start of the range
>>> * @size: Size of the range
>>> + * @contiguous: Whether one entry describes the whole range
>>> * @cur: cursor object to initialize
>>> *
>>> * Start walking over the range of allocations between @start and @size.
>>> */
>>> static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
>>> - u64 start, u64 size,
>>> + u64 start, u64 size, bool contiguous,
>>> struct xe_res_cursor *cur)
>>> {
>>> XE_WARN_ON(!dma_addr);
>>> @@ -248,7 +249,7 @@ static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
>>> cur->node = NULL;
>>> cur->start = start;
>>> cur->remaining = size;
>>> - cur->dma_seg_size = PAGE_SIZE << dma_addr->order;
>>> + cur->dma_seg_size = contiguous ? start + size : PAGE_SIZE << dma_addr->order;
>>> cur->dma_start = 0;
>>> cur->size = 0;
>>> cur->dma_addr = dma_addr;
>>> diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
>>> index 7eb80d4d6db..2ef4ef026cc 100644
>>> --- a/drivers/gpu/drm/xe/xe_svm.h
>>> +++ b/drivers/gpu/drm/xe/xe_svm.h
>>> @@ -223,13 +223,14 @@ static inline unsigned long xe_svm_range_size(struct xe_svm_range *range)
>>> /**
>>> * xe_svm_range_first_dma() - Resolve the device address array of a SVM range
>>> * @range: SVM range
>>> + * @contiguous: Where to store whether one entry spans the whole range
>>> *
>>> * Return: Pointer to the first device address, NULL if none is populated.
>>> */
>>> static inline const struct drm_pagemap_addr *
>>> -xe_svm_range_first_dma(struct xe_svm_range *range)
>>> +xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
>>> {
>>> - return drm_gpusvm_pages_first_dma(&range->pages);
>>> + return drm_gpusvm_pages_first_dma(&range->pages, contiguous);
>>> }
>>>
>>> void xe_svm_flush(struct xe_vm *vm);
>>> @@ -449,8 +450,9 @@ static inline bool xe_svm_range_is_removed(struct xe_svm_range *range)
>>> }
>>>
>>> static inline const struct drm_pagemap_addr *
>>> -xe_svm_range_first_dma(struct xe_svm_range *range)
>>> +xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
>>> {
>>> + *contiguous = false;
>>> return NULL;
>>> }
>>>
>>> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
>>> index aaad5c9b510..9e35584812f 100644
>>> --- a/include/drm/drm_gpusvm.h
>>> +++ b/include/drm/drm_gpusvm.h
>>> @@ -380,12 +380,19 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
>>> /**
>>> * drm_gpusvm_pages_first_dma() - Resolve the device address array
>>> * @svm_pages: Pointer to the drm_gpusvm_pages.
>>> + * @contiguous: Where to store whether one entry spans the whole range, or NULL
>>> *
>>> * drm_gpusvm_pages use unions to optimize the storage of DMA addresses,
>>> * this function abstracts the access to the first device address. The driver
>>> * should use this helper instead of reading dma_addr directly to prevent
>>> * array out of bounds access.
>>> *
>>> + * @contiguous comes from the same read of the flags as the array itself, so a
>>> + * caller cannot see the two disagree and walk past that single entry into the
>>> + * fields behind it. When it is set the length comes from the range rather than
>>> + * from the order. The order still states what one PTE may cover: the range
>>> + * length for a huge page, PAGE_SIZE for an IOVA mapped range of single pages.
>>> + *
>>> * Only get_pages() and the free path switch between the two union members.
>>> * Both hold the notifier lock for read, so taking that lock does not stop
>>> * them; callers need the driver lock that does, which every reader of the
>>> @@ -396,13 +403,17 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
>>> * Return: Pointer to the first device address, NULL if none is populated.
>>> */
>>> static inline const struct drm_pagemap_addr *
>>> -drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages)
>>> +drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages,
>>> + bool *contiguous)
>>> {
>>> struct drm_gpusvm_pages_flags flags = {
>>> /* READ_ONCE pairs with the WRITE_ONCE of the flag writers */
>>> .__flags = READ_ONCE(svm_pages->flags.__flags),
>>> };
>>>
>>> + if (contiguous)
>>> + *contiguous = flags.inline_dma_mapping;
>>> +
>>> if (flags.inline_dma_mapping)
>>> return &svm_pages->inline_addr;
>>>
>>> --
>>> 2.34.1
>>>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-09-08 5:40 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-05 13:31 [PATCH v4 0/6] drm/gpusvm: share one HMM fault and keep single mappings inline Honglei Huang
2026-09-05 13:31 ` [PATCH v4 1/6] drm/gpusvm: move dma_addr allocation before the notifier lock Honglei Huang
2026-09-05 13:31 ` [PATCH v4 2/6] drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper Honglei Huang
2026-09-05 13:31 ` [PATCH v4 3/6] drm/gpusvm: let drm_gpusvm_get_pages() map an array of pages Honglei Huang
2026-09-05 13:31 ` [PATCH v4 4/6] drm/gpusvm: make the DMA mapping step in get_pages() optional Honglei Huang
2026-09-05 13:31 ` [PATCH v4 5/6] drm/gpusvm: keep a single DMA mapping inline for THP Honglei Huang
2026-09-08 3:12 ` Matthew Brost
2026-09-05 13:31 ` [PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline Honglei Huang
2026-09-08 3:18 ` Matthew Brost
2026-09-08 3:49 ` Matthew Brost
2026-09-08 5:40 ` Huang, Honglei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox