Re: [PATCH v3 2/3] drm/gpusvm: do not route system pages to device_unmap() on IOVA unmap

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

On Tue, Jun 30, 2026 at 03:08:58PM +0800, Honglei Huang wrote:
> In a mixed range: ctx->allow_mixed dpagemap is not NULL while some entries
> are system pages. The unmap loop used:
> 
>         dma_unmap_page(...);
>     else if (dpagemap && dpagemap->ops->device_unmap)
>         dpagemap->ops->device_unmap(...);
> 
> When use_iova is true the first condition is false for system pages,
> so they fall through to device_unmap() and a system DMA address is
> handed to the device specific unmap callback, risking invalid accesses
> or state corruption.
> 
> Key the branch off addr->proto instead: system pages only need an explicit
> dma_unmap_page() in the non IOVA case, IOVA system pages are already torn
> down by the single dma_iova_destroy(), and only genuine device pages
> reach device_unmap().
> 
> This issue was found by Sashiko AI review.
> 
> Fixes: 37ad039fb367 ("drm/gpusvm: Use dma-map IOVA alloc, link, and sync API in GPU SVM")
> Cc: stable@vger.kernel.org
> Signed-off-by: Honglei Huang <honghuan@amd.com>

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

> ---
>  drivers/gpu/drm/drm_gpusvm.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 3145d55cd86..44bb19658dd 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1163,12 +1163,18 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
>  		for (i = 0, j = 0; i < npages; j++) {
>  			struct drm_pagemap_addr *addr = &svm_pages->dma_addr[j];
>  
> -			if (!use_iova && addr->proto == DRM_INTERCONNECT_SYSTEM)
> -				dma_unmap_page(dev,
> -					       addr->addr,
> -					       PAGE_SIZE << addr->order,
> -					       addr->dir);
> -			else if (dpagemap && dpagemap->ops->device_unmap)
> +			if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
> +				/*
> +				 * Linked IOVA pages were already torn down by
> +				 * the dma_iova_unlink()/dma_iova_free() above;
> +				 * only the non-IOVA mappings need unmap here.
> +				 */
> +				if (!use_iova)
> +					dma_unmap_page(dev,
> +						       addr->addr,
> +						       PAGE_SIZE << addr->order,
> +						       addr->dir);
> +			} else if (dpagemap && dpagemap->ops->device_unmap)
>  				dpagemap->ops->device_unmap(dpagemap,
>  							    dev, addr);
>  			i += 1 << addr->order;
> -- 
> 2.34.1
>