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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux