Re: [PATCH v3 2/4] drm/gpusvm: Add devmem callback to get_pages

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Sep 22, 2026 at 10:08:02AM +0530, Himal Prasad Ghimiray wrote:
> Add an optional drm_gpusvm_ctx.devmem_fn, invoked for each device-memory
> allocation backing the range's faulted pages. It runs under the notifier
> lock and independently of the DMA map, so it also fires with no_dma_map.
> Adjacent pages of one allocation are coalesced, so the callback must be
> idempotent.
> 
> v2
> Reset last to NULL for non-devmem pages(Sashiko/Matt)
> 
> v3
> Run drm_gpusvm_pages_valid_unlocked() on the map_dma path even when
> devmem_fn is set, so an invalidated range's stale inline mapping is
> reset.
> 
> Suggested-by: Matthew Brost <matthew.brost@intel.com>

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

> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/drm_gpusvm.c | 105 ++++++++++++++++++++++++++---------
>  include/drm/drm_gpusvm.h     |   6 ++
>  2 files changed, 86 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index b6c9d3a07dc8..a37b26881b73 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1528,7 +1528,49 @@ static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
>  }
>  
>  /**
> - * drm_gpusvm_dma_map_pages() - DMA map one drm_gpusvm_pages instance
> + * drm_gpusvm_walk_devmem() - Invoke the devmem callback across faulted pages
> + * @gpusvm: Pointer to the GPU SVM structure
> + * @pfns: The already-faulted pfn array (size @npages)
> + * @npages: Number of pages in the CPU range
> + * @ctx: GPU SVM context, with a non-NULL &drm_gpusvm_ctx.devmem_fn
> + *
> + * Invoke &drm_gpusvm_ctx.devmem_fn once per contiguous run of @pfns backed by
> + * the same device-memory allocation. Must be called under the notifier lock.
> + */
> +static void drm_gpusvm_walk_devmem(struct drm_gpusvm *gpusvm,
> +				   unsigned long *pfns,
> +				   unsigned long npages,
> +				   const struct drm_gpusvm_ctx *ctx)
> +{
> +	struct drm_pagemap_devmem *last = NULL;
> +	unsigned int order = 0;
> +	unsigned long i;
> +
> +	lockdep_assert_held(&gpusvm->notifier_lock);
> +
> +	for (i = 0; i < npages; i += 1 << order) {
> +		struct page *page = hmm_pfn_to_page(pfns[i]);
> +		struct drm_pagemap_devmem *devmem;
> +
> +		order = drm_gpusvm_hmm_pfn_to_order(pfns[i], i, npages);
> +
> +		if (!is_device_private_page(page) &&
> +		    !is_device_coherent_page(page)) {
> +			last = NULL;
> +			continue;
> +		}
> +
> +		devmem = drm_pagemap_page_to_devmem(page);
> +		if (devmem == last)
> +			continue;
> +
> +		last = devmem;
> +		ctx->devmem_fn(devmem);
> +	}
> +}
> +
> +/**
> + * drm_gpusvm_dma_map_pages() - Walk and DMA map one drm_gpusvm_pages instance
>   * @gpusvm: Pointer to the GPU SVM structure
>   * @svm_pages: The SVM pages instance to populate with dma-addresses
>   * @pfns: The already-faulted pfn array (size @npages)
> @@ -1536,10 +1578,10 @@ static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
>   * @ctx: GPU SVM context
>   * @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 and only for an instance
> - * without a live mapping. On failure this unwinds the partial mapping of this
> - * instance before returning.
> + * Walk the faulted @pfns and map them into @svm_pages for DMA access through
> + * its owning 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.
>   */
> @@ -1774,9 +1816,20 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>  
>  	hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
>  
> -	if (map_dma &&
> -	    drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages, num_pages))
> -		goto set_seqno;
> +	/*
> +	 * drm_gpusvm_pages_valid_unlocked() also resets any instance whose
> +	 * mapping was invalidated, so it must run on the map_dma path even when
> +	 * devmem_fn suppresses the fast path; otherwise a stale inline mapping
> +	 * is later mistaken for a dma_addr array by drm_gpusvm_dma_map_pages().
> +	 */
> +	if (map_dma) {
> +		bool pages_valid = drm_gpusvm_pages_valid_unlocked(gpusvm,
> +								   svm_pages,
> +								   num_pages);
> +
> +		if (!ctx->devmem_fn && pages_valid)
> +			goto set_seqno;
> +	}
>  
>  	pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
>  	if (!pfns)
> @@ -1829,28 +1882,30 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>  		goto retry;
>  	}
>  
> -	if (!map_dma)
> -		goto done_mapping;
> +	if (ctx->devmem_fn)
> +		drm_gpusvm_walk_devmem(gpusvm, pfns, npages, ctx);
>  
> -	for (p = 0; p < num_pages; ++p) {
> -		if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
> -			continue;
> +	if (map_dma) {
> +		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;
> +			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;
> +			}
>  		}
>  	}
>  
> -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 9e35584812fd..4d31c6bba745 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -274,6 +274,11 @@ struct drm_gpusvm {
>   *              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.
> + * @devmem_fn: Optional callback invoked under the notifier lock, once per
> + *             contiguous run of pages backed by the same &drm_pagemap_devmem.
> + *             Must not sleep and must be idempotent, as a non-contiguous
> + *             allocation is reported more than once. Fires even with
> + *             @no_dma_map. May be NULL.
>   *
>   * Context that is DRM GPUSVM is operating in (i.e. user arguments).
>   */
> @@ -281,6 +286,7 @@ struct drm_gpusvm_ctx {
>  	void *device_private_page_owner;
>  	unsigned long check_pages_threshold;
>  	unsigned long timeslice_ms;
> +	void (*devmem_fn)(struct drm_pagemap_devmem *devmem);
>  	unsigned int in_notifier :1;
>  	unsigned int read_only :1;
>  	unsigned int devmem_possible :1;
> -- 
> 2.43.0
>