All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
To: "Rob Clark" <robdclark@gmail.com>,
	"Christian König" <christian.koenig@amd.com>
Cc: "David Airlie" <airlied@linux.ie>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Chia-I Wu" <olvaffe@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Emil Velikov" <emil.l.velikov@gmail.com>,
	"Thomas Hellström" <thomas_os@shipmail.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"open list:VIRTIO GPU DRIVER"
	<virtualization@lists.linux-foundation.org>,
	linux-tegra@vger.kernel.org, "Dmitry Osipenko" <digetx@gmail.com>,
	kernel@collabora.com
Subject: Re: [PATCH v8 2/2] drm/gem: Don't map imported GEMs
Date: Wed, 6 Jul 2022 10:02:07 +0300	[thread overview]
Message-ID: <2a646ce4-c2ec-3b11-77a0-cc720afd6fe1@collabora.com> (raw)
In-Reply-To: <CAF6AEGtqPeF1DjmBKgzWK39Yi81YiNjTjDNn85TKx7uwicFTSA@mail.gmail.com>

On 7/6/22 00:48, Rob Clark wrote:
> On Tue, Jul 5, 2022 at 4:51 AM Christian König <christian.koenig@amd.com> wrote:
>>
>> Am 01.07.22 um 11:02 schrieb Dmitry Osipenko:
>>> 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. 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.
>>>
>>> Majority of DRM drivers prohibit mapping of the imported GEM objects.
>>> Mapping of imported GEMs require special care from userspace since it
>>> should sync dma-buf because mapping coherency of the exporter device may
>>> not match the DRM device. Let's prohibit the mapping for all DRM drivers
>>> for consistency.
>>>
>>> Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>
>> I'm pretty sure that this is the right approach, but it's certainly more
>> than possible that somebody abused this already.
> 
> I suspect that this is abused if you run deqp cts on android.. ie. all
> winsys buffers are dma-buf imports from gralloc.  And then when you
> hit readpix...
> 
> You might only hit this in scenarios with separate gpu and display (or
> dGPU+iGPU) because self-imports are handled differently in
> drm_gem_prime_import_dev().. and maybe not in cases where you end up
> with a blit from tiled/compressed to linear.. maybe that narrows the
> scope enough to just fix it in userspace?

Given that that only drivers which use DRM-SHMEM potentially could've
map imported dma-bufs (Panfrost, Lima) and they already don't allow to
do that, I think we're good.

-- 
Best regards,
Dmitry

WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
To: "Rob Clark" <robdclark@gmail.com>,
	"Christian König" <christian.koenig@amd.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
	kernel@collabora.com, "Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@linux.ie>,
	"Emil Velikov" <emil.l.velikov@gmail.com>,
	"Thomas Hellström" <thomas_os@shipmail.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	linux-tegra@vger.kernel.org, "Dmitry Osipenko" <digetx@gmail.com>,
	"open list:VIRTIO GPU DRIVER"
	<virtualization@lists.linux-foundation.org>
Subject: Re: [PATCH v8 2/2] drm/gem: Don't map imported GEMs
Date: Wed, 6 Jul 2022 10:02:07 +0300	[thread overview]
Message-ID: <2a646ce4-c2ec-3b11-77a0-cc720afd6fe1@collabora.com> (raw)
In-Reply-To: <CAF6AEGtqPeF1DjmBKgzWK39Yi81YiNjTjDNn85TKx7uwicFTSA@mail.gmail.com>

On 7/6/22 00:48, Rob Clark wrote:
> On Tue, Jul 5, 2022 at 4:51 AM Christian König <christian.koenig@amd.com> wrote:
>>
>> Am 01.07.22 um 11:02 schrieb Dmitry Osipenko:
>>> 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. 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.
>>>
>>> Majority of DRM drivers prohibit mapping of the imported GEM objects.
>>> Mapping of imported GEMs require special care from userspace since it
>>> should sync dma-buf because mapping coherency of the exporter device may
>>> not match the DRM device. Let's prohibit the mapping for all DRM drivers
>>> for consistency.
>>>
>>> Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>
>> I'm pretty sure that this is the right approach, but it's certainly more
>> than possible that somebody abused this already.
> 
> I suspect that this is abused if you run deqp cts on android.. ie. all
> winsys buffers are dma-buf imports from gralloc.  And then when you
> hit readpix...
> 
> You might only hit this in scenarios with separate gpu and display (or
> dGPU+iGPU) because self-imports are handled differently in
> drm_gem_prime_import_dev().. and maybe not in cases where you end up
> with a blit from tiled/compressed to linear.. maybe that narrows the
> scope enough to just fix it in userspace?

Given that that only drivers which use DRM-SHMEM potentially could've
map imported dma-bufs (Panfrost, Lima) and they already don't allow to
do that, I think we're good.

