[PATCH] drm/amdkfd: Relocate TBA/TMA to opposite side of VM hole (v2)

[Christian König]

Am 29.01.24 um 18:48 schrieb Felix Kuehling:
> On 2024-01-29 11:50, Arunpravin Paneer Selvam wrote:
>>>>> @@ -339,18 +346,19 @@ static void kfd_init_apertures_v9(struct
>>>>> kfd_process_device *pdd, uint8_t id)
>>>>>        pdd->lds_base = MAKE_LDS_APP_BASE_V9();
>>>>>        pdd->lds_limit = MAKE_LDS_APP_LIMIT(pdd->lds_base);
>>>>>
>>>>> -        /* Raven needs SVM to support graphic handle, etc. Leave 
>>>>> the small
>>>>> -         * reserved space before SVM on Raven as well, even 
>>>>> though we don't
>>>>> -         * have to.
>>>>> -         * Set gpuvm_base and gpuvm_limit to CANONICAL addresses 
>>>>> so that they
>>>>> -         * are used in Thunk to reserve SVM.
>>>>> -         */
>>>>> -        pdd->gpuvm_base = SVM_USER_BASE;
>>>>> +      pdd->gpuvm_base = AMDGPU_VA_RESERVED_BOTTOM;
>>>> Hi Felix,
>>>>
>>>> pdd->gpuvm_base changes from 16KB to 2MB after this patch.
>>>>
>>>> The default mmap_min_addr(/proc/sys/vm/mmap_min_addr) is 64KB.
>>>>
>>>> That means user could get a CPU VA < 2MB while the corresponding 
>>>> GPU VA has been reserved. Will this break SVM?
>>>
>>> It could break SVM if a process tries to map or access something 
>>> below 2MB. I'm not sure what AMDGPU_VA_RESERVED_BOTTOM is used for 
>>> in the GPU page tables. But if it's causing problems for real 
>>> applications with SVM, we should look into lowering that reservation.
>>>
>> We have decided to keep AMDGPU_VA_RESERVED_BOTTOM free for catching 
>> NULL pointer dereferences.
>
> Can this be made smaller? I think the 64KB minimum address used on the 
> CPU side serves the same purpose. Could we match that on the GPU side?

Yeah I think that should work. There used to be a VCN firmware which was 
buggy and would write randomly into the first x MiB of the virtual 
address space.

Apart from that we never really encountered an issue with NULL pointers 
on the GFX side which a smaller reserved areas wouldn't have catched as 
well.

Regards,
Christian.

>
> Regards,
>   Felix
>
>
>>
>> Regards,
>> Arun.