Re: [PATCH v2 2/4] drm/ttm: Clear all DMA mappings on demand

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 26.08.21 um 19:27 schrieb Andrey Grodzovsky:
> Used by drivers supporting hot unplug to handle all
> DMA IOMMU group related dependencies before the group
> is removed during device removal and we try to access
> it after free when last device pointer from user space
> is dropped.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_device.c | 45 ++++++++++++++++++++++++++++++++
>   include/drm/ttm/ttm_device.h     |  1 +
>   2 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index 5f31acec3ad7..ea50aba13743 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -245,3 +245,48 @@ void ttm_device_fini(struct ttm_device *bdev)
>   	ttm_global_release();
>   }
>   EXPORT_SYMBOL(ttm_device_fini);
> +
> +void ttm_device_clear_dma_mappings(struct ttm_device *bdev)
> +{
> +	struct ttm_resource_manager *man;
> +	struct ttm_buffer_object *bo;
> +	unsigned int i, j;
> +
> +	spin_lock(&bdev->lru_lock);
> +	for (i = TTM_PL_SYSTEM; i < TTM_NUM_MEM_TYPES; ++i) {
> +		man = ttm_manager_type(bdev, i);
> +		if (!man || !man->use_tt)
> +			continue;
> +
> +		while (!list_empty(&man->pinned)) {
> +			bo = list_first_entry(&man->pinned, struct ttm_buffer_object, lru);
> +			/* Take ref against racing releases once lru_lock is unlocked */
> +			ttm_bo_get(bo);

This should probably use ttm_bo_get_unless_zero() since it is possible 
that the BO is already cleaning up in another thread.

> +			list_del_init(&bo->lru);
> +			spin_unlock(&bdev->lru_lock);
> +
> +			if (bo->ttm)
> +				ttm_tt_destroy_common(bo->bdev, bo->ttm);

That needs to be ttm_tt_unpopulate() or otherwise we will mess up the 
statistics.

> +
> +			ttm_bo_put(bo);
> +			spin_lock(&bdev->lru_lock);
> +		}
> +
> +		for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
> +			while (!list_empty(&man->lru[j])) {
> +				bo = list_first_entry(&man->lru[j], struct ttm_buffer_object, lru);
> +				ttm_bo_get(bo);

Same here of course.

> +				list_del_init(&bo->lru);
> +				spin_unlock(&bdev->lru_lock);
> +
> +				if (bo->ttm)
> +					ttm_tt_destroy_common(bo->bdev, bo->ttm);.

Same here.

Christian.

> +
> +				ttm_bo_put(bo);
> +				spin_lock(&bdev->lru_lock);
> +			}
> +		}
> +	}
> +	spin_unlock(&bdev->lru_lock);
> +}
> +EXPORT_SYMBOL(ttm_device_clear_dma_mappings);
> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
> index cd592f8e941b..d2837decb49a 100644
> --- a/include/drm/ttm/ttm_device.h
> +++ b/include/drm/ttm/ttm_device.h
> @@ -298,5 +298,6 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs,
>   		    struct drm_vma_offset_manager *vma_manager,
>   		    bool use_dma_alloc, bool use_dma32);
>   void ttm_device_fini(struct ttm_device *bdev);
> +void ttm_device_clear_dma_mappings(struct ttm_device *bdev);
>   
>   #endif