[lkml]   [2022]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v6 02/22] drm/gem: Move mapping of imported dma-bufs to drm_gem_mmap_obj()

On 6/29/22 10:22, Dmitry Osipenko wrote:
> On 6/29/22 09:40, Thomas Hellström (Intel) wrote:
>> On 5/27/22 01:50, Dmitry Osipenko wrote:
>>> Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers don't
>>> handle imported dma-bufs properly, which results in mapping of something
>>> else than the imported dma-buf. For example, on NVIDIA Tegra we get a
>>> hard
>>> lockup when userspace writes to the memory mapping of a dma-buf that was
>>> imported into Tegra's DRM GEM.
>>> To fix this bug, move mapping of imported dma-bufs to drm_gem_mmap_obj().
>>> Now mmaping of imported dma-bufs works properly for all DRM drivers.
>> Same comment about Fixes: as in patch 1,
>>> Cc:
>>> Signed-off-by: Dmitry Osipenko <>
>>> ---
>>>   drivers/gpu/drm/drm_gem.c              | 3 +++
>>>   drivers/gpu/drm/drm_gem_shmem_helper.c | 9 ---------
>>>   drivers/gpu/drm/tegra/gem.c            | 4 ++++
>>>   3 files changed, 7 insertions(+), 9 deletions(-)
>>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>>> index 86d670c71286..7c0b025508e4 100644
>>> --- a/drivers/gpu/drm/drm_gem.c
>>> +++ b/drivers/gpu/drm/drm_gem.c
>>> @@ -1038,6 +1038,9 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj,
>>> unsigned long obj_size,
>>>       if (obj_size < vma->vm_end - vma->vm_start)
>>>           return -EINVAL;
>>>   +    if (obj->import_attach)
>>> +        return dma_buf_mmap(obj->dma_buf, vma, 0);
>> If we start enabling mmaping of imported dma-bufs on a majority of
>> drivers in this way, how do we ensure that user-space is not blindly
>> using the object mmap without calling the needed DMA_BUF_IOCTL_SYNC
>> which is needed before and after cpu access of mmap'ed dma-bufs?
>> I was under the impression (admittedly without looking) that the few
>> drivers that actually called into dma_buf_mmap() had some private
>> user-mode driver code in place that ensured this happened.
> Since it's a userspace who does the mapping, then it should be a
> responsibility of userspace to do all the necessary syncing.

Sure, but nothing prohibits user-space to ignore the syncing thinking
"It works anyway", testing those drivers where the syncing is a NOP. And
when a driver that finally needs syncing is tested it's too late to fix
all broken user-space.

> I'm not
> sure whether anyone in userspace really needs to map imported dma-bufs
> in practice. Nevertheless, this use-case is broken and should be fixed
> by either allowing to do the mapping or prohibiting it.
Then I'd vote for prohibiting it, at least for now. And for the future
moving forward we could perhaps revisit the dma-buf need for syncing,
requiring those drivers that actually need it to implement emulated
coherent memory which can be done not too inefficiently (vmwgfx being
one example).


 \ /
  Last update: 2022-06-29 10:45    [W:0.135 / U:0.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site