Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 17.01.22 um 15:50 schrieb Marek Olšák: I don't think fork() would work with userspace where all buffers are shared. It certainly doesn't work now. The driver needs to be notified that a buffer or texture is shared to ensure data coherency between processes, and the driver must execute decomp

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

I don't think fork() would work with userspace where all buffers are shared. It certainly doesn't work now. The driver needs to be notified that a buffer or texture is shared to ensure data coherency between processes, and the driver must execute decompression and other render passes when a buffer

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2022-01-17 um 9:21 a.m. schrieb Christian König: > Am 17.01.22 um 15:17 schrieb Felix Kuehling: >> Am 2022-01-17 um 6:44 a.m. schrieb Christian König: >>> Am 14.01.22 um 18:40 schrieb Felix Kuehling: Am 2022-01-14 um 12:26 p.m. schrieb Christian König: > Am 14.01.22 um 17:44 schrieb Dan

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 17.01.22 um 15:17 schrieb Felix Kuehling: Am 2022-01-17 um 6:44 a.m. schrieb Christian König: Am 14.01.22 um 18:40 schrieb Felix Kuehling: Am 2022-01-14 um 12:26 p.m. schrieb Christian König: Am 14.01.22 um 17:44 schrieb Daniel Vetter: Top post because I tried to catch up on the entire dis

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2022-01-17 um 6:44 a.m. schrieb Christian König: > Am 14.01.22 um 18:40 schrieb Felix Kuehling: >> Am 2022-01-14 um 12:26 p.m. schrieb Christian König: >>> Am 14.01.22 um 17:44 schrieb Daniel Vetter: Top post because I tried to catch up on the entire discussion here. So fundamen

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 14.01.22 um 18:40 schrieb Felix Kuehling: Am 2022-01-14 um 12:26 p.m. schrieb Christian König: Am 14.01.22 um 17:44 schrieb Daniel Vetter: Top post because I tried to catch up on the entire discussion here. So fundamentally I'm not opposed to just close this fork() hole once and for all. Th

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2022-01-14 um 12:26 p.m. schrieb Christian König: > Am 14.01.22 um 17:44 schrieb Daniel Vetter: >> Top post because I tried to catch up on the entire discussion here. >> >> So fundamentally I'm not opposed to just close this fork() hole once and >> for all. The thing that worries me from a upstr

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 14.01.22 um 17:44 schrieb Daniel Vetter: Top post because I tried to catch up on the entire discussion here. So fundamentally I'm not opposed to just close this fork() hole once and for all. The thing that worries me from a upstream/platform pov is really only if we don't do it consistently a

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Top post because I tried to catch up on the entire discussion here. So fundamentally I'm not opposed to just close this fork() hole once and for all. The thing that worries me from a upstream/platform pov is really only if we don't do it consistently across all drivers. So maybe as an idea: - Do

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Hi Christian I have reverted the change from the amd-staging-drm-next as per the discussion.  Thank you. Regards Rajneesh On 1/4/2022 1:08 PM, Felix Kuehling wrote: [+Adrian] Am 2021-12-23 um 2:05 a.m. schrieb Christian König: Am 22.12.21 um 21:53 schrieb Daniel Vetter: On Mon, Dec 20

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2022-01-07 um 3:56 a.m. schrieb Christian König: > Am 06.01.22 um 17:51 schrieb Felix Kuehling: >> Am 2022-01-06 um 11:48 a.m. schrieb Christian König: >>> Am 06.01.22 um 17:45 schrieb Felix Kuehling: Am 2022-01-06 um 4:05 a.m. schrieb Christian König: [SNIP] Also, why does your A

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 06.01.22 um 17:51 schrieb Felix Kuehling: Am 2022-01-06 um 11:48 a.m. schrieb Christian König: Am 06.01.22 um 17:45 schrieb Felix Kuehling: Am 2022-01-06 um 4:05 a.m. schrieb Christian König: [SNIP] Also, why does your ACK or NAK depend on this at all. If it's the right thing to do, it's the

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2022-01-06 um 4:05 a.m. schrieb Christian König: > Am 05.01.22 um 17:16 schrieb Felix Kuehling: >> [SNIP] But KFD doesn't know anything about the inherited BOs from the parent process. >>> Ok, why that? When the KFD is reinitializing it's context why >>> shouldn't it cleanup those VMAs

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2022-01-06 um 11:48 a.m. schrieb Christian König: > Am 06.01.22 um 17:45 schrieb Felix Kuehling: >> Am 2022-01-06 um 4:05 a.m. schrieb Christian König: >>> Am 05.01.22 um 17:16 schrieb Felix Kuehling: [SNIP] >> But KFD doesn't know anything about the inherited BOs >> from the parent

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 06.01.22 um 17:45 schrieb Felix Kuehling: Am 2022-01-06 um 4:05 a.m. schrieb Christian König: Am 05.01.22 um 17:16 schrieb Felix Kuehling: [SNIP] But KFD doesn't know anything about the inherited BOs from the parent process. Ok, why that? When the KFD is reinitializing it's context why sho

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 05.01.22 um 17:16 schrieb Felix Kuehling: [SNIP] But KFD doesn't know anything about the inherited BOs from the parent process. Ok, why that? When the KFD is reinitializing it's context why shouldn't it cleanup those VMAs? That cleanup has to be initiated by user mode. Basically closing the

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2022-01-05 um 11:16 a.m. schrieb Felix Kuehling: >> I was already wondering which mmaps through the KFD node we have left >> which cause problems here. > We still use the KFD FD for mapping doorbells and HDP flushing. These > are both SG BOs, so they cannot be CPU-mapped through render nodes. Th

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2022-01-05 um 3:08 a.m. schrieb Christian König: > Am 04.01.22 um 19:08 schrieb Felix Kuehling: >> [+Adrian] >> >> Am 2021-12-23 um 2:05 a.m. schrieb Christian König: >> >>> Am 22.12.21 um 21:53 schrieb Daniel Vetter: On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote:

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 04.01.22 um 19:08 schrieb Felix Kuehling: [+Adrian] Am 2021-12-23 um 2:05 a.m. schrieb Christian König: Am 22.12.21 um 21:53 schrieb Daniel Vetter: On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote: [SNIP] Still sounds funky. I think minimally we should have an ack from C

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

