[PATCH v2 0/4] drm/gpusvm: share one HMM fault across per-device DMA mappings

[PATCH v2 0/4] drm/gpusvm: share one HMM fault across per-device DMA mappings

[Honglei Huang]

This series is a follow up to the earlier drm_gpusvm_pages decoupling
series [1], and follows the direction Matt suggested [2].

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.

The series builds up to that in small steps, no functional change until
the last two patches:

  - 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 4 has no in-tree user yet, AMDXDNA is the intended consumer but
that conversion is not ready. It is independent of patches 1-3, happy to
drop it and repost it together with its user.

v2:
  - Rebased on drm-tip. Matt asked for Stanislav's
    hmm_range_fault_unlocked_timeout() patch [3] to be carried at the
    base of this series, it has since landed in drm-tip, so this version
    is just rebased on top of it.
  - 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.

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 gfx943 (MI300X) and gfx906 (MI60) with XNACK on/off:
  - 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/

Honglei Huang (4):
  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

 drivers/gpu/drm/drm_gpusvm.c    | 336 +++++++++++++++++++++-----------
 drivers/gpu/drm/xe/xe_svm.c     |   2 +-
 drivers/gpu/drm/xe/xe_userptr.c |   2 +-
 include/drm/drm_gpusvm.h        |  10 +
 4 files changed, 239 insertions(+), 111 deletions(-)


base-commit: c8420bcb1802510e5411ea4c98e2e07bdb43d74e
-- 
2.34.1

[PATCH v2 1/4] drm/gpusvm: move dma_addr allocation before the notifier lock

[Honglei Huang]

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.

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

[PATCH v2 2/4] drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper

[Honglei Huang]

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>
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

[PATCH v2 3/4] drm/gpusvm: let drm_gpusvm_get_pages() map an array of pages

[Honglei Huang]

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>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
 drivers/gpu/drm/drm_gpusvm.c    | 141 ++++++++++++++++++++++++--------
 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, 108 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 89c3061d8ef..810f801a9f7 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,34 @@ 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
+ * This function determines if every GPU SVM pages instance is valid, dropping
+ * the stale dma_addr array of any instance which is not. 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 +1489,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 +1514,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 +1645,29 @@ 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
  * @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.
+ * The common 1:1 case passes @num_pages == 1.
+ *
+ * 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,9 +1689,11 @@ 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)
-		return -EINVAL;
+	for (p = 0; p < num_pages; ++p)
+		if (!svm_pages[p].drm)
+			return -EINVAL;
 
 retry:
 	remaining = timeout - jiffies;
@@ -1649,7 +1702,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 +1721,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,10 +1739,12 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 	 */
 	drm_gpusvm_notifier_lock(gpusvm);
 
-	if (svm_pages->flags.unmapped) {
-		drm_gpusvm_notifier_unlock(gpusvm);
-		err = -EFAULT;
-		goto err_free;
+	for (p = 0; p < num_pages; ++p) {
+		if (svm_pages[p].flags.unmapped) {
+			drm_gpusvm_notifier_unlock(gpusvm);
+			err = -EFAULT;
+			goto err_free;
+		}
 	}
 
 	if (mmu_interval_read_retry(notifier, hmm_range.notifier_seq)) {
@@ -1698,15 +1753,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

[PATCH v2 4/4] drm/gpusvm: make the DMA mapping step in get_pages() optional

[Honglei Huang]

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>
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 810f801a9f7..0156ee82d7c 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,11 +1695,18 @@ 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;
 
-	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;
@@ -1703,7 +1716,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);
@@ -1721,14 +1735,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;
+			}
 		}
 	}
 
@@ -1753,6 +1769,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;
@@ -1771,6 +1790,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

Re: [PATCH v2 1/4] drm/gpusvm: move dma_addr allocation before the notifier lock

[Matthew Brost]

On Tue, Sep 01, 2026 at 05:00:57PM +0800, Honglei Huang wrote:
> 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.
> 
> Signed-off-by: Honglei Huang <honghuan@amd.com>

One follow up suggestion below - not blocker for merging this series.
Feel free to implement this or at some point (we) Intel will get around
to this.

This patch is:
Reviewed-by: Matthew Brost <matthew.brost@intel.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);

One thing that isn't great about the current code is that, regardless of
`npages`, the overwhelming majority of cases result in exactly one DMA
mapping. As a result, we end up wasting a significant amount of memory.
If an IOVA is allocated, we only ever need a single DMA mapping. Even
without IOVA support (e.g., `iommu=off` or `iommu=pt`), a 2 MiB backing
store implemented as a THP would still yield a single DMA mapping via
`dma_map_page()`.

Longer term, I'd like to do something like the structure changes below.
We would still preallocate unless `npages == 1`, but if
`drm_gpusvm_dma_map_pages()` finds exactly one DMA mapping, we could free
`dma_addr`, store the `drm_pagemap_addr` in `inline_addr`, and set an
`inline_dma_mapping` flag. This would save 8 KiB per page of
`drm_gpusvm_pages` for every 2 MiB THP-backed allocation.

