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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] IOVA address space is permanently leaked when unmapping mixed ranges because `dma_iova_try_alloc` allocates for the entire range but `dma_iova_destroy` only frees the linked portion.
--

commit 00ffc2a555c26b1b1afa96a3e7aabeee992b2163
Author: Honglei Huang <honghuan@amd.com>

drm/gpusvm: do not route system pages to device_unmap() on IOVA unmap

This commit fixes an issue in mixed ranges where system pages were incorrectly
routed to device_unmap() during IOVA unmap by checking the page protocol.

> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 2d74e2aae0a77..80b9a3f4cb919 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1161,12 +1161,17 @@ 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) {
> +				/*
> +				 * IOVA pages are cleaned by the ma_iova_destroy() above.

[Severity: High]
This isn't a bug introduced by this patch, but looking at how IOVA pages are
cleaned up here, does this leak IOVA address space for mixed ranges?

In drm_gpusvm_get_pages(), dma_iova_try_alloc() allocates IOVA space for the
entire range size:

    dma_iova_try_alloc(gpusvm->drm->dev, state, 0, npages * PAGE_SIZE);

However, when this range is unmapped in __drm_gpusvm_unmap_pages(),
dma_iova_destroy() is called with svm_pages->state_offset as the length:

    dma_iova_destroy(dev, &svm_pages->state,
                     svm_pages->state_offset,
                     npages ? svm_pages->dma_addr[0].dir :
                              DMA_BIDIRECTIONAL, 0);

If a user-space application allocates a range and migrates part of it to
device memory (creating a mixed range), state_offset will only be the sum
of the linked system page sizes.

Since dma_iova_destroy() only frees exactly the requested length, it seems
the unlinked portion of the original IOVA allocation is permanently leaked,
which could eventually exhaust the IOVA space.

> +				 */
> +				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;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260627033325.3795298-1-honglei1.huang@amd.com?part=2