[+Adrian] Am 2021-12-23 um 2:05 a.m. schrieb Christian König: > Am 22.12.21 um 21:53 schrieb Daniel Vetter: >> On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote: >> >> [SNIP] >> Still sounds funky. I think minimally we should have an ack from CRIU >> developers that this is offic

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 22.12.21 um 21:53 schrieb Daniel Vetter: On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote: [SNIP] Still sounds funky. I think minimally we should have an ack from CRIU developers that this is officially the right way to solve this problem. I really don't want to have random

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Sorry for the typo in my previous email. Please read Adrian Reber* On 12/22/2021 8:49 PM, Bhardwaj, Rajneesh wrote: Adding Adrian Rebel who is the CRIU maintainer and CRIU list On 12/22/2021 3:53 PM, Daniel Vetter wrote: On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote: On

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Adding Adrian Rebel who is the CRIU maintainer and CRIU list On 12/22/2021 3:53 PM, Daniel Vetter wrote: On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote: On 12/20/2021 4:29 AM, Daniel Vetter wrote: On Fri, Dec 10, 2021 at 07:58:50AM +0100, Christian König wrote: Am 09.12.21

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote: > > On 12/20/2021 4:29 AM, Daniel Vetter wrote: > > On Fri, Dec 10, 2021 at 07:58:50AM +0100, Christian König wrote: > > > Am 09.12.21 um 19:28 schrieb Felix Kuehling: > > > > Am 2021-12-09 um 10:30 a.m. schrieb Christian König: >

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

On 12/20/2021 4:29 AM, Daniel Vetter wrote: On Fri, Dec 10, 2021 at 07:58:50AM +0100, Christian König wrote: Am 09.12.21 um 19:28 schrieb Felix Kuehling: Am 2021-12-09 um 10:30 a.m. schrieb Christian König: That still won't work. But I think we could do this change for the amdgpu mmap callb

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

On Fri, Dec 10, 2021 at 07:58:50AM +0100, Christian König wrote: > Am 09.12.21 um 19:28 schrieb Felix Kuehling: > > Am 2021-12-09 um 10:30 a.m. schrieb Christian König: > > > That still won't work. > > > > > > But I think we could do this change for the amdgpu mmap callback only. > > If graphics u

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 09.12.21 um 19:28 schrieb Felix Kuehling: Am 2021-12-09 um 10:30 a.m. schrieb Christian König: That still won't work. But I think we could do this change for the amdgpu mmap callback only. If graphics user mode has problems with it, we could even make this specific to KFD BOs in the amdgpu_

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 2021-12-09 um 10:30 a.m. schrieb Christian König: > That still won't work. > > But I think we could do this change for the amdgpu mmap callback only. If graphics user mode has problems with it, we could even make this specific to KFD BOs in the amdgpu_gem_object_mmap callback. Regards,   Felix

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Sounds good. I will send a v2 with only ttm_bo_mmap_obj change. Thank you! On 12/9/2021 10:27 AM, Christian König wrote: Hi Rajneesh, yes, separating this from the drm_gem_mmap_obj() change is certainly a good idea. The child cannot access the BOs mapped by the parent anyway with access res

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

That still won't work. But I think we could do this change for the amdgpu mmap callback only. Regards, Christian. Am 09.12.21 um 16:29 schrieb Bhardwaj, Rajneesh: Sounds good. I will send a v2 with only ttm_bo_mmap_obj change. Thank you! On 12/9/2021 10:27 AM, Christian König wrote: Hi Rajn

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Thanks Christian. Would it make it less intrusive if I just use the flag for ttm bo mmap and remove the drm_gem_mmap_obj change from this patch? For our use case, just the ttm_bo_mmap_obj change should suffice and we don't want to put any more work arounds in the user space (thunk, in our case)

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Hi Rajneesh, yes, separating this from the drm_gem_mmap_obj() change is certainly a good idea. The child cannot access the BOs mapped by the parent anyway with access restrictions applied exactly that is not correct. That behavior is actively used by some userspace stacks as far as I know.

Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

Am 08.12.21 um 21:53 schrieb Rajneesh Bhardwaj: When an application having open file access to a node forks, its shared mappings also get reflected in the address space of child process even though it cannot access them with the object permissions applied. With the existing permission checks on t

[PATCH] drm/ttm: Don't inherit GEM object VMAs in child process

When an application having open file access to a node forks, its shared mappings also get reflected in the address space of child process even though it cannot access them with the object permissions applied. With the existing permission checks on the gem objects, it might be reasonable to also cre