[PATCH] drm/amdgpu: fix vm_cpu_update hit NULL pointer

[PATCH] drm/amdgpu: fix vm_cpu_update hit NULL pointer

[Monk Liu]

should use amdgpu_bo_map, otherwise you'll hit NULL
pointer bug if with amdgpu_bo_kptr

Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
index 9d53982..1fb6295a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
@@ -76,8 +76,10 @@ static int amdgpu_vm_cpu_update(struct amdgpu_vm_update_params *p,
 {
 	unsigned int i;
 	uint64_t value;
+	void *ptr;
 
-	pe += (unsigned long)amdgpu_bo_kptr(bo);
+	amdgpu_bo_kmap(bo, &ptr);
+	pe += (unsigned long)ptr;
 
 	trace_amdgpu_vm_set_ptes(pe, addr, count, incr, flags);
 
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: fix vm_cpu_update hit NULL pointer

[Christian König]

Am 22.03.19 um 06:30 schrieb Monk Liu:
> should use amdgpu_bo_map, otherwise you'll hit NULL
> pointer bug if with amdgpu_bo_kptr

Yeah that is a known problem. NAK to this one cause that would map the 
BO at the wrong time.

But in general I have a proper fix for this in the pipeline.

Christian.

>
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> index 9d53982..1fb6295a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> @@ -76,8 +76,10 @@ static int amdgpu_vm_cpu_update(struct amdgpu_vm_update_params *p,
>   {
>   	unsigned int i;
>   	uint64_t value;
> +	void *ptr;
>   
> -	pe += (unsigned long)amdgpu_bo_kptr(bo);
> +	amdgpu_bo_kmap(bo, &ptr);
> +	pe += (unsigned long)ptr;
>   
>   	trace_amdgpu_vm_set_ptes(pe, addr, count, incr, flags);
>   

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

RE: [PATCH] drm/amdgpu: fix vm_cpu_update hit NULL pointer

[Liu, Monk]

Hi Christian

Why it is the wrong time ?  the pte/pd bo already reserved, some details maybe?

Thanks
/Monk
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Friday, March 22, 2019 11:43 PM
To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: fix vm_cpu_update hit NULL pointer

Am 22.03.19 um 06:30 schrieb Monk Liu:
> should use amdgpu_bo_map, otherwise you'll hit NULL pointer bug if 
> with amdgpu_bo_kptr

Yeah that is a known problem. NAK to this one cause that would map the BO at the wrong time.

But in general I have a proper fix for this in the pipeline.

Christian.

>
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> index 9d53982..1fb6295a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> @@ -76,8 +76,10 @@ static int amdgpu_vm_cpu_update(struct amdgpu_vm_update_params *p,
>   {
>   	unsigned int i;
>   	uint64_t value;
> +	void *ptr;
>   
> -	pe += (unsigned long)amdgpu_bo_kptr(bo);
> +	amdgpu_bo_kmap(bo, &ptr);
> +	pe += (unsigned long)ptr;
>   
>   	trace_amdgpu_vm_set_ptes(pe, addr, count, incr, flags);
>   

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

Re: [PATCH] drm/amdgpu: fix vm_cpu_update hit NULL pointer

[Christian König]

Hi Monk,

well it would certainly work, but for the error handling it's just not 
the best idea to do the mapping so late.

E.g. we should do the kmap for CPU based updates as well as the GTT map 
for SDMA based updates much more earlier to avoid rolling back all work 
again when we are interrupted by a signal for example.

Give me a moment to write a patch for this, it's basically the same 
thing as your patch just called from an earlier point.

Regards,
Christian.

Am 24.03.19 um 04:42 schrieb Liu, Monk:
> Hi Christian
>
> Why it is the wrong time ?  the pte/pd bo already reserved, some details maybe?
>
> Thanks
> /Monk
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Friday, March 22, 2019 11:43 PM
> To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: fix vm_cpu_update hit NULL pointer
>
> Am 22.03.19 um 06:30 schrieb Monk Liu:
>> should use amdgpu_bo_map, otherwise you'll hit NULL pointer bug if
>> with amdgpu_bo_kptr
> Yeah that is a known problem. NAK to this one cause that would map the BO at the wrong time.
>
> But in general I have a proper fix for this in the pipeline.
>
> Christian.
>
>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 4 +++-
>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
>> index 9d53982..1fb6295a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
>> @@ -76,8 +76,10 @@ static int amdgpu_vm_cpu_update(struct amdgpu_vm_update_params *p,
>>    {
>>    	unsigned int i;
>>    	uint64_t value;
>> +	void *ptr;
>>    
>> -	pe += (unsigned long)amdgpu_bo_kptr(bo);
>> +	amdgpu_bo_kmap(bo, &ptr);
>> +	pe += (unsigned long)ptr;
>>    
>>    	trace_amdgpu_vm_set_ptes(pe, addr, count, incr, flags);
>>    
> _______________________________________________
> 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