Then the final piece is teach drivers to understand 'inline_dma_mapping'
in their iterators (xe_res_cursor.h in Xe) to correctly walk the
dma-mapping.

Matt 

diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index b7d987bf76aa..73b7065610f0 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -121,6 +121,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;
        };
@@ -140,7 +141,10 @@ struct drm_gpusvm_pages_flags {
  */
 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;


> +		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
> 

Re: [PATCH v2 2/4] drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper

[Matthew Brost]

On Tue, Sep 01, 2026 at 05:00:58PM +0800, Honglei Huang wrote:
> 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>

For some reason, Sashiko didn't run on this particular patch [1]. It
would be good to have it run through the tooling in case I'm missing
something.

That said, I can't spot anything incorrect, and this looks like a solid
cleanup.

With that:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

[1] https://sashiko.dev/#/patchset/20260901090100.2024933-1-honghuan%40amd.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
> 

Re: [PATCH v2 3/4] drm/gpusvm: let drm_gpusvm_get_pages() map an array of pages

[Matthew Brost]

On Tue, Sep 01, 2026 at 05:00:59PM +0800, Honglei Huang wrote:
> 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>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
>  drivers/gpu/drm/drm_gpusvm.c    | 141 ++++++++++++++++++++++++--------
>  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, 108 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 89c3061d8ef..810f801a9f7 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,34 @@ 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
> + * This function determines if every GPU SVM pages instance is valid, dropping
> + * the stale dma_addr array of any instance which is not. 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 +1489,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 +1514,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 +1645,29 @@ 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
>   * @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.
> + * The common 1:1 case passes @num_pages == 1.
> + *
> + * 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,9 +1689,11 @@ 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)
> -		return -EINVAL;
> +	for (p = 0; p < num_pages; ++p)
> +		if (!svm_pages[p].drm)
> +			return -EINVAL;
>  
>  retry:
>  	remaining = timeout - jiffies;
> @@ -1649,7 +1702,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 +1721,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,10 +1739,12 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>  	 */
>  	drm_gpusvm_notifier_lock(gpusvm);
>  
> -	if (svm_pages->flags.unmapped) {
> -		drm_gpusvm_notifier_unlock(gpusvm);
> -		err = -EFAULT;
> -		goto err_free;
> +	for (p = 0; p < num_pages; ++p) {
> +		if (svm_pages[p].flags.unmapped) {
> +			drm_gpusvm_notifier_unlock(gpusvm);
> +			err = -EFAULT;
> +			goto err_free;
> +		}

I believe, given how the notifiers work, that checking
`svm_pages[0].flags.unmapped` is actually sufficient. It's a
micro-optimization, so I'm fine with it either way.
 
>  	}
>  
>  	if (mmu_interval_read_retry(notifier, hmm_range.notifier_seq)) {
> @@ -1698,15 +1753,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);

One thing that is different here is that if `drm_gpusvm_dma_map_pages()`
fails, say at `p == 1`, then `p[0]` will already contain valid DMA
mappings. I think this is actually fine, though, because the existing
cleanup paths will eventually release those mappings one way or another.
 
That said, it's probably worth confirming this through a code-path audit
and adding a comment here explaining why this is safe.
 
Again, Sashiko didn't run on this patch, and it would be good to get a
run before merging this series.

Matt

> +		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
> 

Re: [PATCH v2 4/4] drm/gpusvm: make the DMA mapping step in get_pages() optional

[Matthew Brost]

On Tue, Sep 01, 2026 at 05:01:00PM +0800, Honglei Huang wrote:
> 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 810f801a9f7..0156ee82d7c 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,11 +1695,18 @@ 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;
>  
> -	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;
> @@ -1703,7 +1716,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);
> @@ -1721,14 +1735,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;
> +			}
>  		}
>  	}
>  
> @@ -1753,6 +1769,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;
> @@ -1771,6 +1790,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
> 

Re: [PATCH v2 1/4] drm/gpusvm: move dma_addr allocation before the notifier lock

[Huang, Honglei]

On 9/2/2026 3:39 AM, Matthew Brost wrote:
> On Tue, Sep 01, 2026 at 05:00:57PM +0800, Honglei Huang wrote:
>> 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.
>>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
> 
> One follow up suggestion below - not blocker for merging this series.
> Feel free to implement this or at some point (we) Intel will get around
> to this.

Got it, you can give me RB after you are really satisfied. I can keep 
modifying until you feel there are no problems.

> 
> This patch is:
> Reviewed-by: Matthew Brost <matthew.brost@intel.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);
> 
> One thing that isn't great about the current code is that, regardless of
> `npages`, the overwhelming majority of cases result in exactly one DMA
> mapping. As a result, we end up wasting a significant amount of memory.
> If an IOVA is allocated, we only ever need a single DMA mapping. Even
> without IOVA support (e.g., `iommu=off` or `iommu=pt`), a 2 MiB backing
> store implemented as a THP would still yield a single DMA mapping via
> `dma_map_page()`.
> 
> Longer term, I'd like to do something like the structure changes below.
> We would still preallocate unless `npages == 1`, but if
> `drm_gpusvm_dma_map_pages()` finds exactly one DMA mapping, we could free
> `dma_addr`, store the `drm_pagemap_addr` in `inline_addr`, and set an
> `inline_dma_mapping` flag. This would save 8 KiB per page of
> `drm_gpusvm_pages` for every 2 MiB THP-backed allocation.

