[PATCH v2 0/4] Various fixes to pass libdrm hotunplug tests

[PATCH v2 0/4] Various fixes to pass libdrm hotunplug tests

[Andrey Grodzovsky]

Bunch of fixes to enable passing hotplug tests i previosly added
here[1] with latest code. 
Once accepted I will enable the tests on libdrm side.

[1] - https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/172

v2:
Dropping VCE patch since relevant function already fixed in latest
code.
Moving IOMMU hnadling to TTM layer.

Andrey Grodzovsky (4):
  drm/ttm: Create pinned list
  drm/ttm: Clear all DMA mappings on demand
  drm/amdgpu: drm/amdgpu: Handle IOMMU enabled case
  drm/amdgpu: Add a UAPI flag for hot plug/unplug

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  3 +-
 drivers/gpu/drm/ttm/ttm_bo.c               | 30 +++++++++++++--
 drivers/gpu/drm/ttm/ttm_device.c           | 45 ++++++++++++++++++++++
 drivers/gpu/drm/ttm/ttm_resource.c         |  1 +
 include/drm/ttm/ttm_device.h               |  1 +
 include/drm/ttm/ttm_resource.h             |  1 +
 7 files changed, 78 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] drm/ttm: Create pinned list

[Andrey Grodzovsky]

This list will be used to capture all non VRAM BOs not
on LRU so when device is hot unplugged we can iterate
the list and unmap DMA mappings before device is removed.

v2: 
Reanme function to ttm_bo_move_to_pinned
Keep deleting BOs from LRU in the new function
if they have no resource struct assigned to them.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Suggested-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c       | 30 ++++++++++++++++++++++++++----
 drivers/gpu/drm/ttm/ttm_resource.c |  1 +
 include/drm/ttm/ttm_resource.h     |  1 +
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 1b950b45cf4b..64594819e9e7 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -69,7 +69,29 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
 	}
 }
 
-static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
+static inline void ttm_bo_move_to_pinned_or_del(struct ttm_buffer_object *bo)
+{
+	struct ttm_device *bdev = bo->bdev;
+	struct ttm_resource_manager *man = NULL;
+
+	if (bo->resource)
+		man = ttm_manager_type(bdev, bo->resource->mem_type);
+
+	/*
+	 * Some BOs might be in transient state where they don't belong
+	 * to any domain at the moment, simply remove them from whatever
+	 * LRU list they are still hanged on to keep previous functionality
+	 */
+	if (man && man->use_tt)
+		list_move_tail(&bo->lru, &man->pinned);
+	else
+		list_del_init(&bo->lru);
+
+	if (bdev->funcs->del_from_lru_notify)
+		bdev->funcs->del_from_lru_notify(bo);
+}
+
+static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
 {
 	struct ttm_device *bdev = bo->bdev;
 
@@ -98,7 +120,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
 		dma_resv_assert_held(bo->base.resv);
 
 	if (bo->pin_count) {
-		ttm_bo_del_from_lru(bo);
+		ttm_bo_move_to_pinned_or_del(bo);
 		return;
 	}
 
@@ -339,7 +361,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
 		return ret;
 	}
 
-	ttm_bo_del_from_lru(bo);
+	ttm_bo_move_to_pinned_or_del(bo);
 	list_del_init(&bo->ddestroy);
 	spin_unlock(&bo->bdev->lru_lock);
 	ttm_bo_cleanup_memtype_use(bo);
@@ -1154,7 +1176,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
 		return 0;
 	}
 
-	ttm_bo_del_from_lru(bo);
+	ttm_bo_move_to_pinned_or_del(bo);
 	/* TODO: Cleanup the locking */
 	spin_unlock(&bo->bdev->lru_lock);
 
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 2431717376e7..91165f77fe0e 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -85,6 +85,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
 
 	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
 		INIT_LIST_HEAD(&man->lru[i]);
