All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: fix max_entries calculation v3
@ 2020-09-02 15:21 Christian König
  2020-09-02 16:15 ` Pan, Xinhui
  0 siblings, 1 reply; 3+ messages in thread
From: Christian König @ 2020-09-02 15:21 UTC (permalink / raw)
  To: amd-gfx, Xinhui.Pan

Calculate the correct value for max_entries or we might run after the
page_address array.

v2: Xinhui pointed out we don't need the shift
v3: use local copy of start and simplify some calculation

Signed-off-by: Christian König <christian.koenig@amd.com>
Fixes: 1e691e244487 drm/amdgpu: stop allocating dummy GTT nodes
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8bc2253939be..7cbee1a7ec84 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1697,7 +1697,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 				AMDGPU_GPU_PAGES_IN_CPU_PAGE;
 		} else {
 			addr = 0;
-			max_entries = S64_MAX;
+			max_entries = mapping->last - start + 1;
 		}
 
 		if (pages_addr) {
@@ -1727,7 +1727,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 			addr += pfn << PAGE_SHIFT;
 		}
 
-		last = min((uint64_t)mapping->last, start + max_entries - 1);
+		last = start + max_entries - 1;
 		r = amdgpu_vm_bo_update_mapping(adev, vm, false, false, resv,
 						start, last, flags, addr,
 						dma_addr, fence);
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: fix max_entries calculation v3
  2020-09-02 15:21 [PATCH] drm/amdgpu: fix max_entries calculation v3 Christian König
@ 2020-09-02 16:15 ` Pan, Xinhui
  2020-09-03  7:57   ` Christian König
  0 siblings, 1 reply; 3+ messages in thread
From: Pan, Xinhui @ 2020-09-02 16:15 UTC (permalink / raw)
  To: Christian König; +Cc: Pan, Xinhui, amd-gfx



> 2020年9月2日 23:21，Christian König <ckoenig.leichtzumerken@gmail.com> 写道：
> 
> Calculate the correct value for max_entries or we might run after the
> page_address array.
> 
> v2: Xinhui pointed out we don't need the shift
> v3: use local copy of start and simplify some calculation
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Fixes: 1e691e244487 drm/amdgpu: stop allocating dummy GTT nodes
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 8bc2253939be..7cbee1a7ec84 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1697,7 +1697,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
> 				AMDGPU_GPU_PAGES_IN_CPU_PAGE;
> 		} else {
> 			addr = 0;
> -			max_entries = S64_MAX;
> +			max_entries = mapping->last - start + 1;
> 		}
> 
> 		if (pages_addr) {
> @@ -1727,7 +1727,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
> 			addr += pfn << PAGE_SHIFT;
> 		}
> 
> -		last = min((uint64_t)mapping->last, start + max_entries - 1);
> +		last = start + max_entries - 1;

I am not sure if we calculate the max_entries by nodes->size. does it work in that case?

> 		r = amdgpu_vm_bo_update_mapping(adev, vm, false, false, resv,
> 						start, last, flags, addr,
> 						dma_addr, fence);
> -- 
> 2.17.1
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cxinhui.pan%40amd.com%7Cbb2c2456534842d24e9c08d84f53cfc3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637346568673703868&amp;sdata=rLB3ME25AkyRaP6kd3JxOkvqz3iSKhHu9bkZnMMqS74%3D&amp;reserved=0

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: fix max_entries calculation v3
  2020-09-02 16:15 ` Pan, Xinhui
@ 2020-09-03  7:57   ` Christian König
  0 siblings, 0 replies; 3+ messages in thread
From: Christian König @ 2020-09-03  7:57 UTC (permalink / raw)
  To: Pan, Xinhui; +Cc: amd-gfx

Am 02.09.20 um 18:15 schrieb Pan, Xinhui:
>
>> 2020年9月2日 23:21，Christian König <ckoenig.leichtzumerken@gmail.com> 写道：
>>
>> Calculate the correct value for max_entries or we might run after the
>> page_address array.
>>
>> v2: Xinhui pointed out we don't need the shift
>> v3: use local copy of start and simplify some calculation
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Fixes: 1e691e244487 drm/amdgpu: stop allocating dummy GTT nodes
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 8bc2253939be..7cbee1a7ec84 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1697,7 +1697,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>> 				AMDGPU_GPU_PAGES_IN_CPU_PAGE;
>> 		} else {
>> 			addr = 0;
>> -			max_entries = S64_MAX;
>> +			max_entries = mapping->last - start + 1;
>> 		}
>>
>> 		if (pages_addr) {
>> @@ -1727,7 +1727,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>> 			addr += pfn << PAGE_SHIFT;
>> 		}
>>
>> -		last = min((uint64_t)mapping->last, start + max_entries - 1);
>> +		last = start + max_entries - 1;
> I am not sure if we calculate the max_entries by nodes->size. does it work in that case?

Oh, good point. When the VA range between start and last is smaller than 
the buffer itself we could indeed overrun here. Going to fix that.

Thanks,
Christian.

>
>> 		r = amdgpu_vm_bo_update_mapping(adev, vm, false, false, resv,
>> 						start, last, flags, addr,
>> 						dma_addr, fence);
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cxinhui.pan%40amd.com%7Cbb2c2456534842d24e9c08d84f53cfc3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637346568673703868&amp;sdata=rLB3ME25AkyRaP6kd3JxOkvqz3iSKhHu9bkZnMMqS74%3D&amp;reserved=0

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-03  7:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02 15:21 [PATCH] drm/amdgpu: fix max_entries calculation v3 Christian König
2020-09-02 16:15 ` Pan, Xinhui
2020-09-03  7:57   ` Christian König

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.