Re: [PATCH 1/2] drm/amdgpu: increase the reserved VM size to 2MB

["Christian König" <christian.koenig@amd.com>]

Am 14.10.20 um 17:06 schrieb Chauhan, Madhav:
> [AMD Public Use]
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Wednesday, October 14, 2020 2:06 PM
> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
> Subject: Re: [PATCH 1/2] drm/amdgpu: increase the reserved VM size to 2MB
>
> Am 14.10.20 um 10:26 schrieb Chauhan, Madhav:
>> [AMD Public Use]
>>
>> -----Original Message-----
>> From: Christian König <ckoenig.leichtzumerken@gmail.com>
>> Sent: Tuesday, October 13, 2020 10:38 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Chauhan, Madhav <Madhav.Chauhan@amd.com>; Pan, Xinhui
>> <Xinhui.Pan@amd.com>
>> Subject: [PATCH 1/2] drm/amdgpu: increase the reserved VM size to 2MB
>>
>> Ideally this should be a multiple of the VM block size.
>> 2MB should at least fit for Vega/Navi.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++--
>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index 7c46937c1c0e..81ccd0a0c3db 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -112,8 +112,8 @@ struct amdgpu_bo_list_entry;
>>    #define AMDGPU_MMHUB_0				1
>>    #define AMDGPU_MMHUB_1				2
>>    
>> -/* hardcode that limit for now */
>> -#define AMDGPU_VA_RESERVED_SIZE			(1ULL << 20)
>> +/* Reserve 2MB at top/bottom of address space for kernel use */
>> +#define AMDGPU_VA_RESERVED_SIZE			(2ULL << 20)
>>
>> Looks fine to me: Reviewed-by: Madhav Chauhan <madhav.chauhan@amd.com>
>> Clarification on comment:
>> We check va_address < AMDGPU_VA_RESERVED_SIZE for invalid reservations, shouldn’t this be "bottom"instead of "top/bottom" of address space??
> In amdgpu_info_ioctl() we report AMDGPU_VA_RESERVED_SIZE as start of the usable address space and vm_size - AMDGPU_VA_RESERVED_SIZE as end.
>
> Could be that we don't check if the address in the reserved hole at the end of the address space. That would be a bug and should probably be fixed.
>
> Thanks. Inside amdgpu_gem_va_ioctl, Shouldn’t we also validate addresses bigger than AMDGPU_GMC_HOLE_END??
> Currently we allow them and just remove last 16 bits. What will happen if User passed 0x ffff ffff ffff ffff, it will be treated as 0x ffff ffff ffff
> While address space end is 0x ffff ffff 0000??

The alignment of the lower bits will we checked in amdgpu_vm_bo_map() 
and amdgpu_vm_bo_replace_map().

But we need to make sure that nobody maps something in the reserved 2MB 
at the top and that is currently indeed missing.

Going to write a patch for that.

Christian.

>
> Regards,
> Madhav
>
> Christian.
>
>> Regards,
>> Madhav
>>    
>>    /* max vmids dedicated for process */
>>    #define AMDGPU_VM_MAX_RESERVED_VMID	1

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