+	INIT_LIST_HEAD(&man->pinned);
 	man->move = NULL;
 }
 EXPORT_SYMBOL(ttm_resource_manager_init);
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 140b6b9a8bbe..1ec0d5ebb59f 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -130,6 +130,7 @@ struct ttm_resource_manager {
 	 */
 
 	struct list_head lru[TTM_MAX_BO_PRIORITY];
+	struct list_head pinned;
 
 	/*
 	 * Protected by @move_lock.
-- 
2.25.1

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

[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);
+			list_del_init(&bo->lru);
+			spin_unlock(&bdev->lru_lock);
+
+			if (bo->ttm)
+				ttm_tt_destroy_common(bo->bdev, bo->ttm);
+
+			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);
+				list_del_init(&bo->lru);
+				spin_unlock(&bdev->lru_lock);
+
+				if (bo->ttm)
+					ttm_tt_destroy_common(bo->bdev, bo->ttm);
+
+				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
-- 
2.25.1

[PATCH v2 3/4] drm/amdgpu: drm/amdgpu: Handle IOMMU enabled case

[Andrey Grodzovsky]

Handle all DMA IOMMU group related dependencies before the
group is removed and we try to access it after free.

v2:
Move the actul handling function to TTM

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 0b5764aa98a4..653bd8fdaa33 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3860,6 +3860,8 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
 
 	amdgpu_device_ip_fini_early(adev);
 
+	ttm_device_clear_dma_mappings(&adev->mman.bdev);
+
 	amdgpu_gart_dummy_page_fini(adev);
 
 	amdgpu_device_unmap_mmio(adev);
-- 
2.25.1

[PATCH v2 4/4] drm/amdgpu: Add a UAPI flag for hot plug/unplug

[Andrey Grodzovsky]

To support libdrm tests.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6400259a7c4b..c2fdf67ff551 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -96,9 +96,10 @@
  * - 3.40.0 - Add AMDGPU_IDS_FLAGS_TMZ
  * - 3.41.0 - Add video codec query
  * - 3.42.0 - Add 16bpc fixed point display support
+ * - 3.43.0 - Add device hot plug/unplug support
  */
 #define KMS_DRIVER_MAJOR	3
-#define KMS_DRIVER_MINOR	42
+#define KMS_DRIVER_MINOR	43
 #define KMS_DRIVER_PATCHLEVEL	0
 
 int amdgpu_vram_limit;
-- 
2.25.1

Re: [PATCH v2 1/4] drm/ttm: Create pinned list

[Christian König]

Am 26.08.21 um 19:27 schrieb Andrey Grodzovsky:
> This list will be used to capture all non VRAM BOs not
> on LRU so when device is hot unplugged we can iterate
> the list and unmap DMA mappings before device is removed.
>
> v2:
> Reanme function to ttm_bo_move_to_pinned
> Keep deleting BOs from LRU in the new function
> if they have no resource struct assigned to them.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> Suggested-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c       | 30 ++++++++++++++++++++++++++----
>   drivers/gpu/drm/ttm/ttm_resource.c |  1 +
>   include/drm/ttm/ttm_resource.h     |  1 +
>   3 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 1b950b45cf4b..64594819e9e7 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -69,7 +69,29 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
>   	}
>   }
>   
> -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
> +static inline void ttm_bo_move_to_pinned_or_del(struct ttm_buffer_object *bo)
> +{
> +	struct ttm_device *bdev = bo->bdev;
> +	struct ttm_resource_manager *man = NULL;
> +
> +	if (bo->resource)
> +		man = ttm_manager_type(bdev, bo->resource->mem_type);
> +
> +	/*
> +	 * Some BOs might be in transient state where they don't belong
> +	 * to any domain at the moment, simply remove them from whatever
> +	 * LRU list they are still hanged on to keep previous functionality
> +	 */
> +	if (man && man->use_tt)
> +		list_move_tail(&bo->lru, &man->pinned);
> +	else
> +		list_del_init(&bo->lru);

Mhm, I'm wondering if we shouldn't keep the pinned list per device then.

But either way patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>

Thanks,
Christian.

> +
> +	if (bdev->funcs->del_from_lru_notify)
> +		bdev->funcs->del_from_lru_notify(bo);
> +}
> +
> +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>   {
>   	struct ttm_device *bdev = bo->bdev;
>   
> @@ -98,7 +120,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>   		dma_resv_assert_held(bo->base.resv);
>   
>   	if (bo->pin_count) {
> -		ttm_bo_del_from_lru(bo);
> +		ttm_bo_move_to_pinned_or_del(bo);
>   		return;
>   	}
>   
> @@ -339,7 +361,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
>   		return ret;
>   	}
>   
> -	ttm_bo_del_from_lru(bo);
> +	ttm_bo_move_to_pinned_or_del(bo);
>   	list_del_init(&bo->ddestroy);
>   	spin_unlock(&bo->bdev->lru_lock);
>   	ttm_bo_cleanup_memtype_use(bo);
> @@ -1154,7 +1176,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
>   		return 0;
>   	}
>   
> -	ttm_bo_del_from_lru(bo);
> +	ttm_bo_move_to_pinned_or_del(bo);
>   	/* TODO: Cleanup the locking */
>   	spin_unlock(&bo->bdev->lru_lock);
>   
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 2431717376e7..91165f77fe0e 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -85,6 +85,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
>   
>   	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
>   		INIT_LIST_HEAD(&man->lru[i]);
> +	INIT_LIST_HEAD(&man->pinned);
>   	man->move = NULL;
>   }
>   EXPORT_SYMBOL(ttm_resource_manager_init);
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 140b6b9a8bbe..1ec0d5ebb59f 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -130,6 +130,7 @@ struct ttm_resource_manager {
>   	 */
>   
>   	struct list_head lru[TTM_MAX_BO_PRIORITY];
> +	struct list_head pinned;
>   
>   	/*
>   	 * Protected by @move_lock.

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

[Christian König]

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

Re: [PATCH v2 0/4] Various fixes to pass libdrm hotunplug tests

[Andrey Grodzovsky]

Ping

Andrey

On 2021-08-26 1:27 p.m., Andrey Grodzovsky wrote:
> Bunch of fixes to enable passing hotplug tests i previosly added
> here[1] with latest code.
> Once accepted I will enable the tests on libdrm side.
>
> [1] - https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/172
>
> v2:
> Dropping VCE patch since relevant function already fixed in latest
> code.
> Moving IOMMU hnadling to TTM layer.
>
> Andrey Grodzovsky (4):
>    drm/ttm: Create pinned list
>    drm/ttm: Clear all DMA mappings on demand
>    drm/amdgpu: drm/amdgpu: Handle IOMMU enabled case
>    drm/amdgpu: Add a UAPI flag for hot plug/unplug
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  3 +-
>   drivers/gpu/drm/ttm/ttm_bo.c               | 30 +++++++++++++--
>   drivers/gpu/drm/ttm/ttm_device.c           | 45 ++++++++++++++++++++++
>   drivers/gpu/drm/ttm/ttm_resource.c         |  1 +
>   include/drm/ttm/ttm_device.h               |  1 +
>   include/drm/ttm/ttm_resource.h             |  1 +
>   7 files changed, 78 insertions(+), 5 deletions(-)
>