Re: [PATCH v5 20/27] drm: Scope all DRM IOCTLs with drm_dev_enter/exit

From: Felix Kuehling <felix.kuehling@amd.com>
To: Andrey Grodzovsky <andrey.grodzovsky@amd.com>,
	Daniel Vetter <daniel@ffwll.ch>
Cc: ckoenig.leichtzumerken@gmail.com, gregkh@linuxfoundation.org,
	daniel.vetter@ffwll.ch, amd-gfx@lists.freedesktop.org,
	helgaas@kernel.org, dri-devel@lists.freedesktop.org,
	linux-pci@vger.kernel.org, Alexander.Deucher@amd.com
Subject: Re: [PATCH v5 20/27] drm: Scope all DRM IOCTLs with drm_dev_enter/exit
Date: Thu, 29 Apr 2021 12:29:43 -0400	[thread overview]
Message-ID: <9c91bdae-d78c-202b-8b58-6977929f1273@amd.com> (raw)
In-Reply-To: <a0cfd25b-f9fd-5788-d2d8-331b69102622@amd.com>

Am 2021-04-29 um 12:21 p.m. schrieb Andrey Grodzovsky:
>
>
> On 2021-04-29 12:15 p.m., Felix Kuehling wrote:
>> Am 2021-04-29 um 12:04 p.m. schrieb Andrey Grodzovsky:
>>> So as I understand your preferred approach is that I scope any
>>> back_end, HW specific function with drm_dev_enter/exit because that
>>> where MMIO
>>> access takes place. But besides explicit MMIO access thorough
>>> register accessors in the HW back-end there is also indirect MMIO
>>> access
>>> taking place throughout the code in the driver because of various VRAM
>>> BOs which provide CPU access to VRAM through the VRAM BAR. This kind of
>>> access is spread all over in the driver and even in mid-layers such as
>>> TTM and not limited to HW back-end functions. It means it's much harder
>>> to spot such places to surgically scope them with drm_dev_enter/exit
>>> and
>>> also that any new such code introduced will immediately break hot
>>> unplug
>>> because the developers can't be expected to remember making their code
>>> robust to this specific use case. That why when we discussed internally
>>> what approach to take to protecting code with drm_dev_enter/exit we
>>> opted for using the widest available scope.
>>
>> VRAM can also be mapped in user mode. Is there anything preventing user
>> mode from accessing the memory after unplug? I guess the best you could
>> do is unmap it from the CPU page table and let the application segfault
>> on the next access. Or replace the mapping with a dummy page in system
>> memory?
>
> We indeed unmap but instead of letting it segfault insert dummy page on
> the next page fault. See here
> https://cgit.freedesktop.org/~agrodzov/linux/commit/?h=drm-misc-next&id=6dde3330ffa450e2e6da4d19e2fd0bb94b66b6ce
> And I am aware that this doesn't take care of KFD user mapping.
> As you know, we had some discussions with you on this topic and it's on
> my TODO list to follow up on this to solve it for KFD too.

ROCm user mode maps VRAM BOs using render nodes. So I'd expect
ttm_bo_vm_dummy_page to work for KFD as well.

I guess we'd need something special for KFD's doorbell and MMIO (HDP
flush) mappings. Was that the discussion about the file address space?

Regards,
  Felix

>
> Andrey
>
>>
>> Regards,
>>    Felix
>>
>>
>>>
>>> Andrey
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel