All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
@ 2018-07-05 18:56 Andrey Grodzovsky
       [not found] ` <1530816978-19239-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Andrey Grodzovsky @ 2018-07-05 18:56 UTC (permalink / raw
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Andrey Grodzovsky, zhoucm1-5C7GfCeVMHo,
	christian.koenig-5C7GfCeVMHo

Problem: When PD/PT update made by CPU root PD was not yet mapped causing
page fault.

Fix: Verify root PD is mapped into CPU address space.

v2:
Make sure that we add the root PD to the relocated list
since then it's get mapped into CPU address space bt default
in amdgpu_vm_update_directories.

Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 845f73a..1a8caf1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
 		return;
 	list_add_tail(&base->bo_list, &bo->va);
 
+	if (bo->tbo.type == ttm_bo_type_kernel)
+		list_move(&base->vm_status, &vm->relocated);
+
 	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
 		return;
 
@@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
 	 * is currently evicted. add the bo to the evicted list to make sure it
 	 * is validated on next vm use to avoid fault.
 	 * */
-	list_move_tail(&base->vm_status, &vm->evicted);
+	if (bo->tbo.type != ttm_bo_type_kernel)
+		list_move_tail(&base->vm_status, &vm->evicted);
 }
 
 /**
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
       [not found] ` <1530816978-19239-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-06  8:02   ` Christian König
       [not found]     ` <7c62ac90-6cba-95e9-c659-5edbdda36569-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2018-07-06  8:02 UTC (permalink / raw
  To: Andrey Grodzovsky, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: zhoucm1-5C7GfCeVMHo, christian.koenig-5C7GfCeVMHo

Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
> Problem: When PD/PT update made by CPU root PD was not yet mapped causing
> page fault.
>
> Fix: Verify root PD is mapped into CPU address space.
>
> v2:
> Make sure that we add the root PD to the relocated list
> since then it's get mapped into CPU address space bt default
> in amdgpu_vm_update_directories.
>
> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 845f73a..1a8caf1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>   		return;
>   	list_add_tail(&base->bo_list, &bo->va);
>   
> +	if (bo->tbo.type == ttm_bo_type_kernel)
> +		list_move(&base->vm_status, &vm->relocated);
> +
>   	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>   		return;
>   
> @@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>   	 * is currently evicted. add the bo to the evicted list to make sure it
>   	 * is validated on next vm use to avoid fault.
>   	 * */
> -	list_move_tail(&base->vm_status, &vm->evicted);
> +	if (bo->tbo.type != ttm_bo_type_kernel)
> +		list_move_tail(&base->vm_status, &vm->evicted);

You need to drop that chunk, the evicted state supersedes the relocated 
state (e.g. when they are validated they move from evicted to relocated).

Additional to that the now superfluous move in amdgpu_vm_alloc_levels() 
can be removed.

Christian.

>   }
>   
>   /**

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
       [not found]     ` <7c62ac90-6cba-95e9-c659-5edbdda36569-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-07-06 11:02       ` Huang Rui
  2018-07-06 11:15         ` Christian König
  2018-07-06 13:50       ` Andrey Grodzovsky
  1 sibling, 1 reply; 9+ messages in thread
From: Huang Rui @ 2018-07-06 11:02 UTC (permalink / raw
  To: christian.koenig-5C7GfCeVMHo
  Cc: Andrey Grodzovsky, zhoucm1-5C7GfCeVMHo,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Fri, Jul 06, 2018 at 10:02:32AM +0200, Christian König wrote:
> Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
> >Problem: When PD/PT update made by CPU root PD was not yet mapped causing
> >page fault.
> >
> >Fix: Verify root PD is mapped into CPU address space.
> >
> >v2:
> >Make sure that we add the root PD to the relocated list
> >since then it's get mapped into CPU address space bt default
> >in amdgpu_vm_update_directories.
> >
> >Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
> >Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> >
> >Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> >---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >index 845f73a..1a8caf1 100644
> >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >@@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
> >  		return;
> >  	list_add_tail(&base->bo_list, &bo->va);
> >+	if (bo->tbo.type == ttm_bo_type_kernel)
> >+		list_move(&base->vm_status, &vm->relocated);
> >+
> >  	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
> >  		return;
> >@@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
> >  	 * is currently evicted. add the bo to the evicted list to make sure it
> >  	 * is validated on next vm use to avoid fault.
> >  	 * */
> >-	list_move_tail(&base->vm_status, &vm->evicted);
> >+	if (bo->tbo.type != ttm_bo_type_kernel)
> >+		list_move_tail(&base->vm_status, &vm->evicted);
> 
> You need to drop that chunk, the evicted state supersedes the
> relocated state (e.g. when they are validated they move from evicted
> to relocated).
> 

