[SNIP]This is not a generic interface that anyone can freely access. The same permissions used by ptrace are checked when opening such an interface. See [1] [2]. [1] https://patchwork.freedesktop.org/patch/617470/?series=136572&rev=2 [2] https://patchwork.freedesktop.org/patch/617471/?series=136572&rev=2
Thanks a lot for those pointers, that is exactly what I was looking for.
And yeah, it is what I feared. You are re-implementing existing functionality, but see below.
kmap/vmap are used everywhere in the DRM subsystem to access BOs, so I’m failing to see the problem with adding a simple helper based on existing code.What#s possible and often done is to do kmap/vmap if you need to implement a CPU copy for scanout for example or for copying/validating command buffers. But that usually requires accessing the whole BO and has separate security checks. When you want to access only a few bytes of a BO that sounds massively like a peek/poke like interface and we have already rejected that more than once. There even used to be standardized GEM IOCTLs for that which have been removed by now. If you need to access BOs which are placed in not CPU accessible memory then implement the access callback for ptrace, see amdgpu_ttm_access_memory for an example how to do this.Ptrace access via vm_operations_struct.access → ttm_bo_vm_access. This series renames ttm_bo_vm_access to ttm_bo_access, with no code changes. The above function accesses a BO via kmap if it is in SYSTEM / TT, which is existing code. This function is only exposed to user space via ptrace permissions.
In this series, we implement a function [3] similar to amdgpu_ttm_access_memory for the TTM vfunc access_memory. What is missing is non-visible CPU memory access, similar to amdgpu_ttm_access_memory_sdma. This will be addressed in a follow-up and was omitted in this series given its complexity. So, this looks more or less identical to AMD's ptrace implementation, but in GPU address space. Again, I fail to see what the problem is here. What am I missing?
The main question is why can't you use the existing interfaces directly?
Additional to the peek/poke interface of ptrace Linux has the pidfd_getfd system call, see here https://man7.org/linux/man-pages/man2/pidfd_getfd.2.html.
The pidfd_getfd() allows to dup() the render node file descriptor into your gdb process. That in turn gives you all the access you need from gdb, including mapping BOs and command submission on behalf of the application.
As far as I can see that allows for the same functionality as the eudebug interface, just without any driver specific code messing with ptrace permissions and peek/poke interfaces.
So the question is still why do you need the whole eudebug interface in the first place? I might be missing something, but that seems to be superfluous from a high level view.
It's true that the AMD KFD part has still similar functionality, but that is because of the broken KFD design of tying driver state to the CPU process (which makes it inaccessible for gdb even with imported render node fd).
Both Sima and I (and partially Dave as well) have pushed back on the KFD approach. And the long term plan is to get rid of such device driver specific interface which re-implement existing functionality just differently.
So you need to have a really really good explanation why the eudebug interface is actually necessary.
Regards,
Christian.
Matt [3] https://patchwork.freedesktop.org/patch/622520/?series=140200&rev=6Regards, Christian.MattRegards, Christian.
