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

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

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
>