From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0510AC43334 for ; Wed, 29 Jun 2022 08:44:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232824AbiF2IoB (ORCPT ); Wed, 29 Jun 2022 04:44:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57314 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232774AbiF2In5 (ORCPT ); Wed, 29 Jun 2022 04:43:57 -0400 Received: from pio-pvt-msa2.bahnhof.se (pio-pvt-msa2.bahnhof.se [79.136.2.41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AE65B1F601; Wed, 29 Jun 2022 01:43:54 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by pio-pvt-msa2.bahnhof.se (Postfix) with ESMTP id 2A07C3F496; Wed, 29 Jun 2022 10:43:51 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at bahnhof.se Authentication-Results: pio-pvt-msa2.bahnhof.se (amavisd-new); dkim=pass (1024-bit key) header.d=shipmail.org Received: from pio-pvt-msa2.bahnhof.se ([127.0.0.1]) by localhost (pio-pvt-msa2.bahnhof.se [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4hntFlTqeXsy; Wed, 29 Jun 2022 10:43:49 +0200 (CEST) Received: by pio-pvt-msa2.bahnhof.se (Postfix) with ESMTPA id BD3173F380; Wed, 29 Jun 2022 10:43:46 +0200 (CEST) Received: from [192.168.0.209] (h-155-4-205-35.A357.priv.bahnhof.se [155.4.205.35]) by mail1.shipmail.org (Postfix) with ESMTPSA id 009DD3600FE; Wed, 29 Jun 2022 10:43:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=shipmail.org; s=mail; t=1656492226; bh=x90oW/9huaBEVhlYQnplarSqAv+GdIC9FB+WPge8YXg=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=To+37xln8JJvgy8na/moijj7u3YDsnUNuVyQoNBDexiiEMSNqGDIeVvz1kBQFzukt YcG6WWmRdCE6ePPgCuKLz9/xnAc5XIr2/e3dHA4nZ+veHTgrFehSwVxq0B1yIKdGKa 7lAinpshdXw/bjo7xkx8+5chsPjxtjKahuceuJ/M= Message-ID: Date: Wed, 29 Jun 2022 10:43:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v6 02/22] drm/gem: Move mapping of imported dma-bufs to drm_gem_mmap_obj() Content-Language: en-US To: Dmitry Osipenko Cc: intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, virtualization@lists.linux-foundation.org, linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org, linux-tegra@vger.kernel.org, Dmitry Osipenko , kernel@collabora.com, linux-media@vger.kernel.org, David Airlie , Gerd Hoffmann , Gurchetan Singh , Chia-I Wu , Daniel Vetter , Daniel Almeida , Gert Wollny , Gustavo Padovan , Daniel Stone , Tomeu Vizoso , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Rob Herring , Steven Price , Alyssa Rosenzweig , Rob Clark , Emil Velikov , Robin Murphy , Qiang Yu , Sumit Semwal , =?UTF-8?Q?Christian_K=c3=b6nig?= , "Pan, Xinhui" , Thierry Reding , Tomasz Figa , Marek Szyprowski , Mauro Carvalho Chehab , Alex Deucher , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Tvrtko Ursulin References: <20220526235040.678984-1-dmitry.osipenko@collabora.com> <20220526235040.678984-3-dmitry.osipenko@collabora.com> From: =?UTF-8?Q?Thomas_Hellstr=c3=b6m_=28Intel=29?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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: stable@vger.kernel.org >>> 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). /Thomas