> 2020年9月2日 22:31,Christian König <ckoenig.leichtzumer...@gmail.com> 写道:
> 
> Am 02.09.20 um 16:27 schrieb Pan, Xinhui:
>> 
>>> 2020年9月2日 22:05,Christian König <ckoenig.leichtzumer...@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
>>> 
>>> Signed-off-by: Christian König <christian.koe...@amd.com>
>>> Fixes: 1e691e244487 drm/amdgpu: stop allocating dummy GTT nodes
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 8bc2253939be..be886bdca5c6 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 - mapping->start + 1;
>> You need minus pfn here.
> 
> That doesn't sound correct either. The pfn is the destination of the mapping, 
> e.g. the offset inside the BO and not related to the virtual address range we 
> map.

I mean we need minus pfn too. pfn is mapping->offset >> PAGE_SHIFT.

In amdgpu_vm_bo_map(), there is a check  below
if (bo && offset + size > amdgpu_bo_size(bo))
return -EINVAL;
so mapping->offset is just an offset_in_bytes inside the BO as you said. 

mapping->start and mapping->last are virtual addresses in pfns, the range we 
are going to touch then is [start+ offset_in_pfns, last].

> 
>> The range we are going to touch is [start + offset, last].
>> so the max_entries is last - (start + offset) + 1. and offset is pfn in this 
>> case.
>> 
>> I still hit panic with this patch in practice.
> 
> Thanks for testing, I think I know what the problem is.
> 
> We need to start instead of mapping->start or otherwise the values is to 
> large after the first iteration.
> 
> Give me a second for a v3.
> 
> Christian.
> 
>> 
>>>             }
>>> 
>>>             if (pages_addr) {
>>> -- 
>>> 2.17.1
>>> 
> 

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

Reply via email to