Is that mean we must also move root bo to relocated list like other PD/PT
BOs, then we do the kmap under amdgpu_vm_update_directories for updating
pte with mmio from pcie?

Why we avoid kernel type bo to move evicted list if they are not in
preferred_domains here? You know we checked (bo->preferred_domains &
amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)) last step.

Thanks,
Ray

> Additional to that the now superfluous move in
> amdgpu_vm_alloc_levels() can be removed.
> 
> Christian.
> 
> >  }
> >  /**
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
  2018-07-06 11:02       ` Huang Rui
@ 2018-07-06 11:15         ` Christian König
  0 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2018-07-06 11:15 UTC (permalink / raw
  To: Huang Rui, christian.koenig-5C7GfCeVMHo
  Cc: Andrey Grodzovsky, zhoucm1-5C7GfCeVMHo,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 06.07.2018 um 13:02 schrieb Huang Rui:
> On Fri, Jul 06, 2018 at 10:02:32AM +0200, Christian König wrote:
>> Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
>>> Problem: When PD/PT update made by CPU root PD was not yet mapped causing
>>> page fault.
>>>
>>> Fix: Verify root PD is mapped into CPU address space.
>>>
>>> v2:
>>> Make sure that we add the root PD to the relocated list
>>> since then it's get mapped into CPU address space bt default
>>> in amdgpu_vm_update_directories.
>>>
>>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 845f73a..1a8caf1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>>>   		return;
>>>   	list_add_tail(&base->bo_list, &bo->va);
>>> +	if (bo->tbo.type == ttm_bo_type_kernel)
>>> +		list_move(&base->vm_status, &vm->relocated);
>>> +
>>>   	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>>>   		return;
>>> @@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>>>   	 * is currently evicted. add the bo to the evicted list to make sure it
>>>   	 * is validated on next vm use to avoid fault.
>>>   	 * */
>>> -	list_move_tail(&base->vm_status, &vm->evicted);
>>> +	if (bo->tbo.type != ttm_bo_type_kernel)
>>> +		list_move_tail(&base->vm_status, &vm->evicted);
>> You need to drop that chunk, the evicted state supersedes the
>> relocated state (e.g. when they are validated they move from evicted
>> to relocated).
>>
> Is that mean we must also move root bo to relocated list like other PD/PT
> BOs, then we do the kmap under amdgpu_vm_update_directories for updating
> pte with mmio from pcie?

Yes, exactly.

> Why we avoid kernel type bo to move evicted list if they are not in
> preferred_domains here? You know we checked (bo->preferred_domains &
> amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)) last step.

Well that is the point, we must move the PDs to the evicted list when 
they are not in one of the preferred_domains.

Christian.

>
> Thanks,
> Ray
>
>> Additional to that the now superfluous move in
>> amdgpu_vm_alloc_levels() can be removed.
>>
>> Christian.
>>
>>>   }
>>>   /**
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
       [not found]     ` <7c62ac90-6cba-95e9-c659-5edbdda36569-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2018-07-06 11:02       ` Huang Rui
@ 2018-07-06 13:50       ` Andrey Grodzovsky
       [not found]         ` <dc4e7d5d-cb48-5d51-fdd3-186e5f94d77d-5C7GfCeVMHo@public.gmane.org>
  1 sibling, 1 reply; 9+ messages in thread
From: Andrey Grodzovsky @ 2018-07-06 13:50 UTC (permalink / raw
  To: christian.koenig-5C7GfCeVMHo,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: zhoucm1-5C7GfCeVMHo



On 07/06/2018 04:02 AM, Christian König wrote:
> Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
>> Problem: When PD/PT update made by CPU root PD was not yet mapped 
>> causing
>> page fault.
>>
>> Fix: Verify root PD is mapped into CPU address space.
>>
>> v2:
>> Make sure that we add the root PD to the relocated list
>> since then it's get mapped into CPU address space bt default
>> in amdgpu_vm_update_directories.
>>
>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>
>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 845f73a..1a8caf1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct 
>> amdgpu_vm_bo_base *base,
>>           return;
>>       list_add_tail(&base->bo_list, &bo->va);
>>   +    if (bo->tbo.type == ttm_bo_type_kernel)
>> +        list_move(&base->vm_status, &vm->relocated);
>> +
>>       if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>>           return;
>>   @@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct 
>> amdgpu_vm_bo_base *base,
>>        * is currently evicted. add the bo to the evicted list to make 
>> sure it
>>        * is validated on next vm use to avoid fault.
>>        * */
>> -    list_move_tail(&base->vm_status, &vm->evicted);
>> +    if (bo->tbo.type != ttm_bo_type_kernel)
>> +        list_move_tail(&base->vm_status, &vm->evicted);
>
> You need to drop that chunk, the evicted state supersedes the 
> relocated state (e.g. when they are validated they move from evicted 
> to relocated).

