From: Thomas Zimmermann <tzimmermann@suse.de>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: "DRI Development" <dri-devel@lists.freedesktop.org>,
linaro-mm-sig@lists.linaro.org, "Jason Gunthorpe" <jgg@ziepe.ca>,
"Matthew Wilcox" <willy@infradead.org>,
"Daniel Vetter" <daniel.vetter@intel.com>,
"Suren Baghdasaryan" <surenb@google.com>,
"Christian König" <christian.koenig@amd.com>,
linux-media@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH v4 1/4] dma-buf: Require VM_PFNMAP vma for mmap
Date: Fri, 23 Jul 2021 20:45:48 +0200 [thread overview]
Message-ID: <7009d234-a19a-bc37-5b1f-9f11bd5db747@suse.de> (raw)
In-Reply-To: <20210713205153.1896059-2-daniel.vetter@ffwll.ch>
[-- Attachment #1.1.1: Type: text/plain, Size: 4490 bytes --]
Hi
Am 13.07.21 um 22:51 schrieb Daniel Vetter:
> tldr; DMA buffers aren't normal memory, expecting that you can use
> them like that (like calling get_user_pages works, or that they're
> accounting like any other normal memory) cannot be guaranteed.
>
> Since some userspace only runs on integrated devices, where all
> buffers are actually all resident system memory, there's a huge
> temptation to assume that a struct page is always present and useable
> like for any more pagecache backed mmap. This has the potential to
> result in a uapi nightmare.
>
> To stop this gap require that DMA buffer mmaps are VM_PFNMAP, which
> blocks get_user_pages and all the other struct page based
> infrastructure for everyone. In spirit this is the uapi counterpart to
> the kernel-internal CONFIG_DMABUF_DEBUG.
>
> Motivated by a recent patch which wanted to swich the system dma-buf
> heap to vm_insert_page instead of vm_insert_pfn.
>
> v2:
>
> Jason brought up that we also want to guarantee that all ptes have the
> pte_special flag set, to catch fast get_user_pages (on architectures
> that support this). Allowing VM_MIXEDMAP (like VM_SPECIAL does) would
> still allow vm_insert_page, but limiting to VM_PFNMAP will catch that.
>
> From auditing the various functions to insert pfn pte entires
> (vm_insert_pfn_prot, remap_pfn_range and all it's callers like
> dma_mmap_wc) it looks like VM_PFNMAP is already required anyway, so
> this should be the correct flag to check for.
>
> References: https://lore.kernel.org/lkml/CAKMK7uHi+mG0z0HUmNt13QCCvutuRVjpcR0NjRL12k-WbWzkRg@mail.gmail.com/
> Acked-by: Christian König <christian.koenig@amd.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> --
> Resending this so I can test the next two patches for vgem/shmem in
> intel-gfx-ci. Last round failed somehow, but I can't repro that at all
> locally here.
>
> No immediate plans to merge this patch here since ttm isn't addressed
> yet (and there we have the hugepte issue, for which I don't think we
> have a clear consensus yet).
> -Daniel
> ---
> drivers/dma-buf/dma-buf.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 510b42771974..65cbd7f0f16a 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -130,6 +130,7 @@ static struct file_system_type dma_buf_fs_type = {
> static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
> {
> struct dma_buf *dmabuf;
> + int ret;
>
> if (!is_dma_buf_file(file))
> return -EINVAL;
> @@ -145,7 +146,11 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
> dmabuf->size >> PAGE_SHIFT)
> return -EINVAL;
>
> - return dmabuf->ops->mmap(dmabuf, vma);
> + ret = dmabuf->ops->mmap(dmabuf, vma);
> +
> + WARN_ON(!(vma->vm_flags & VM_PFNMAP));
Maybe change this to WARN_ON_ONCE(), so it doesn't fill up the kernel
log. Same comment below.
For either version
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Best regards
Thomas
> +
> + return ret;
> }
>
> static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence)
> @@ -1276,6 +1281,8 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access);
> int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
> unsigned long pgoff)
> {
> + int ret;
> +
> if (WARN_ON(!dmabuf || !vma))
> return -EINVAL;
>
> @@ -1296,7 +1303,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
> vma_set_file(vma, dmabuf->file);
> vma->vm_pgoff = pgoff;
>
> - return dmabuf->ops->mmap(dmabuf, vma);
> + ret = dmabuf->ops->mmap(dmabuf, vma);
> +
> + WARN_ON(!(vma->vm_flags & VM_PFNMAP));
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(dma_buf_mmap);
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-07-23 18:45 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-13 20:51 [Intel-gfx] [PATCH v4 0/4] shmem helpers for vgem Daniel Vetter
2021-07-13 20:51 ` [Intel-gfx] [PATCH v4 1/4] dma-buf: Require VM_PFNMAP vma for mmap Daniel Vetter
2021-07-23 18:45 ` Thomas Zimmermann [this message]
2021-07-13 20:51 ` [Intel-gfx] [PATCH v4 2/4] drm/shmem-helper: Switch to vmf_insert_pfn Daniel Vetter
2021-07-22 18:22 ` Thomas Zimmermann
2021-07-23 7:32 ` Daniel Vetter
2021-08-12 13:05 ` Daniel Vetter
2021-07-13 20:51 ` [Intel-gfx] [PATCH v4 3/4] drm/shmem-helpers: Allocate wc pages on x86 Daniel Vetter
2021-07-14 11:54 ` Christian König
2021-07-14 12:48 ` Daniel Vetter
2021-07-14 12:58 ` Christian König
2021-07-14 16:16 ` Daniel Vetter
2021-07-22 18:40 ` Thomas Zimmermann
2021-07-23 7:36 ` Daniel Vetter
2021-07-23 8:02 ` Christian König
2021-07-23 8:34 ` Daniel Vetter
2021-08-05 18:40 ` Thomas Zimmermann
2021-07-13 20:51 ` [Intel-gfx] [PATCH v4 4/4] drm/vgem: use shmem helpers Daniel Vetter
2021-07-14 12:45 ` [Intel-gfx] [PATCH] " Daniel Vetter
2021-07-22 18:50 ` [Intel-gfx] [PATCH v4 4/4] " Thomas Zimmermann
2021-07-23 7:38 ` Daniel Vetter
2021-07-13 23:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for shmem helpers for vgem (rev6) Patchwork
2021-07-14 0:11 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-07-16 13:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for shmem helpers for vgem (rev8) Patchwork
2021-07-16 13:58 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-07-16 16:43 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
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=7009d234-a19a-bc37-5b1f-9f11bd5db747@suse.de \
--to=tzimmermann@suse.de \
--cc=christian.koenig@amd.com \
--cc=daniel.vetter@ffwll.ch \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jgg@ziepe.ca \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-media@vger.kernel.org \
--cc=surenb@google.com \
--cc=willy@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).