On Sat, Sep 05, 2026 at 12:39:53AM +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 <[email protected]>
Reviewed-by: Matthew Brost <[email protected]> > Signed-off-by: Honglei Huang <[email protected]> > --- > drivers/gpu/drm/drm_gpusvm.c | 140 ++++++++++++++++++++++++-------- > drivers/gpu/drm/xe/xe_svm.c | 2 +- > drivers/gpu/drm/xe/xe_userptr.c | 2 +- > include/drm/drm_gpusvm.h | 1 + > 4 files changed, 110 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c > index 89c3061d8ef..d8ee0e5f2ae 100644 > --- a/drivers/gpu/drm/drm_gpusvm.c > +++ b/drivers/gpu/drm/drm_gpusvm.c > @@ -80,6 +80,13 @@ > * }; > * }; > * > + * static struct drm_gpusvm_pages * > + * driver_pages(struct driver_range *drange) > + * { > + * return drange->num_pages == 1 ? &drange->inline_pages : > + * drange->pages; > + * } > + * > * In the N:1 case the driver allocates the pages array with a zeroing > * allocator (e.g. kcalloc(num_pages, ...)), initialises each entry with > * drm_gpusvm_init_pages(), and frees each entry with > @@ -89,6 +96,28 @@ > * Each drm_gpusvm_pages must be zero-initialised and initialised with > * drm_gpusvm_init_pages(), called once per entry. > * > + * The 1:1 examples below pass @num_pages == 1 and &drange->pages. In the > + * N:1 case the driver instead passes the whole array and its count, so a > + * single call faults the CPU range once and DMA maps it for every owning > + * drm_device, e.g.: > + * > + * .. code-block:: c > + * > + * // GPU fault handler: one fault, one DMA mapping per device > + * err = drm_gpusvm_get_pages(gpusvm, driver_pages(drange), > + * drange->num_pages, gpusvm->mm, > + * &range->notifier->notifier, > + * drm_gpusvm_range_start(range), > + * drm_gpusvm_range_end(range), &ctx); > + * > + * // Notifier callback: mark every instance unmapped in one call > + * drm_gpusvm_range_set_unmapped(range, driver_pages(drange), > + * drange->num_pages, mmu_range); > + * > + * The unmap and free paths stay per-instance: iterate @num_pages over > + * driver_pages(drange) and call drm_gpusvm_unmap_pages() / > + * drm_gpusvm_free_pages() for each entry. > + * > * - Operations: > * Define the interface for driver-specific GPU SVM operations such as > * range allocation, notifier allocation, and invalidations. > @@ -232,7 +261,7 @@ > * goto retry; > * } > * > - * err = drm_gpusvm_get_pages(gpusvm, &drange->pages, > + * err = drm_gpusvm_get_pages(gpusvm, &drange->pages, 1, > * gpusvm->mm, > &range->notifier->notifier, > * drm_gpusvm_range_start(range), > * drm_gpusvm_range_end(range), &ctx); > @@ -1417,25 +1446,35 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_pages_valid); > /** > * drm_gpusvm_pages_valid_unlocked() - GPU SVM pages valid unlocked > * @gpusvm: Pointer to the GPU SVM structure > - * @svm_pages: Pointer to the GPU SVM pages structure > + * @svm_pages: Array of GPU SVM pages structures > + * @num_pages: Number of drm_gpusvm_pages instances in @svm_pages > * > - * This function determines if a GPU SVM pages are valid. Expected be called > - * without holding gpusvm->notifier_lock. > + * This function determines if every GPU SVM pages instance is valid, > resetting > + * every instance which is not so that get_pages() maps it afresh. It > therefore > + * has to walk them all. Expected be called without holding > + * gpusvm->notifier_lock. > * > - * Return: True if GPU SVM pages are valid, False otherwise > + * Return: True if all GPU SVM pages are valid, False otherwise > */ > static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm, > - struct drm_gpusvm_pages *svm_pages) > + struct drm_gpusvm_pages *svm_pages, > + unsigned int num_pages) > { > - bool pages_valid; > + bool pages_valid = true; > + unsigned int p; > > - if (!svm_pages->dma_addr) > - return false; > + for (p = 0; p < num_pages; ++p) { > + if (!svm_pages[p].dma_addr) > + return false; > + } > > drm_gpusvm_notifier_lock(gpusvm); > - pages_valid = drm_gpusvm_pages_valid(gpusvm, svm_pages); > - if (!pages_valid) > - __drm_gpusvm_free_pages(gpusvm, svm_pages); > + for (p = 0; p < num_pages; ++p) { > + if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p])) > + continue; > + __drm_gpusvm_free_pages(gpusvm, &svm_pages[p]); > + pages_valid = false; > + } > drm_gpusvm_notifier_unlock(gpusvm); > > return pages_valid; > @@ -1451,8 +1490,9 @@ static bool drm_gpusvm_pages_valid_unlocked(struct > drm_gpusvm *gpusvm, > * @dma_dir: DMA data direction for the mappings > * > * Map the faulted @pfns into @svm_pages for DMA access through its owning > - * drm_device. Must be called under the notifier lock. On failure this > unwinds > - * the partial mapping of this instance before returning. > + * drm_device. Must be called under the notifier lock and only for an > instance > + * without a live mapping. On failure this unwinds the partial mapping of > this > + * instance before returning. > * > * Return: 0 on success, negative error code on failure. > */ > @@ -1475,6 +1515,9 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm > *gpusvm, > > lockdep_assert_held(&gpusvm->notifier_lock); > > + *state = (struct dma_iova_state){}; > + svm_pages->state_offset = 0; > + > flags.__flags = svm_pages->flags.__flags; > > for (i = 0, j = 0; i < npages; ++j) { > @@ -1603,20 +1646,28 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm > *gpusvm, > /** > * drm_gpusvm_get_pages() - Get pages and populate GPU SVM pages struct > * @gpusvm: Pointer to the GPU SVM structure > - * @svm_pages: The SVM pages to populate. This will contain the dma-addresses > + * @svm_pages: Array of SVM pages instances to populate with dma addresses > + * @num_pages: Number of drm_gpusvm_pages instances in @svm_pages, must not > be 0 > * @mm: The mm corresponding to the CPU range > * @notifier: The corresponding notifier for the given CPU range > * @pages_start: Start CPU address for the pages > * @pages_end: End CPU address for the pages (exclusive) > * @ctx: GPU SVM context > * > - * This function gets and maps pages for CPU range and ensures they are > - * mapped for DMA access. > + * This function gets and maps pages for a CPU range and ensures they are > + * mapped for DMA access. The HMM fault for the CPU range is performed once, > + * the DMA mapping by drm_gpusvm_dma_map_pages() is then done per instance, > + * one per owning drm_device. The retry against notifier races is kept here > + * in common code so drivers never open code it. > + * > + * On error the instances mapped before the failing one stay mapped, so the > + * caller must unmap and free every instance regardless of the return value. > * > * Return: 0 on success, negative error code on failure. > */ > int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > struct drm_gpusvm_pages *svm_pages, > + unsigned int num_pages, > struct mm_struct *mm, > struct mmu_interval_notifier *notifier, > unsigned long pages_start, unsigned long pages_end, > @@ -1638,10 +1689,15 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > int err = 0; > enum dma_data_direction dma_dir = ctx->read_only ? DMA_TO_DEVICE : > DMA_BIDIRECTIONAL; > + unsigned int p; > > - if (!svm_pages->drm) > + if (!num_pages) > return -EINVAL; > > + for (p = 0; p < num_pages; ++p) > + if (!svm_pages[p].drm) > + return -EINVAL; > + > retry: > remaining = timeout - jiffies; > > @@ -1649,7 +1705,8 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > return -EBUSY; > > hmm_range.notifier_seq = mmu_interval_read_begin(notifier); > - if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages)) > + > + if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages, num_pages)) > goto set_seqno; > > pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL); > @@ -1667,18 +1724,17 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > if (err) > goto err_free; > > - if (!svm_pages->dma_addr) { > - svm_pages->dma_addr = > - kvzalloc_objs(*svm_pages->dma_addr, npages); > - if (!svm_pages->dma_addr) { > + for (p = 0; p < num_pages; ++p) { > + if (svm_pages[p].dma_addr) > + continue; > + svm_pages[p].dma_addr = > + kvzalloc_objs(*svm_pages[p].dma_addr, npages); > + if (!svm_pages[p].dma_addr) { > err = -ENOMEM; > goto err_free; > } > } > > - svm_pages->state = (struct dma_iova_state){}; > - svm_pages->state_offset = 0; > - > /* > * Perform all dma mappings under the notifier lock to not > * access freed pages. A notifier will either block on > @@ -1686,7 +1742,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > */ > drm_gpusvm_notifier_lock(gpusvm); > > - if (svm_pages->flags.unmapped) { > + /* > + * drm_gpusvm_range_set_unmapped() flags the whole array in one go under > + * the write lock, so any instance answers for all of them here. > + */ > + if (svm_pages[0].flags.unmapped) { > drm_gpusvm_notifier_unlock(gpusvm); > err = -EFAULT; > goto err_free; > @@ -1698,15 +1758,29 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > goto retry; > } > > - err = drm_gpusvm_dma_map_pages(gpusvm, svm_pages, pfns, npages, ctx, > - dma_dir); > - drm_gpusvm_notifier_unlock(gpusvm); > - if (err) > - goto err_free; > + for (p = 0; p < num_pages; ++p) { > + if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p])) > + continue; > > + err = drm_gpusvm_dma_map_pages(gpusvm, &svm_pages[p], pfns, > + npages, ctx, dma_dir); > + if (err) { > + /* > + * The failing instance was unwound by the helper. Keep > + * the ones mapped earlier: the -EAGAIN retry reuses > + * them, and the driver unmaps every instance with the > + * range on the other error paths. > + */ > + drm_gpusvm_notifier_unlock(gpusvm); > + goto err_free; > + } > + } > + > + drm_gpusvm_notifier_unlock(gpusvm); > kvfree(pfns); > set_seqno: > - svm_pages->notifier_seq = hmm_range.notifier_seq; > + for (p = 0; p < num_pages; ++p) > + svm_pages[p].notifier_seq = hmm_range.notifier_seq; > > return 0; > > diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c > index 627a741293d..1c7793d8caa 100644 > --- a/drivers/gpu/drm/xe/xe_svm.c > +++ b/drivers/gpu/drm/xe/xe_svm.c > @@ -1598,7 +1598,7 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct > xe_svm_range *range, > > lockdep_assert_held(&range->lock); > > - err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages, > + err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages, 1, > vm->svm.gpusvm.mm, > &range->base.notifier->notifier, > drm_gpusvm_range_start(&range->base), > diff --git a/drivers/gpu/drm/xe/xe_userptr.c b/drivers/gpu/drm/xe/xe_userptr.c > index 90ac141fc12..9c1dac0fce6 100644 > --- a/drivers/gpu/drm/xe/xe_userptr.c > +++ b/drivers/gpu/drm/xe/xe_userptr.c > @@ -91,7 +91,7 @@ int xe_vma_userptr_pin_pages(struct xe_userptr_vma *uvma) > if (vma->gpuva.flags & XE_VMA_DESTROYED) > return 0; > > - return drm_gpusvm_get_pages(&vm->svm.gpusvm, &uvma->userptr.pages, > + return drm_gpusvm_get_pages(&vm->svm.gpusvm, &uvma->userptr.pages, 1, > uvma->userptr.notifier.mm, > &uvma->userptr.notifier, > xe_vma_userptr(vma), > diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h > index b7d987bf76a..d2b6f3d2b84 100644 > --- a/include/drm/drm_gpusvm.h > +++ b/include/drm/drm_gpusvm.h > @@ -324,6 +324,7 @@ void drm_gpusvm_range_set_unmapped(struct > drm_gpusvm_range *range, > > int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > struct drm_gpusvm_pages *svm_pages, > + unsigned int num_pages, > struct mm_struct *mm, > struct mmu_interval_notifier *notifier, > unsigned long pages_start, unsigned long pages_end, > -- > 2.34.1 >
