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

[Andrey Grodzovsky]

On 2021-04-29 12:29 p.m., Felix Kuehling wrote:
> 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?

Yes

Andrey

> 
> Regards,
>    Felix
> 
> 
>>
>> Andrey
>>
>>>
>>> Regards,
>>>     Felix
>>>
>>>
>>>>
>>>> Andrey