Makes sense, working on it to reduce memory waste.

> 
> Then the final piece is teach drivers to understand 'inline_dma_mapping'
> in their iterators (xe_res_cursor.h in Xe) to correctly walk the
> dma-mapping.

Agreed, the important part is driver maybe miss this change and the 
compiles still fine, maybe worth changing xe_res_first_dma() to take a 
drm_gpusvm_pages instead of a bare pointer.

> 
> Matt
> 
> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> index b7d987bf76aa..73b7065610f0 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -121,6 +121,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;
>          };
> @@ -140,7 +141,10 @@ struct drm_gpusvm_pages_flags {
>    */
>   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;

Will modify the code according to your suggestion.

Regards,
Honglei


> 
> 
>> +		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
>>

Re: [PATCH v2 2/4] drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper

[Huang, Honglei]

On 9/2/2026 3:43 AM, Matthew Brost wrote:
> On Tue, Sep 01, 2026 at 05:00:58PM +0800, Honglei Huang wrote:
>> 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>
> 
> For some reason, Sashiko didn't run on this particular patch [1]. It
> would be good to have it run through the tooling in case I'm missing
> something.
> 
> That said, I can't spot anything incorrect, and this looks like a solid
> cleanup.
> 
> With that:
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>

Will resent this series only to intel-xe@list and dri-devel@list to 
retigger the Sashiko review.

Regards,
Honglei

> 
> [1] https://sashiko.dev/#/patchset/20260901090100.2024933-1-honghuan%40amd.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
>>

Re: [PATCH v2 3/4] drm/gpusvm: let drm_gpusvm_get_pages() map an array of pages

[Huang, Honglei]

On 9/2/2026 3:57 AM, Matthew Brost wrote:
> On Tue, Sep 01, 2026 at 05:00:59PM +0800, Honglei Huang wrote:
>> 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>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>>   drivers/gpu/drm/drm_gpusvm.c    | 141 ++++++++++++++++++++++++--------
>>   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, 108 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
>> index 89c3061d8ef..810f801a9f7 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,34 @@ 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
>> + * This function determines if every GPU SVM pages instance is valid, dropping
>> + * the stale dma_addr array of any instance which is not. 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 +1489,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 +1514,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 +1645,29 @@ 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
>>    * @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.
>> + * The common 1:1 case passes @num_pages == 1.
>> + *
>> + * 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,9 +1689,11 @@ 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)
>> -		return -EINVAL;
>> +	for (p = 0; p < num_pages; ++p)
>> +		if (!svm_pages[p].drm)
>> +			return -EINVAL;
>>   
>>   retry:
>>   	remaining = timeout - jiffies;
>> @@ -1649,7 +1702,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 +1721,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,10 +1739,12 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>>   	 */
>>   	drm_gpusvm_notifier_lock(gpusvm);
>>   
>> -	if (svm_pages->flags.unmapped) {
>> -		drm_gpusvm_notifier_unlock(gpusvm);
>> -		err = -EFAULT;
>> -		goto err_free;
>> +	for (p = 0; p < num_pages; ++p) {
>> +		if (svm_pages[p].flags.unmapped) {
>> +			drm_gpusvm_notifier_unlock(gpusvm);
>> +			err = -EFAULT;
>> +			goto err_free;
>> +		}
> 
> I believe, given how the notifiers work, that checking
> `svm_pages[0].flags.unmapped` is actually sufficient. It's a
> micro-optimization, so I'm fine with it either way.

Agreed, will change to `svm_pages[0].flags.unmapped` .

>   
>>   	}
>>   
>>   	if (mmu_interval_read_retry(notifier, hmm_range.notifier_seq)) {
>> @@ -1698,15 +1753,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);
> 
> One thing that is different here is that if `drm_gpusvm_dma_map_pages()`
> fails, say at `p == 1`, then `p[0]` will already contain valid DMA
> mappings. I think this is actually fine, though, because the existing
> cleanup paths will eventually release those mappings one way or another.
>   
> That said, it's probably worth confirming this through a code-path audit
> and adding a comment here explaining why this is safe.
>   
> Again, Sashiko didn't run on this patch, and it would be good to get a
> run before merging this series.
> 

Actually, I hesitated when modifying the code whether to perform a 
rollback here. And I decided to not rollback here, driver can reuse the
previous success mapping when return -EAGAIN, and for other error code,
driver will call drm_gpusvm_unmap_pages then range_free safely.

And actullay I put some comments below, after if (err),
and int drm_gpusvm.c:1663, I put same documents for this situlation.

And I will also resent this series to trigger Sashiko review, and use AI 
to scan this part to ensure it is safe.

Regards,
Honglei

> Matt
> 
>> +		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
>>