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

[PATCH v3 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.

v3:
Move pinned list to ttm device and a few others.

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               | 18 ++++++--
 drivers/gpu/drm/ttm/ttm_device.c           | 48 ++++++++++++++++++++++
 include/drm/ttm/ttm_device.h               |  2 +
 5 files changed, 68 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH v3 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
v3: Move the pinned list to ttm device

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     | 18 ++++++++++++++----
 drivers/gpu/drm/ttm/ttm_device.c |  1 +
 include/drm/ttm/ttm_device.h     |  1 +
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 1b950b45cf4b..1fedd0eb67ba 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -69,7 +69,17 @@ 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(struct ttm_buffer_object *bo)
+{
+	struct ttm_device *bdev = bo->bdev;
+
+	list_move_tail(&bo->lru, &bdev->pinned);
+
+	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 +108,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(bo);
 		return;
 	}
 
@@ -339,7 +349,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(bo);
 	list_del_init(&bo->ddestroy);
 	spin_unlock(&bo->bdev->lru_lock);
 	ttm_bo_cleanup_memtype_use(bo);
@@ -1154,7 +1164,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(bo);
 	/* TODO: Cleanup the locking */
 	spin_unlock(&bo->bdev->lru_lock);
 
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 5f31acec3ad7..530a9c36be37 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs,
 	INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
 	spin_lock_init(&bdev->lru_lock);
 	INIT_LIST_HEAD(&bdev->ddestroy);
+	INIT_LIST_HEAD(&bdev->pinned);
 	bdev->dev_mapping = mapping;
 	mutex_lock(&ttm_global_mutex);
 	list_add_tail(&bdev->device_list, &glob->device_list);
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index cd592f8e941b..03fb44d061e0 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -265,6 +265,7 @@ struct ttm_device {
 	 */
 	spinlock_t lru_lock;
 	struct list_head ddestroy;
+	struct list_head pinned;
 
 	/*
 	 * Protected by load / firstopen / lastclose /unload sync.
-- 
2.25.1

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

[Christian König]

Am 27.08.21 um 22:39 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
> v3: Move the pinned list to ttm device

As far as I can see there is not list_del() remaining. So this won't 
work correctly.

I suggest to rather rebase on top of the stuff I'm working on for a 
while to move the LRU into the resource instead.

Just send out the latest patch set of this with you in CC.

Christian.

>
> 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     | 18 ++++++++++++++----
>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>   include/drm/ttm/ttm_device.h     |  1 +
>   3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 1b950b45cf4b..1fedd0eb67ba 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -69,7 +69,17 @@ 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(struct ttm_buffer_object *bo)
> +{
> +	struct ttm_device *bdev = bo->bdev;
> +
> +	list_move_tail(&bo->lru, &bdev->pinned);
> +
> +	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 +108,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(bo);
>   		return;
>   	}
>   
> @@ -339,7 +349,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(bo);
>   	list_del_init(&bo->ddestroy);
>   	spin_unlock(&bo->bdev->lru_lock);
>   	ttm_bo_cleanup_memtype_use(bo);
> @@ -1154,7 +1164,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(bo);
>   	/* TODO: Cleanup the locking */
>   	spin_unlock(&bo->bdev->lru_lock);
>   
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index 5f31acec3ad7..530a9c36be37 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs,
>   	INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>   	spin_lock_init(&bdev->lru_lock);
>   	INIT_LIST_HEAD(&bdev->ddestroy);
> +	INIT_LIST_HEAD(&bdev->pinned);
>   	bdev->dev_mapping = mapping;
>   	mutex_lock(&ttm_global_mutex);
>   	list_add_tail(&bdev->device_list, &glob->device_list);
> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
> index cd592f8e941b..03fb44d061e0 100644
> --- a/include/drm/ttm/ttm_device.h
> +++ b/include/drm/ttm/ttm_device.h
> @@ -265,6 +265,7 @@ struct ttm_device {
>   	 */
>   	spinlock_t lru_lock;
>   	struct list_head ddestroy;
> +	struct list_head pinned;
>   
>   	/*
>   	 * Protected by load / firstopen / lastclose /unload sync.

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

[Andrey Grodzovsky]

On 2021-08-30 4:58 a.m., Christian König wrote:
> Am 27.08.21 um 22:39 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
>> v3: Move the pinned list to ttm device
>
> As far as I can see there is not list_del() remaining. So this won't 
> work correctly.


It's in ttm_bo_release, there was no code change there hence it's not 
captured in the patch.

Andrey


>
> I suggest to rather rebase on top of the stuff I'm working on for a 
> while to move the LRU into the resource instead.
>
> Just send out the latest patch set of this with you in CC.
>
> Christian.
>
>>
>> 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     | 18 ++++++++++++++----
>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>   include/drm/ttm/ttm_device.h     |  1 +
>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 1b950b45cf4b..1fedd0eb67ba 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -69,7 +69,17 @@ 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(struct ttm_buffer_object *bo)
>> +{
>> +    struct ttm_device *bdev = bo->bdev;
>> +
>> +    list_move_tail(&bo->lru, &bdev->pinned);
>> +
>> +    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 +108,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(bo);
>>           return;
>>       }
>>   @@ -339,7 +349,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(bo);
>>       list_del_init(&bo->ddestroy);
>>       spin_unlock(&bo->bdev->lru_lock);
>>       ttm_bo_cleanup_memtype_use(bo);
>> @@ -1154,7 +1164,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(bo);
>>       /* TODO: Cleanup the locking */
>>       spin_unlock(&bo->bdev->lru_lock);
>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>> b/drivers/gpu/drm/ttm/ttm_device.c
>> index 5f31acec3ad7..530a9c36be37 100644
>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>> struct ttm_device_funcs *funcs,
>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>       spin_lock_init(&bdev->lru_lock);
>>       INIT_LIST_HEAD(&bdev->ddestroy);
>> +    INIT_LIST_HEAD(&bdev->pinned);
>>       bdev->dev_mapping = mapping;
>>       mutex_lock(&ttm_global_mutex);
>>       list_add_tail(&bdev->device_list, &glob->device_list);
>> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
>> index cd592f8e941b..03fb44d061e0 100644
>> --- a/include/drm/ttm/ttm_device.h
>> +++ b/include/drm/ttm/ttm_device.h
>> @@ -265,6 +265,7 @@ struct ttm_device {
>>        */
>>       spinlock_t lru_lock;
>>       struct list_head ddestroy;
>> +    struct list_head pinned;
>>         /*
>>        * Protected by load / firstopen / lastclose /unload sync.
>

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

[Christian König]

Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky:
>
> On 2021-08-30 4:58 a.m., Christian König wrote:
>> Am 27.08.21 um 22:39 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
>>> v3: Move the pinned list to ttm device
>>
>> As far as I can see there is not list_del() remaining. So this won't 
>> work correctly.
>
>
> It's in ttm_bo_release, there was no code change there hence it's not 
> captured in the patch.

Ah! So you keep the logic as is there. Sorry totally missed that.

In this case the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>

Can you push this to drm-misc-next?

Thanks,
Christian.

>
> Andrey
>
>
>>
>> I suggest to rather rebase on top of the stuff I'm working on for a 
>> while to move the LRU into the resource instead.
>>
>> Just send out the latest patch set of this with you in CC.
>>
>> Christian.
>>
>>>
>>> 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     | 18 ++++++++++++++----
>>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>>   include/drm/ttm/ttm_device.h     |  1 +
>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>> index 1b950b45cf4b..1fedd0eb67ba 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -69,7 +69,17 @@ 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(struct ttm_buffer_object *bo)
>>> +{
>>> +    struct ttm_device *bdev = bo->bdev;
>>> +
>>> +    list_move_tail(&bo->lru, &bdev->pinned);
>>> +
>>> +    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 +108,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(bo);
>>>           return;
>>>       }
>>>   @@ -339,7 +349,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(bo);
>>>       list_del_init(&bo->ddestroy);
>>>       spin_unlock(&bo->bdev->lru_lock);
>>>       ttm_bo_cleanup_memtype_use(bo);
>>> @@ -1154,7 +1164,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(bo);
>>>       /* TODO: Cleanup the locking */
>>>       spin_unlock(&bo->bdev->lru_lock);
>>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>> index 5f31acec3ad7..530a9c36be37 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>>> struct ttm_device_funcs *funcs,
>>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>>       spin_lock_init(&bdev->lru_lock);
>>>       INIT_LIST_HEAD(&bdev->ddestroy);
>>> +    INIT_LIST_HEAD(&bdev->pinned);
>>>       bdev->dev_mapping = mapping;
>>>       mutex_lock(&ttm_global_mutex);
>>>       list_add_tail(&bdev->device_list, &glob->device_list);
>>> diff --git a/include/drm/ttm/ttm_device.h 
>>> b/include/drm/ttm/ttm_device.h
>>> index cd592f8e941b..03fb44d061e0 100644
>>> --- a/include/drm/ttm/ttm_device.h
>>> +++ b/include/drm/ttm/ttm_device.h
>>> @@ -265,6 +265,7 @@ struct ttm_device {
>>>        */
>>>       spinlock_t lru_lock;
>>>       struct list_head ddestroy;
>>> +    struct list_head pinned;
>>>         /*
>>>        * Protected by load / firstopen / lastclose /unload sync.
>>

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

[Andrey Grodzovsky]

On 2021-08-30 12:51 p.m., Christian König wrote:
> Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky:
>>
>> On 2021-08-30 4:58 a.m., Christian König wrote:
>>> Am 27.08.21 um 22:39 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
>>>> v3: Move the pinned list to ttm device
>>>
>>> As far as I can see there is not list_del() remaining. So this won't 
>>> work correctly.
>>
>>
>> It's in ttm_bo_release, there was no code change there hence it's not 
>> captured in the patch.
>
> Ah! So you keep the logic as is there. Sorry totally missed that.
>
> In this case the patch is Reviewed-by: Christian König 
> <christian.koenig@amd.com>
>
> Can you push this to drm-misc-next?
>
> Thanks,
> Christian.


I think It's supposed to go on top of your changes you mention here 
which are not pushed yet.
I will need to apply all the patches on top of yours and retest (I was 
doing everything in amd-staging-drm-next)
until now.

Andrey


>
>>
>> Andrey
>>
>>
>>>
>>> I suggest to rather rebase on top of the stuff I'm working on for a 
>>> while to move the LRU into the resource instead.
>>>
>>> Just send out the latest patch set of this with you in CC.
>>>
>>> Christian.
>>>
>>>>
>>>> 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     | 18 ++++++++++++++----
>>>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>>>   include/drm/ttm/ttm_device.h     |  1 +
>>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> index 1b950b45cf4b..1fedd0eb67ba 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> @@ -69,7 +69,17 @@ 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(struct ttm_buffer_object 
>>>> *bo)
>>>> +{
>>>> +    struct ttm_device *bdev = bo->bdev;
>>>> +
>>>> +    list_move_tail(&bo->lru, &bdev->pinned);
>>>> +
>>>> +    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 +108,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(bo);
>>>>           return;
>>>>       }
>>>>   @@ -339,7 +349,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(bo);
>>>>       list_del_init(&bo->ddestroy);
>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>       ttm_bo_cleanup_memtype_use(bo);
>>>> @@ -1154,7 +1164,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(bo);
>>>>       /* TODO: Cleanup the locking */
>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>>> index 5f31acec3ad7..530a9c36be37 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>>>> struct ttm_device_funcs *funcs,
>>>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>>>       spin_lock_init(&bdev->lru_lock);
>>>>       INIT_LIST_HEAD(&bdev->ddestroy);
>>>> +    INIT_LIST_HEAD(&bdev->pinned);
>>>>       bdev->dev_mapping = mapping;
>>>>       mutex_lock(&ttm_global_mutex);
>>>>       list_add_tail(&bdev->device_list, &glob->device_list);
>>>> diff --git a/include/drm/ttm/ttm_device.h 
>>>> b/include/drm/ttm/ttm_device.h
>>>> index cd592f8e941b..03fb44d061e0 100644
>>>> --- a/include/drm/ttm/ttm_device.h
>>>> +++ b/include/drm/ttm/ttm_device.h
>>>> @@ -265,6 +265,7 @@ struct ttm_device {
>>>>        */
>>>>       spinlock_t lru_lock;
>>>>       struct list_head ddestroy;
>>>> +    struct list_head pinned;
>>>>         /*
>>>>        * Protected by load / firstopen / lastclose /unload sync.
>>>
>

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

[Christian König]

Am 30.08.21 um 19:02 schrieb Andrey Grodzovsky:
>
> On 2021-08-30 12:51 p.m., Christian König wrote:
>> Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky:
>>>
>>> On 2021-08-30 4:58 a.m., Christian König wrote:
>>>> Am 27.08.21 um 22:39 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
>>>>> v3: Move the pinned list to ttm device
>>>>
>>>> As far as I can see there is not list_del() remaining. So this 
>>>> won't work correctly.
>>>
>>>
>>> It's in ttm_bo_release, there was no code change there hence it's 
>>> not captured in the patch.
>>
>> Ah! So you keep the logic as is there. Sorry totally missed that.
>>
>> In this case the patch is Reviewed-by: Christian König 
>> <christian.koenig@amd.com>
>>
>> Can you push this to drm-misc-next?
>>
>> Thanks,
>> Christian.
>
>
> I think It's supposed to go on top of your changes you mention here 
> which are not pushed yet.
> I will need to apply all the patches on top of yours and retest (I was 
> doing everything in amd-staging-drm-next)
> until now.

Works for me as well. Alternatively you can just push this patch here to 
drm-misc-next so that I can rebase on top and merge the rest through 
amd-staging-drm-next.

The patch pushed to drm-misc-next should automatically fall out when 
Alex rebases his stuff on upstream the next time.

Christian.

>
> Andrey
>
>
>>
>>>
>>> Andrey
>>>
>>>
>>>>
>>>> I suggest to rather rebase on top of the stuff I'm working on for a 
>>>> while to move the LRU into the resource instead.
>>>>
>>>> Just send out the latest patch set of this with you in CC.
>>>>
>>>> Christian.
>>>>
>>>>>
>>>>> 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     | 18 ++++++++++++++----
>>>>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>>>>   include/drm/ttm/ttm_device.h     |  1 +
>>>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>> index 1b950b45cf4b..1fedd0eb67ba 100644
>>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>> @@ -69,7 +69,17 @@ 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(struct ttm_buffer_object 
>>>>> *bo)
>>>>> +{
>>>>> +    struct ttm_device *bdev = bo->bdev;
>>>>> +
>>>>> +    list_move_tail(&bo->lru, &bdev->pinned);
>>>>> +
>>>>> +    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 +108,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(bo);
>>>>>           return;
>>>>>       }
>>>>>   @@ -339,7 +349,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(bo);
>>>>>       list_del_init(&bo->ddestroy);
>>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>>       ttm_bo_cleanup_memtype_use(bo);
>>>>> @@ -1154,7 +1164,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(bo);
>>>>>       /* TODO: Cleanup the locking */
>>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>>>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>>>> index 5f31acec3ad7..530a9c36be37 100644
>>>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>>>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>>>>> struct ttm_device_funcs *funcs,
>>>>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>>>>       spin_lock_init(&bdev->lru_lock);
>>>>>       INIT_LIST_HEAD(&bdev->ddestroy);
>>>>> +    INIT_LIST_HEAD(&bdev->pinned);
>>>>>       bdev->dev_mapping = mapping;
>>>>>       mutex_lock(&ttm_global_mutex);
>>>>>       list_add_tail(&bdev->device_list, &glob->device_list);
>>>>> diff --git a/include/drm/ttm/ttm_device.h 
>>>>> b/include/drm/ttm/ttm_device.h
>>>>> index cd592f8e941b..03fb44d061e0 100644
>>>>> --- a/include/drm/ttm/ttm_device.h
>>>>> +++ b/include/drm/ttm/ttm_device.h
>>>>> @@ -265,6 +265,7 @@ struct ttm_device {
>>>>>        */
>>>>>       spinlock_t lru_lock;
>>>>>       struct list_head ddestroy;
>>>>> +    struct list_head pinned;
>>>>>         /*
>>>>>        * Protected by load / firstopen / lastclose /unload sync.
>>>>
>>

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

[Andrey Grodzovsky]

On 2021-08-30 1:05 p.m., Christian König wrote:
>
>
> Am 30.08.21 um 19:02 schrieb Andrey Grodzovsky:
>>
>> On 2021-08-30 12:51 p.m., Christian König wrote:
>>> Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky:
>>>>
>>>> On 2021-08-30 4:58 a.m., Christian König wrote:
>>>>> Am 27.08.21 um 22:39 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
>>>>>> v3: Move the pinned list to ttm device
>>>>>
>>>>> As far as I can see there is not list_del() remaining. So this 
>>>>> won't work correctly.
>>>>
>>>>
>>>> It's in ttm_bo_release, there was no code change there hence it's 
>>>> not captured in the patch.
>>>
>>> Ah! So you keep the logic as is there. Sorry totally missed that.
>>>
>>> In this case the patch is Reviewed-by: Christian König 
>>> <christian.koenig@amd.com>
>>>
>>> Can you push this to drm-misc-next?
>>>
>>> Thanks,
>>> Christian.
>>
>>
>> I think It's supposed to go on top of your changes you mention here 
>> which are not pushed yet.
>> I will need to apply all the patches on top of yours and retest (I 
>> was doing everything in amd-staging-drm-next)
>> until now.
>
> Works for me as well. Alternatively you can just push this patch here 
> to drm-misc-next so that I can rebase on top and merge the rest 
> through amd-staging-drm-next.
>
> The patch pushed to drm-misc-next should automatically fall out when 
> Alex rebases his stuff on upstream the next time.
>
> Christian.


So i can both push this specific patch to drm-misc-next and also push 
the entire 4 patch series to amd-stagin-drm-next (after rest of the 
patches RBed)?

Andrey


>
>>
>> Andrey
>>
>>
>>>
>>>>
>>>> Andrey
>>>>
>>>>
>>>>>
>>>>> I suggest to rather rebase on top of the stuff I'm working on for 
>>>>> a while to move the LRU into the resource instead.
>>>>>
>>>>> Just send out the latest patch set of this with you in CC.
>>>>>
>>>>> Christian.
>>>>>
>>>>>>
>>>>>> 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     | 18 ++++++++++++++----
>>>>>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>>>>>   include/drm/ttm/ttm_device.h     |  1 +
>>>>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>>>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> index 1b950b45cf4b..1fedd0eb67ba 100644
>>>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> @@ -69,7 +69,17 @@ 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(struct 
>>>>>> ttm_buffer_object *bo)
>>>>>> +{
>>>>>> +    struct ttm_device *bdev = bo->bdev;
>>>>>> +
>>>>>> +    list_move_tail(&bo->lru, &bdev->pinned);
>>>>>> +
>>>>>> +    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 +108,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(bo);
>>>>>>           return;
>>>>>>       }
>>>>>>   @@ -339,7 +349,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(bo);
>>>>>>       list_del_init(&bo->ddestroy);
>>>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>>>       ttm_bo_cleanup_memtype_use(bo);
>>>>>> @@ -1154,7 +1164,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(bo);
>>>>>>       /* TODO: Cleanup the locking */
>>>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>>>>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>>>>> index 5f31acec3ad7..530a9c36be37 100644
>>>>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>>>>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>>>>>> struct ttm_device_funcs *funcs,
>>>>>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>>>>>       spin_lock_init(&bdev->lru_lock);
>>>>>>       INIT_LIST_HEAD(&bdev->ddestroy);
>>>>>> +    INIT_LIST_HEAD(&bdev->pinned);
>>>>>>       bdev->dev_mapping = mapping;
>>>>>>       mutex_lock(&ttm_global_mutex);
>>>>>>       list_add_tail(&bdev->device_list, &glob->device_list);
>>>>>> diff --git a/include/drm/ttm/ttm_device.h 
>>>>>> b/include/drm/ttm/ttm_device.h
>>>>>> index cd592f8e941b..03fb44d061e0 100644
>>>>>> --- a/include/drm/ttm/ttm_device.h
>>>>>> +++ b/include/drm/ttm/ttm_device.h
>>>>>> @@ -265,6 +265,7 @@ struct ttm_device {
>>>>>>        */
>>>>>>       spinlock_t lru_lock;
>>>>>>       struct list_head ddestroy;
>>>>>> +    struct list_head pinned;
>>>>>>         /*
>>>>>>        * Protected by load / firstopen / lastclose /unload sync.
>>>>>
>>>
>

[PATCH v3 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.

v3:
Switch to ttm_bo_get_unless_zerom
Iterate bdev for pinned list
Switch to ttm_tt_unpopulate

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
---
 drivers/gpu/drm/ttm/ttm_device.c | 47 ++++++++++++++++++++++++++++++++
 include/drm/ttm/ttm_device.h     |  1 +
 2 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 530a9c36be37..a691c89f5b20 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -246,3 +246,50 @@ 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);
+	while (!list_empty(&bdev->pinned)) {
+		bo = list_first_entry(&bdev->pinned, struct ttm_buffer_object, lru);
+		/* Take ref against racing releases once lru_lock is unlocked */
+		if (ttm_bo_get_unless_zero(bo)) {
+			list_del_init(&bo->lru);
+			spin_unlock(&bdev->lru_lock);
+
+			if (bo->ttm)
+				ttm_tt_unpopulate(bo->bdev, bo->ttm);
+
+			ttm_bo_put(bo);
+			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;
+
+		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);
+				if (ttm_bo_get_unless_zero(bo)) {
+					list_del_init(&bo->lru);
+					spin_unlock(&bdev->lru_lock);
+
+					if (bo->ttm)
+						ttm_tt_unpopulate(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 03fb44d061e0..07d722950d5b 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -299,5 +299,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 v3 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 v3 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