-- 
Best regards,
Dmitry

  reply	other threads:[~2022-07-06  7:02 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-01  9:02 [PATCH v8 0/2] DRM GEM fixes Dmitry Osipenko
2022-07-01  9:02 ` Dmitry Osipenko
2022-07-01  9:02 ` [PATCH v8 1/2] drm/gem: Properly annotate WW context on drm_gem_lock_reservations() error Dmitry Osipenko
2022-07-01  9:02   ` Dmitry Osipenko
2022-07-05 11:33   ` Christian König
2022-07-05 11:33     ` Christian König
2022-07-05 11:33     ` Christian König via Virtualization
2022-08-09 16:44     ` Daniel Vetter
2022-08-09 16:44       ` Daniel Vetter
2022-08-09 16:44       ` Daniel Vetter
2022-08-10  6:52       ` Christian König
2022-08-10  6:52         ` Christian König via Virtualization
2022-08-10  8:33         ` Daniel Vetter
2022-08-10  8:33           ` Daniel Vetter
2022-08-10  8:33           ` Daniel Vetter
2022-08-10  9:04           ` Christian König via Virtualization
2022-08-10  9:04             ` Christian König
2022-08-10  9:04             ` Christian König
2022-07-01  9:02 ` [PATCH v8 2/2] drm/gem: Don't map imported GEMs Dmitry Osipenko
2022-07-01  9:02   ` Dmitry Osipenko
2022-07-05 11:39   ` Christian König
2022-07-05 11:39     ` Christian König
2022-07-05 11:39     ` Christian König via Virtualization
2022-07-05 21:48     ` Rob Clark
2022-07-05 21:48       ` Rob Clark
2022-07-05 21:48       ` Rob Clark
2022-07-06  7:02       ` Dmitry Osipenko [this message]
2022-07-06  7:02         ` Dmitry Osipenko
2022-08-10 11:47         ` Daniel Vetter
2022-08-10 11:47           ` Daniel Vetter
2022-08-10 11:47           ` Daniel Vetter
2022-08-10 15:08           ` Rob Clark
2022-08-10 15:08             ` Rob Clark
2022-08-10 15:08             ` Rob Clark
2022-08-10 19:26             ` Dmitry Osipenko
2022-08-10 19:26               ` Dmitry Osipenko
2022-08-10 22:03               ` Rob Clark
2022-08-10 22:03                 ` Rob Clark
2022-08-10 22:03                 ` Rob Clark
2022-08-10 22:23                 ` Dmitry Osipenko
2022-08-10 22:23                   ` Dmitry Osipenko
2022-08-10 23:19                   ` Rob Clark
2022-08-10 23:19                     ` Rob Clark
2022-08-10 23:19                     ` Rob Clark
2022-08-12 11:26                     ` Dmitry Osipenko
2022-08-12 11:26                       ` Dmitry Osipenko
2022-08-12 14:57                       ` Rob Clark
2022-08-12 14:57                         ` Rob Clark
2022-08-12 14:57                         ` Rob Clark
2022-08-12 15:01                         ` Rob Clark
2022-08-12 15:01                           ` Rob Clark
2022-08-12 15:01                           ` Rob Clark
2022-08-16 11:44                           ` Dmitry Osipenko
2022-08-16 11:44                             ` Dmitry Osipenko
2022-08-16 11:52                             ` Dmitry Osipenko
2022-08-16 11:52                               ` Dmitry Osipenko
2022-08-16 12:03                             ` Christian König
2022-08-16 12:03                               ` Christian König
2022-08-16 12:03                               ` Christian König
2022-08-16 19:55                               ` Dmitry Osipenko
2022-08-16 19:55                                 ` Dmitry Osipenko
2022-08-22 17:26                                 ` Dmitry Osipenko
2022-08-22 17:26                                   ` Dmitry Osipenko
2022-08-23 10:01                                   ` Christian König
2022-08-23 10:01                                     ` Christian König
2022-08-23 10:01                                     ` Christian König
2022-08-23 16:47                                     ` Rob Clark
2022-08-23 16:47                                       ` Rob Clark
2022-08-23 16:47                                       ` Rob Clark
2022-09-07 10:25                                       ` Dmitry Osipenko
2022-09-07 10:25                                         ` Dmitry Osipenko
2022-09-11 18:00                                         ` Rob Clark
2022-09-11 18:00                                           ` Rob Clark
2022-09-11 18:00                                           ` Rob Clark
2022-08-16 22:18                             ` Rob Clark
2022-08-16 22:18                               ` Rob Clark
2022-08-16 22:18                               ` Rob Clark
2022-08-10 19:27           ` Dmitry Osipenko
2022-07-22 11:49 ` [PATCH v8 0/2] DRM GEM fixes Dmitry Osipenko
2022-07-22 11:49   ` Dmitry Osipenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2a646ce4-c2ec-3b11-77a0-cc720afd6fe1@collabora.com \
    --to=dmitry.osipenko@collabora.com \
    --cc=airlied@linux.ie \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=digetx@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emil.l.velikov@gmail.com \
    --cc=gurchetansingh@chromium.org \
    --cc=kernel@collabora.com \
    --cc=kraxel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=olvaffe@gmail.com \
    --cc=robdclark@gmail.com \
    --cc=thomas_os@shipmail.org \
    --cc=tzimmermann@suse.de \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.