I don't get it, can you explain more in detail why it's OK to remove it 
? They will not be in evicted list any more.

Andrey

>
> Additional to that the now superfluous move in 
> amdgpu_vm_alloc_levels() can be removed.
>
> Christian.
>
>>   }
>>     /**
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
       [not found]         ` <dc4e7d5d-cb48-5d51-fdd3-186e5f94d77d-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-06 13:55           ` Christian König
       [not found]             ` <aaca46e9-2bdd-9ee0-2d61-462f903e67cf-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2018-07-06 13:55 UTC (permalink / raw
  To: Andrey Grodzovsky, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: zhoucm1-5C7GfCeVMHo

Am 06.07.2018 um 15:50 schrieb Andrey Grodzovsky:
>
>
> On 07/06/2018 04:02 AM, Christian König wrote:
>> Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
>>> Problem: When PD/PT update made by CPU root PD was not yet mapped 
>>> causing
>>> page fault.
>>>
>>> Fix: Verify root PD is mapped into CPU address space.
>>>
>>> v2:
>>> Make sure that we add the root PD to the relocated list
>>> since then it's get mapped into CPU address space bt default
>>> in amdgpu_vm_update_directories.
>>>
>>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 845f73a..1a8caf1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct 
>>> amdgpu_vm_bo_base *base,
>>>           return;
>>>       list_add_tail(&base->bo_list, &bo->va);
>>>   +    if (bo->tbo.type == ttm_bo_type_kernel)
>>> +        list_move(&base->vm_status, &vm->relocated);
>>> +
>>>       if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>>>           return;
>>>   @@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct 
>>> amdgpu_vm_bo_base *base,
>>>        * is currently evicted. add the bo to the evicted list to 
>>> make sure it
>>>        * is validated on next vm use to avoid fault.
>>>        * */
>>> -    list_move_tail(&base->vm_status, &vm->evicted);
>>> +    if (bo->tbo.type != ttm_bo_type_kernel)
>>> +        list_move_tail(&base->vm_status, &vm->evicted);
>>
>> You need to drop that chunk, the evicted state supersedes the 
>> relocated state (e.g. when they are validated they move from evicted 
>> to relocated).
>
> I don't get it, can you explain more in detail why it's OK to remove 
> it ? They will not be in evicted list any more.

See what happens with the BOs on the evicted list -> they get moved to 
the relocated list after validation.

Christian.

>
> Andrey
>
>>
>> Additional to that the now superfluous move in 
>> amdgpu_vm_alloc_levels() can be removed.
>>
>> Christian.
>>
>>>   }
>>>     /**
>>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
       [not found]             ` <aaca46e9-2bdd-9ee0-2d61-462f903e67cf-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-06 14:22               ` Andrey Grodzovsky
       [not found]                 ` <0b0ccd78-31e9-8edd-865c-9bb2f88fda98-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Andrey Grodzovsky @ 2018-07-06 14:22 UTC (permalink / raw
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: zhoucm1-5C7GfCeVMHo



On 07/06/2018 09:55 AM, Christian König wrote:
> Am 06.07.2018 um 15:50 schrieb Andrey Grodzovsky:
>>
>>
>> On 07/06/2018 04:02 AM, Christian König wrote:
>>> Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
>>>> Problem: When PD/PT update made by CPU root PD was not yet mapped 
>>>> causing
>>>> page fault.
>>>>
>>>> Fix: Verify root PD is mapped into CPU address space.
>>>>
>>>> v2:
>>>> Make sure that we add the root PD to the relocated list
>>>> since then it's get mapped into CPU address space bt default
>>>> in amdgpu_vm_update_directories.
>>>>
>>>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>
>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> index 845f73a..1a8caf1 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> @@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct 
>>>> amdgpu_vm_bo_base *base,
>>>>           return;
>>>>       list_add_tail(&base->bo_list, &bo->va);
>>>>   +    if (bo->tbo.type == ttm_bo_type_kernel)
>>>> +        list_move(&base->vm_status, &vm->relocated);
>>>> +
>>>>       if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>>>>           return;
>>>>   @@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct 
>>>> amdgpu_vm_bo_base *base,
>>>>        * is currently evicted. add the bo to the evicted list to 
>>>> make sure it
>>>>        * is validated on next vm use to avoid fault.
>>>>        * */
>>>> -    list_move_tail(&base->vm_status, &vm->evicted);
>>>> +    if (bo->tbo.type != ttm_bo_type_kernel)
>>>> +        list_move_tail(&base->vm_status, &vm->evicted);
>>>
>>> You need to drop that chunk, the evicted state supersedes the 
>>> relocated state (e.g. when they are validated they move from evicted 
>>> to relocated).
>>
>> I don't get it, can you explain more in detail why it's OK to remove 
>> it ? They will not be in evicted list any more.
>
> See what happens with the BOs on the evicted list -> they get moved to 
> the relocated list after validation.
>
> Christian.

Oh, so you meant keep the original code right ? The 
"list_move_tail(&base->vm_status, &vm->evicted); ?
Meaning just remove the " if (bo->tbo.type != ttm_bo_type_kernel)" ?

ANdrey

>
>>
>> Andrey
>>
>>>
>>> Additional to that the now superfluous move in 
>>> amdgpu_vm_alloc_levels() can be removed.
>>>
>>> Christian.
>>>
>>>>   }
>>>>     /**
>>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
       [not found]                 ` <0b0ccd78-31e9-8edd-865c-9bb2f88fda98-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-06 14:30                   ` Christian König
       [not found]                     ` <c99b56b6-ab98-ef9c-3e85-913bcecd1e88-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2018-07-06 14:30 UTC (permalink / raw
  To: Andrey Grodzovsky, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: zhoucm1-5C7GfCeVMHo

Am 06.07.2018 um 16:22 schrieb Andrey Grodzovsky:
>
>
> On 07/06/2018 09:55 AM, Christian König wrote:
>> Am 06.07.2018 um 15:50 schrieb Andrey Grodzovsky:
>>>
>>>
>>> On 07/06/2018 04:02 AM, Christian König wrote:
>>>> Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
>>>>> Problem: When PD/PT update made by CPU root PD was not yet mapped 
>>>>> causing
>>>>> page fault.
>>>>>
>>>>> Fix: Verify root PD is mapped into CPU address space.
>>>>>
>>>>> v2:
>>>>> Make sure that we add the root PD to the relocated list
>>>>> since then it's get mapped into CPU address space bt default
>>>>> in amdgpu_vm_update_directories.
>>>>>
>>>>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
>>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> index 845f73a..1a8caf1 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> @@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct 
>>>>> amdgpu_vm_bo_base *base,
>>>>>           return;
>>>>>       list_add_tail(&base->bo_list, &bo->va);
>>>>>   +    if (bo->tbo.type == ttm_bo_type_kernel)
>>>>> +        list_move(&base->vm_status, &vm->relocated);
>>>>> +
>>>>>       if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>>>>>           return;
>>>>>   @@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct 
>>>>> amdgpu_vm_bo_base *base,
>>>>>        * is currently evicted. add the bo to the evicted list to 
>>>>> make sure it
>>>>>        * is validated on next vm use to avoid fault.
>>>>>        * */
>>>>> -    list_move_tail(&base->vm_status, &vm->evicted);
>>>>> +    if (bo->tbo.type != ttm_bo_type_kernel)
>>>>> +        list_move_tail(&base->vm_status, &vm->evicted);
>>>>
>>>> You need to drop that chunk, the evicted state supersedes the 
>>>> relocated state (e.g. when they are validated they move from 
>>>> evicted to relocated).
>>>
>>> I don't get it, can you explain more in detail why it's OK to remove 
>>> it ? They will not be in evicted list any more.
>>
>> See what happens with the BOs on the evicted list -> they get moved 
>> to the relocated list after validation.
>>
>> Christian.
>
> Oh, so you meant keep the original code right ? The 
> "list_move_tail(&base->vm_status, &vm->evicted); ?
> Meaning just remove the " if (bo->tbo.type != ttm_bo_type_kernel)" ?

Yes, yes, of course. I mean you should drop the chunk of changes, e.g. 
keep it as it is.

Sorry for the confusion,
Christian.

>
> ANdrey
>
>>
>>>
>>> Andrey
>>>
>>>>
>>>> Additional to that the now superfluous move in 
>>>> amdgpu_vm_alloc_levels() can be removed.
>>>>
>>>> Christian.
>>>>
>>>>>   }
>>>>>     /**
>>>>
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.
       [not found]                     ` <c99b56b6-ab98-ef9c-3e85-913bcecd1e88-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-06 14:30                       ` Andrey Grodzovsky
  0 siblings, 0 replies; 9+ messages in thread
From: Andrey Grodzovsky @ 2018-07-06 14:30 UTC (permalink / raw
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: zhoucm1-5C7GfCeVMHo



On 07/06/2018 10:30 AM, Christian König wrote:
> Am 06.07.2018 um 16:22 schrieb Andrey Grodzovsky:
>>
>>
>> On 07/06/2018 09:55 AM, Christian König wrote:
>>> Am 06.07.2018 um 15:50 schrieb Andrey Grodzovsky:
>>>>
>>>>
>>>> On 07/06/2018 04:02 AM, Christian König wrote:
>>>>> Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
>>>>>> Problem: When PD/PT update made by CPU root PD was not yet mapped 
>>>>>> causing
>>>>>> page fault.
>>>>>>
>>>>>> Fix: Verify root PD is mapped into CPU address space.
>>>>>>
>>>>>> v2:
>>>>>> Make sure that we add the root PD to the relocated list
>>>>>> since then it's get mapped into CPU address space bt default
>>>>>> in amdgpu_vm_update_directories.
>>>>>>
>>>>>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
>>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>>>
>>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
>>>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> index 845f73a..1a8caf1 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> @@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct 
>>>>>> amdgpu_vm_bo_base *base,
>>>>>>           return;
>>>>>>       list_add_tail(&base->bo_list, &bo->va);
>>>>>>   +    if (bo->tbo.type == ttm_bo_type_kernel)
>>>>>> +        list_move(&base->vm_status, &vm->relocated);
>>>>>> +
>>>>>>       if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>>>>>>           return;
>>>>>>   @@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct 
>>>>>> amdgpu_vm_bo_base *base,
>>>>>>        * is currently evicted. add the bo to the evicted list to 
>>>>>> make sure it
>>>>>>        * is validated on next vm use to avoid fault.
>>>>>>        * */
>>>>>> -    list_move_tail(&base->vm_status, &vm->evicted);
>>>>>> +    if (bo->tbo.type != ttm_bo_type_kernel)
>>>>>> +        list_move_tail(&base->vm_status, &vm->evicted);
>>>>>
>>>>> You need to drop that chunk, the evicted state supersedes the 
>>>>> relocated state (e.g. when they are validated they move from 
>>>>> evicted to relocated).
>>>>
>>>> I don't get it, can you explain more in detail why it's OK to 
>>>> remove it ? They will not be in evicted list any more.
>>>
>>> See what happens with the BOs on the evicted list -> they get moved 
>>> to the relocated list after validation.
>>>
>>> Christian.
>>
>> Oh, so you meant keep the original code right ? The 
>> "list_move_tail(&base->vm_status, &vm->evicted); ?
>> Meaning just remove the " if (bo->tbo.type != ttm_bo_type_kernel)" ?
>
> Yes, yes, of course. I mean you should drop the chunk of changes, e.g. 
> keep it as it is.
>
> Sorry for the confusion,
> Christian.

OK. Thanks.

Andrey

>
>>
>> ANdrey
>>
>>>
>>>>
>>>> Andrey
>>>>
>>>>>
>>>>> Additional to that the now superfluous move in 
>>>>> amdgpu_vm_alloc_levels() can be removed.
>>>>>
>>>>> Christian.
>>>>>
>>>>>>   }
>>>>>>     /**
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-07-06 14:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-05 18:56 [PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space Andrey Grodzovsky
     [not found] ` <1530816978-19239-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
2018-07-06  8:02   ` Christian König
     [not found]     ` <7c62ac90-6cba-95e9-c659-5edbdda36569-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-07-06 11:02       ` Huang Rui
2018-07-06 11:15         ` Christian König
2018-07-06 13:50       ` Andrey Grodzovsky
     [not found]         ` <dc4e7d5d-cb48-5d51-fdd3-186e5f94d77d-5C7GfCeVMHo@public.gmane.org>
2018-07-06 13:55           ` Christian König
     [not found]             ` <aaca46e9-2bdd-9ee0-2d61-462f903e67cf-5C7GfCeVMHo@public.gmane.org>
2018-07-06 14:22               ` Andrey Grodzovsky
     [not found]                 ` <0b0ccd78-31e9-8edd-865c-9bb2f88fda98-5C7GfCeVMHo@public.gmane.org>
2018-07-06 14:30                   ` Christian König
     [not found]                     ` <c99b56b6-ab98-ef9c-3e85-913bcecd1e88-5C7GfCeVMHo@public.gmane.org>
2018-07-06 14:30                       ` Andrey Grodzovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.