drm/shmem-helper: Switch to vmf_insert_pfn
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 12 Aug 2021 13:14:10 +0000 (15:14 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 12 Aug 2021 19:41:10 +0000 (21:41 +0200)
We want to stop gup, which isn't the case if we use vmf_insert_page
and VM_MIXEDMAP, because that does not set pte_special.

The motivation here is to stop get_user_pages from working on buffer
object mmaps in general. Quoting some discussion with Thomas:

On Thu, Jul 22, 2021 at 08:22:43PM +0200, Thomas Zimmermann wrote:
> Am 13.07.21 um 22:51 schrieb Daniel Vetter:
> > We want to stop gup, which isn't the case if we use vmf_insert_page
>
> What is gup?

get_user_pages. It pins memory wherever it is, which badly wreaks at least
ttm and could also cause trouble with cma allocations. In both cases
becaue we can't move/reuse these pages anymore.

Now get_user_pages fails when the memory isn't considered "normal", like
with VM_PFNMAP and using vm_insert_pfn. For consistency across all dma-buf
I'm trying (together with Christian König) to roll this out everywhere,
for fewer surprises.

E.g. for 5.14 iirc we merged a patch to do the same for ttm, where it
closes an actual bug (ttm gets really badly confused when there's suddenly
pinned pages where it thought it can move them).

cma allcoations already use VM_PFNMAP (because that's what dma_mmap is
using underneath), as is anything that's using remap_pfn_range. Worst case
we have to revert this patch for shmem helpers if it breaks something, but
I hope that's not the case. On the ttm side we've also had some fallout
that we needed to paper over with clever tricks.
v2: With this shmem gem helpers now definitely need CONFIG_MMU (0day)

v3: add more depends on MMU. For usb drivers this is a bit awkward,
but really it's correct: To be able to provide a contig mapping of
buffers to userspace on !MMU platforms we'd need to use the cma
helpers for these drivers on those platforms. As-is this wont work.

Also not exactly sure why vm_insert_page doesn't go boom, because that
definitely wont fly in practice since the pages are non-contig to
begin with.

v4: Explain the entire motivation a lot more (Thomas)

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20210812131412.2487363-2-daniel.vetter@ffwll.ch
drivers/gpu/drm/Kconfig
drivers/gpu/drm/drm_gem_shmem_helper.c
drivers/gpu/drm/gud/Kconfig
drivers/gpu/drm/tiny/Kconfig
drivers/gpu/drm/udl/Kconfig

index 0d372354c2d0402fd5a5b650460eb2efa557c763..314eefa398922178121bbef24a7113875a9d1f28 100644 (file)
@@ -211,7 +211,7 @@ config DRM_KMS_CMA_HELPER
 
 config DRM_GEM_SHMEM_HELPER
        bool
-       depends on DRM
+       depends on DRM && MMU
        help
          Choose this if you need the GEM shmem helper functions
 
index a61946374c8262fb3253ef0051e5baa04a373d7c..cc96d1c3570e9d48b86daabfafdda078b3f3dca7 100644 (file)
@@ -542,7 +542,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
        } else {
                page = shmem->pages[page_offset];
 
-               ret = vmf_insert_page(vma, vmf->address, page);
+               ret = vmf_insert_pfn(vma, vmf->address, page_to_pfn(page));
        }
 
        mutex_unlock(&shmem->pages_lock);
@@ -612,7 +612,7 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
                return ret;
        }
 
-       vma->vm_flags |= VM_MIXEDMAP | VM_DONTEXPAND;
+       vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND;
        vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
        if (shmem->map_wc)
                vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
index 1c8601bf4d91e581fbb3ca516b541cdac275a4a9..9c1e61f9eec3063569030dc8bfd6069326d1c5a8 100644 (file)
@@ -2,7 +2,7 @@
 
 config DRM_GUD
        tristate "GUD USB Display"
-       depends on DRM && USB
+       depends on DRM && USB && MMU
        select LZ4_COMPRESS
        select DRM_KMS_HELPER
        select DRM_GEM_SHMEM_HELPER
index d31be274a2bd98d0c394098ed99fefc430ba7977..1ceb93fbdc504d279ef99a4788af95ce41fb8117 100644 (file)
@@ -44,7 +44,7 @@ config DRM_CIRRUS_QEMU
 
 config DRM_GM12U320
        tristate "GM12U320 driver for USB projectors"
-       depends on DRM && USB
+       depends on DRM && USB && MMU
        select DRM_KMS_HELPER
        select DRM_GEM_SHMEM_HELPER
        help
@@ -53,7 +53,7 @@ config DRM_GM12U320
 
 config DRM_SIMPLEDRM
        tristate "Simple framebuffer driver"
-       depends on DRM
+       depends on DRM && MMU
        select DRM_GEM_SHMEM_HELPER
        select DRM_KMS_HELPER
        help
index 1f497d8f1ae5d085f0c916c82317635fed37ba19..c744175c6992c862ce7e44f353e835083da8548e 100644 (file)
@@ -4,6 +4,7 @@ config DRM_UDL
        depends on DRM
        depends on USB
        depends on USB_ARCH_HAS_HCD
+       depends on MMU
        select DRM_GEM_SHMEM_HELPER
        select DRM_KMS_HELPER
        help