Re: [PATCH 1/2] drm/gpusvm: Add a DMA-mapping accounting callback

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

On Wed, Nov 04, 2026 at 04:49:01PM -0800, Niranjana Vishwanathapura wrote:
> On Wed, Nov 04, 2026 at 01:52:27PM -0800, Matthew Brost wrote:
> > From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > 
> > Drivers that use the GPU SVM core to DMA-map system memory for GPU
> > access may want to keep track of how many pages they have mapped, for
> > example to expose statistics or to detect leaks. Doing this accounting
> > in the driver around drm_gpusvm_get_pages() / drm_gpusvm_unmap_pages()
> > is error prone: the number, order and kind of the dma_addr[] entries can
> > change between map and unmap (migration, partial unmaps, the iova vs
> > non-iova paths), which makes symmetric accounting hard to get right.
> > 
> > Add an optional @dma_map_account callback to struct drm_gpusvm_ops and
> > invoke it once per &drm_pagemap_addr entry at the exact points where the
> > entry is DMA-mapped (sign == +1) in drm_gpusvm_get_pages() and unmapped
> > (sign == -1) in __drm_gpusvm_unmap_pages(). Since the map error path and
> > every unmap/free path funnel through __drm_gpusvm_unmap_pages() for the
> > entries that were actually mapped, the accounting is symmetric by
> > construction. The callback receives the full &drm_pagemap_addr so the
> > driver can use the order and the proto field to distinguish system
> > memory mappings from device interconnect mappings.
> > 
> > The callback must also be usable by the core drm_gpusvm_pages-only API
> > (mm == NULL), as used e.g. for userptr-style mappings. Relax
> > drm_gpusvm_init() to allow such users to pass a restricted @ops that
> > only implements the page-level hooks; the full-SVM hooks (@invalidate)
> > and the chunk configuration must still be unset in that mode.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> > ---
> > drivers/gpu/drm/drm_gpusvm.c | 40 ++++++++++++++++++++++++++++--------
> > include/drm/drm_gpusvm.h     | 19 +++++++++++++++++
> > 2 files changed, 50 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > index 03dbe304e4b8..5ef9d7233491 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > @@ -503,8 +503,15 @@ int drm_gpusvm_init(struct drm_gpusvm *gpusvm,
> > 			return -EINVAL;
> > 		mmgrab(mm);
> > 	} else {
> > -		/* No full SVM mode, only core drm_gpusvm_pages API. */
> > -		if (ops || num_chunks || mm_range || notifier_size)
> > +		/*
> > +		 * No full SVM mode, only core drm_gpusvm_pages API. A
> > +		 * restricted @ops that only implements the page-level hooks
> > +		 * (e.g. @dma_map_account) is allowed; the full-SVM hooks must
> > +		 * not be set.
> > +		 */
> > +		if (num_chunks || mm_range || notifier_size)
> > +			return -EINVAL;
> > +		if (ops && ops->invalidate)
> > 			return -EINVAL;
> 
> Other than 'invalidate' here, should we also check for other ops like
> range_alloc, range_free etc?
> 

Yes. Let me fix that.

Matt

> Niranjana
> 
> > 	}
> > 
> > @@ -1240,9 +1247,12 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> > 		struct drm_gpusvm_pages_flags flags = {
> > 			.__flags = svm_pages->flags.__flags,
> > 		};
> > +		bool contiguous;
> > 		const struct drm_pagemap_addr *addrs =
> > -			drm_gpusvm_pages_first_dma(svm_pages, NULL);
> > +			drm_gpusvm_pages_first_dma(svm_pages, &contiguous);
> > 		bool use_iova = dma_use_iova(&svm_pages->state);
> > +		bool needs_account = gpusvm->ops &&
> > +			gpusvm->ops->dma_map_account;
> > 
> > 		/*
> > 		 * IOVA is reserved for the whole range but only the linked
> > @@ -1260,14 +1270,18 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> > 
> > 		/*
> > 		 * With IOVA and no device page the unlink above tore every
> > -		 * entry down, and that is also when the range may be folded
> > -		 * to one entry, which must not be walked per entry. dpagemap
> > -		 * is set before the first device_map(), so it is also right
> > -		 * on the error path, where the flags are not published yet.
> > +		 * entry down, but the per-entry walk is still needed when the
> > +		 * driver accounts the mappings. A folded (inline) mapping has
> > +		 * a single entry describing the whole range, so it must be
> > +		 * re-read for every iteration instead of indexed by @j.
> > +		 * dpagemap is set before the first device_map(), so it is also
> > +		 * right on the error path, where the flags are not published
> > +		 * yet.
> > 		 */
> > 		for (i = 0, j = 0;
> > -		     (!use_iova || dpagemap) && i < npages; j++) {
> > -			const struct drm_pagemap_addr *addr = &addrs[j];
> > +		     (!use_iova || dpagemap || needs_account) && i < npages; j++) {
> > +			const struct drm_pagemap_addr *addr =
> > +				contiguous ? &addrs[0] : &addrs[j];
> > 
> > 			if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
> > 				/*
> > @@ -1283,6 +1297,10 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> > 			} else if (dpagemap && dpagemap->ops->device_unmap)
> > 				dpagemap->ops->device_unmap(dpagemap,
> > 							    dev, addr);
> > +
> > +			if (needs_account)
> > +				gpusvm->ops->dma_map_account(gpusvm, addr, -1);
> > +
> > 			i += 1 << addr->order;
> > 		}
> > 
> > @@ -1705,6 +1723,10 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
> > 				(addr, DRM_INTERCONNECT_SYSTEM, order,
> > 				 dma_dir);
> > 		}
> > +		if (gpusvm->ops && gpusvm->ops->dma_map_account)
> > +			gpusvm->ops->dma_map_account(gpusvm,
> > +						     &svm_pages->dma_addr[j],
> > +						     1);
> > 		i += 1 << order;
> > 		num_dma_mapped = i;
> > 		flags.has_dma_mapping = true;
> > diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> > index 4d31c6bba745..637c19f7169f 100644
> > --- a/include/drm/drm_gpusvm.h
> > +++ b/include/drm/drm_gpusvm.h
> > @@ -75,6 +75,25 @@ struct drm_gpusvm_ops {
> > 	void (*invalidate)(struct drm_gpusvm *gpusvm,
> > 			   struct drm_gpusvm_notifier *notifier,
> > 			   const struct mmu_notifier_range *mmu_range);
> > +
> > +	/**
> > +	 * @dma_map_account: Account a DMA-mapping change (optional)
> > +	 * @gpusvm: Pointer to the GPU SVM
> > +	 * @addr: The address descriptor of the chunk being (un)mapped
> > +	 * @sign: +1 when @addr has just been DMA-mapped, -1 when it is being
> > +	 *        unmapped
> > +	 *
> > +	 * Called once per &drm_pagemap_addr entry, when the entry is
> > +	 * DMA-mapped by drm_gpusvm_get_pages() (@sign == +1) and when it is
> > +	 * unmapped (@sign == -1), so the driver can keep symmetric accounting
> > +	 * of the pages it has mapped for GPU access. The @addr->proto field
> > +	 * can be used to distinguish system-memory mappings from device
> > +	 * interconnect mappings. May be provided in both full SVM mode and the
> > +	 * core drm_gpusvm_pages-only mode.
> > +	 */
> > +	void (*dma_map_account)(struct drm_gpusvm *gpusvm,
> > +				const struct drm_pagemap_addr *addr,
> > +				int sign);
> > };
> > 
> > /**
> > -- 
> > 2.34.1
> >