add remap_pfn_range_notrack instead of reinventing it in i915 v2

add remap_pfn_range_notrack instead of reinventing it in i915 v2

[Christoph Hellwig]

Hi all,

i915 has some reason to want to avoid the track_pfn_remap overhead in
remap_pfn_range.  Add a function to the core VM to do just that rather
than reinventing the functionality poorly in the driver.

Note that the remap_io_sg path does get exercises when using Xorg on my
Thinkpad X1, so this should be considered lightly tested, I've not
managed to hit the remap_io_mapping path at all.

Changes since v1:
 - create a io_mapping_map_user wrapper instead of exporting
   remap_pfn_range_notrack
 - switch to plain remap_pfn_range for remap_sg as it does not use
   a pre-verified pgprot from an iomap

Diffstat:
 drivers/gpu/drm/i915/Kconfig             |    1 
 drivers/gpu/drm/i915/gem/i915_gem_mman.c |    9 +-
 drivers/gpu/drm/i915/i915_drv.h          |    3 
 drivers/gpu/drm/i915/i915_mm.c           |  117 ++++++-------------------------
 include/linux/io-mapping.h               |    3 
 include/linux/mm.h                       |    2 
 mm/Kconfig                               |    3 
 mm/Makefile                              |    1 
 mm/io-mapping.c                          |   29 +++++++
 mm/memory.c                              |   51 ++++++++-----
 10 files changed, 97 insertions(+), 122 deletions(-)

[PATCH 1/4] mm: add remap_pfn_range_notrack

[Christoph Hellwig]

Add a version of remap_pfn_range that does not call track_pfn_range.
This will be used to fix horrible abuses of VM internals in the i915
driver.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/mm.h |  2 ++
 mm/memory.c        | 51 ++++++++++++++++++++++++++++------------------
 2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 64a71bf2053674..70a6160d729ded 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2709,6 +2709,8 @@ unsigned long change_prot_numa(struct vm_area_struct *vma,
 struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
 int remap_pfn_range(struct vm_area_struct *, unsigned long addr,
 			unsigned long pfn, unsigned long size, pgprot_t);
+int remap_pfn_range_notrack(struct vm_area_struct *vma, unsigned long addr,
+		unsigned long pfn, unsigned long size, pgprot_t prot);
 int vm_insert_page(struct vm_area_struct *, unsigned long addr, struct page *);
 int vm_insert_pages(struct vm_area_struct *vma, unsigned long addr,
 			struct page **pages, unsigned long *num);
diff --git a/mm/memory.c b/mm/memory.c
index 5efa07fb6cdc18..997cda24c4a0be 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2260,26 +2260,17 @@ static inline int remap_p4d_range(struct mm_struct *mm, pgd_t *pgd,
 	return 0;
 }
 
-/**
- * remap_pfn_range - remap kernel memory to userspace
- * @vma: user vma to map to
- * @addr: target page aligned user address to start at
- * @pfn: page frame number of kernel physical memory address
- * @size: size of mapping area
- * @prot: page protection flags for this mapping
- *
- * Note: this is only safe if the mm semaphore is held when called.
- *
- * Return: %0 on success, negative error code otherwise.
+/*
+ * Variant of remap_pfn_range that does not call track_pfn_remap.  The caller
+ * must have pre-validated the caching bits of the pgprot_t.
  */
-int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
-		    unsigned long pfn, unsigned long size, pgprot_t prot)
+int remap_pfn_range_notrack(struct vm_area_struct *vma, unsigned long addr,
+		unsigned long pfn, unsigned long size, pgprot_t prot)
 {
 	pgd_t *pgd;
 	unsigned long next;
 	unsigned long end = addr + PAGE_ALIGN(size);
 	struct mm_struct *mm = vma->vm_mm;
-	unsigned long remap_pfn = pfn;
 	int err;
 
 	if (WARN_ON_ONCE(!PAGE_ALIGNED(addr)))
@@ -2309,10 +2300,6 @@ int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
 		vma->vm_pgoff = pfn;
 	}
 
-	err = track_pfn_remap(vma, &prot, remap_pfn, addr, PAGE_ALIGN(size));
-	if (err)
-		return -EINVAL;
-
 	vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
 
 	BUG_ON(addr >= end);
@@ -2324,12 +2311,36 @@ int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
 		err = remap_p4d_range(mm, pgd, addr, next,
 				pfn + (addr >> PAGE_SHIFT), prot);
 		if (err)
-			break;
+			return err;
 	} while (pgd++, addr = next, addr != end);
 
+	return 0;
+}
+
+/**
+ * remap_pfn_range - remap kernel memory to userspace
+ * @vma: user vma to map to
+ * @addr: target page aligned user address to start at
+ * @pfn: page frame number of kernel physical memory address
+ * @size: size of mapping area
+ * @prot: page protection flags for this mapping
+ *
+ * Note: this is only safe if the mm semaphore is held when called.
+ *
+ * Return: %0 on success, negative error code otherwise.
+ */
+int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
+		    unsigned long pfn, unsigned long size, pgprot_t prot)
+{
+	int err;
+
+	err = track_pfn_remap(vma, &prot, pfn, addr, PAGE_ALIGN(size));
 	if (err)
-		untrack_pfn(vma, remap_pfn, PAGE_ALIGN(size));
+		return -EINVAL;
 
+	err = remap_pfn_range_notrack(vma, addr, pfn, size, prot);
+	if (err)
+		untrack_pfn(vma, pfn, PAGE_ALIGN(size));
 	return err;
 }
 EXPORT_SYMBOL(remap_pfn_range);
-- 
2.30.1

[PATCH 2/4] mm: add a io_mapping_map_user helper

[Christoph Hellwig]

Add a helper that calls remap_pfn_range for an struct io_mapping, relying
on the pgprot pre-validation done when creating the mapping instead of
doing it at runtime.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/io-mapping.h |  3 +++
 mm/Kconfig                 |  3 +++
 mm/Makefile                |  1 +
 mm/io-mapping.c            | 29 +++++++++++++++++++++++++++++
 4 files changed, 36 insertions(+)
 create mode 100644 mm/io-mapping.c

diff --git a/include/linux/io-mapping.h b/include/linux/io-mapping.h
index c093e81310a9b3..e9743cfd858527 100644
--- a/include/linux/io-mapping.h
+++ b/include/linux/io-mapping.h
@@ -220,3 +220,6 @@ io_mapping_free(struct io_mapping *iomap)
 }
 
 #endif /* _LINUX_IO_MAPPING_H */
+
+int io_mapping_map_user(struct io_mapping *iomap, struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size);
diff --git a/mm/Kconfig b/mm/Kconfig
index 24c045b24b9506..6b0f2cfbfac0f3 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -872,4 +872,7 @@ config MAPPING_DIRTY_HELPERS
 config KMAP_LOCAL
 	bool
 
+# struct io_mapping based helper.  Selected by drivers that need them
+config IO_MAPPING
+	bool
 endmenu
diff --git a/mm/Makefile b/mm/Makefile
index 72227b24a61688..c0135e385984bb 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -120,3 +120,4 @@ obj-$(CONFIG_MEMFD_CREATE) += memfd.o
 obj-$(CONFIG_MAPPING_DIRTY_HELPERS) += mapping_dirty_helpers.o
 obj-$(CONFIG_PTDUMP_CORE) += ptdump.o
 obj-$(CONFIG_PAGE_REPORTING) += page_reporting.o
+obj-$(CONFIG_IO_MAPPING) += io-mapping.o
diff --git a/mm/io-mapping.c b/mm/io-mapping.c
new file mode 100644
index 00000000000000..01b3627999304e
--- /dev/null
+++ b/mm/io-mapping.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/mm.h>
+#include <linux/io-mapping.h>
+
+/**
+ * io_mapping_map_user - remap an I/O mapping to userspace
+ * @iomap: the source io_mapping
+ * @vma: user vma to map to
+ * @addr: target user address to start at
+ * @pfn: physical address of kernel memory
+ * @size: size of map area
+ *
+ *  Note: this is only safe if the mm semaphore is held when called.
+ */
+int io_mapping_map_user(struct io_mapping *iomap, struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size)
+{
+	vm_flags_t expected_flags = VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+
+	if (WARN_ON_ONCE((vma->vm_flags & expected_flags) != expected_flags))
+		return -EINVAL;
+
+	/* We rely on prevalidation of the io-mapping to skip track_pfn(). */
+	return remap_pfn_range_notrack(vma, addr, pfn, size,
+		__pgprot((pgprot_val(iomap->prot) & _PAGE_CACHE_MASK) |
+			 (pgprot_val(vma->vm_page_prot) & ~_PAGE_CACHE_MASK)));
+}
+EXPORT_SYMBOL_GPL(io_mapping_map_user);
-- 
2.30.1

Re: [Intel-gfx] [PATCH 2/4] mm: add a io_mapping_map_user helper

[Lucas De Marchi]

On Fri, Mar 26, 2021 at 06:55:03AM +0100, Christoph Hellwig wrote:
>Add a helper that calls remap_pfn_range for an struct io_mapping, relying
>on the pgprot pre-validation done when creating the mapping instead of
>doing it at runtime.
>
>Signed-off-by: Christoph Hellwig <hch@lst.de>
>---
> include/linux/io-mapping.h |  3 +++
> mm/Kconfig                 |  3 +++
> mm/Makefile                |  1 +
> mm/io-mapping.c            | 29 +++++++++++++++++++++++++++++
> 4 files changed, 36 insertions(+)
> create mode 100644 mm/io-mapping.c
>
>diff --git a/include/linux/io-mapping.h b/include/linux/io-mapping.h
>index c093e81310a9b3..e9743cfd858527 100644
>--- a/include/linux/io-mapping.h
>+++ b/include/linux/io-mapping.h
>@@ -220,3 +220,6 @@ io_mapping_free(struct io_mapping *iomap)
> }
>
> #endif /* _LINUX_IO_MAPPING_H */
>+
>+int io_mapping_map_user(struct io_mapping *iomap, struct vm_area_struct *vma,
>+		unsigned long addr, unsigned long pfn, unsigned long size);

I'm not sure what exactly brought me to check this, but while debugging
I noticed this outside the header guard. But then after some more checks I
saw nothing actually selects CONFIG_IO_MAPPING because commit using
it was reverted in commit 0e4fe0c9f2f9 ("Revert "i915: use io_mapping_map_user"")

Is this something we want to re-attempt moving to mm/ ?

thanks
Lucas De Marchi

>diff --git a/mm/Kconfig b/mm/Kconfig
>index 24c045b24b9506..6b0f2cfbfac0f3 100644
>--- a/mm/Kconfig
>+++ b/mm/Kconfig
>@@ -872,4 +872,7 @@ config MAPPING_DIRTY_HELPERS
> config KMAP_LOCAL
> 	bool
>
>+# struct io_mapping based helper.  Selected by drivers that need them
>+config IO_MAPPING
>+	bool
> endmenu
>diff --git a/mm/Makefile b/mm/Makefile
>index 72227b24a61688..c0135e385984bb 100644
>--- a/mm/Makefile
>+++ b/mm/Makefile
>@@ -120,3 +120,4 @@ obj-$(CONFIG_MEMFD_CREATE) += memfd.o
> obj-$(CONFIG_MAPPING_DIRTY_HELPERS) += mapping_dirty_helpers.o
> obj-$(CONFIG_PTDUMP_CORE) += ptdump.o
> obj-$(CONFIG_PAGE_REPORTING) += page_reporting.o
>+obj-$(CONFIG_IO_MAPPING) += io-mapping.o
>diff --git a/mm/io-mapping.c b/mm/io-mapping.c
>new file mode 100644
>index 00000000000000..01b3627999304e
>--- /dev/null
>+++ b/mm/io-mapping.c
>@@ -0,0 +1,29 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+
>+#include <linux/mm.h>
>+#include <linux/io-mapping.h>
>+
>+/**
>+ * io_mapping_map_user - remap an I/O mapping to userspace
>+ * @iomap: the source io_mapping
>+ * @vma: user vma to map to
>+ * @addr: target user address to start at
>+ * @pfn: physical address of kernel memory
>+ * @size: size of map area
>+ *
>+ *  Note: this is only safe if the mm semaphore is held when called.
>+ */
>+int io_mapping_map_user(struct io_mapping *iomap, struct vm_area_struct *vma,
>+		unsigned long addr, unsigned long pfn, unsigned long size)
>+{
>+	vm_flags_t expected_flags = VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
>+
>+	if (WARN_ON_ONCE((vma->vm_flags & expected_flags) != expected_flags))
>+		return -EINVAL;
>+
>+	/* We rely on prevalidation of the io-mapping to skip track_pfn(). */
>+	return remap_pfn_range_notrack(vma, addr, pfn, size,
>+		__pgprot((pgprot_val(iomap->prot) & _PAGE_CACHE_MASK) |
>+			 (pgprot_val(vma->vm_page_prot) & ~_PAGE_CACHE_MASK)));
>+}
>+EXPORT_SYMBOL_GPL(io_mapping_map_user);
>-- 
>2.30.1
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

Re: [Intel-gfx] [PATCH 2/4] mm: add a io_mapping_map_user helper

[Peter Zijlstra]

On Wed, Oct 20, 2021 at 08:40:05AM -0700, Lucas De Marchi wrote:
> On Fri, Mar 26, 2021 at 06:55:03AM +0100, Christoph Hellwig wrote:
> > Add a helper that calls remap_pfn_range for an struct io_mapping, relying
> > on the pgprot pre-validation done when creating the mapping instead of
> > doing it at runtime.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> > include/linux/io-mapping.h |  3 +++
> > mm/Kconfig                 |  3 +++
> > mm/Makefile                |  1 +
> > mm/io-mapping.c            | 29 +++++++++++++++++++++++++++++
> > 4 files changed, 36 insertions(+)
> > create mode 100644 mm/io-mapping.c
> > 
> > diff --git a/include/linux/io-mapping.h b/include/linux/io-mapping.h
> > index c093e81310a9b3..e9743cfd858527 100644
> > --- a/include/linux/io-mapping.h
> > +++ b/include/linux/io-mapping.h
> > @@ -220,3 +220,6 @@ io_mapping_free(struct io_mapping *iomap)
> > }
> > 
> > #endif /* _LINUX_IO_MAPPING_H */
> > +
> > +int io_mapping_map_user(struct io_mapping *iomap, struct vm_area_struct *vma,
> > +		unsigned long addr, unsigned long pfn, unsigned long size);
> 
> I'm not sure what exactly brought me to check this, but while debugging
> I noticed this outside the header guard. But then after some more checks I
> saw nothing actually selects CONFIG_IO_MAPPING because commit using
> it was reverted in commit 0e4fe0c9f2f9 ("Revert "i915: use io_mapping_map_user"")
> 
> Is this something we want to re-attempt moving to mm/ ?

Yes, it would be very good to unexport apply_to_page_range(), it's a
terrible interface to expose.

Re: [Intel-gfx] [PATCH 2/4] mm: add a io_mapping_map_user helper

[Christoph Hellwig]

On Wed, Oct 20, 2021 at 09:37:51PM +0200, Peter Zijlstra wrote:
> > I'm not sure what exactly brought me to check this, but while debugging
> > I noticed this outside the header guard. But then after some more checks I
> > saw nothing actually selects CONFIG_IO_MAPPING because commit using
> > it was reverted in commit 0e4fe0c9f2f9 ("Revert "i915: use io_mapping_map_user"")
> > 
> > Is this something we want to re-attempt moving to mm/ ?
> 
> Yes, it would be very good to unexport apply_to_page_range(), it's a
> terrible interface to expose.

Yes.  We need to get back to this rather sooner than later.  I'm a little
swamped unfortunately.

[PATCH 3/4] i915: use io_mapping_map_user

[Christoph Hellwig]

Replace the home-grown remap_io_mapping that abuses apply_to_page_range
with the proper io_mapping_map_user interface.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/gpu/drm/i915/Kconfig             |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_mman.c |  9 +++--
 drivers/gpu/drm/i915/i915_drv.h          |  3 --
 drivers/gpu/drm/i915/i915_mm.c           | 44 ------------------------
 4 files changed, 5 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 1e1cb245fca778..93f4d059fc89f6 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -20,6 +20,7 @@ config DRM_I915
 	select INPUT if ACPI
 	select ACPI_VIDEO if ACPI
 	select ACPI_BUTTON if ACPI
+	select IO_MAPPING
 	select SYNC_FILE
 	select IOSF_MBI
 	select CRC32
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index ec28a6cde49bdd..3ef734dd0a7c42 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -363,11 +363,10 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
 		goto err_unpin;
 
 	/* Finally, remap it using the new GTT offset */
-	ret = remap_io_mapping(area,
-			       area->vm_start + (vma->ggtt_view.partial.offset << PAGE_SHIFT),
-			       (ggtt->gmadr.start + vma->node.start) >> PAGE_SHIFT,
-			       min_t(u64, vma->size, area->vm_end - area->vm_start),
-			       &ggtt->iomap);
+	ret = io_mapping_map_user(&ggtt->iomap, area, area->vm_start +
+			(vma->ggtt_view.partial.offset << PAGE_SHIFT),
+			(ggtt->gmadr.start + vma->node.start) >> PAGE_SHIFT,
+			min_t(u64, vma->size, area->vm_end - area->vm_start));
 	if (ret)
 		goto err_fence;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cb62ddba203522..d6aaf0baa5c2fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1973,9 +1973,6 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
 
 /* i915_mm.c */
-int remap_io_mapping(struct vm_area_struct *vma,
-		     unsigned long addr, unsigned long pfn, unsigned long size,
-		     struct io_mapping *iomap);
 int remap_io_sg(struct vm_area_struct *vma,
 		unsigned long addr, unsigned long size,
 		struct scatterlist *sgl, resource_size_t iobase);
diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
index 666808cb3a3260..9a777b0ff59b05 100644
--- a/drivers/gpu/drm/i915/i915_mm.c
+++ b/drivers/gpu/drm/i915/i915_mm.c
@@ -37,17 +37,6 @@ struct remap_pfn {
 	resource_size_t iobase;
 };
 
-static int remap_pfn(pte_t *pte, unsigned long addr, void *data)
-{
-	struct remap_pfn *r = data;
-
-	/* Special PTE are not associated with any struct page */
-	set_pte_at(r->mm, addr, pte, pte_mkspecial(pfn_pte(r->pfn, r->prot)));
-	r->pfn++;
-
-	return 0;
-}
-
 #define use_dma(io) ((io) != -1)
 
 static inline unsigned long sgt_pfn(const struct remap_pfn *r)
@@ -77,40 +66,7 @@ static int remap_sg(pte_t *pte, unsigned long addr, void *data)
 	return 0;
 }
 
-/**
- * remap_io_mapping - remap an IO mapping to userspace
- * @vma: user vma to map to
- * @addr: target user address to start at
- * @pfn: physical address of kernel memory
- * @size: size of map area
- * @iomap: the source io_mapping
- *
- *  Note: this is only safe if the mm semaphore is held when called.
- */
-int remap_io_mapping(struct vm_area_struct *vma,
-		     unsigned long addr, unsigned long pfn, unsigned long size,
-		     struct io_mapping *iomap)
-{
-	struct remap_pfn r;
-	int err;
-
 #define EXPECTED_FLAGS (VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP)
-	GEM_BUG_ON((vma->vm_flags & EXPECTED_FLAGS) != EXPECTED_FLAGS);
-
-	/* We rely on prevalidation of the io-mapping to skip track_pfn(). */
-	r.mm = vma->vm_mm;
-	r.pfn = pfn;
-	r.prot = __pgprot((pgprot_val(iomap->prot) & _PAGE_CACHE_MASK) |
-			  (pgprot_val(vma->vm_page_prot) & ~_PAGE_CACHE_MASK));
-
-	err = apply_to_page_range(r.mm, addr, size, remap_pfn, &r);
-	if (unlikely(err)) {
-		zap_vma_ptes(vma, addr, (r.pfn - pfn) << PAGE_SHIFT);
-		return err;
-	}
-
-	return 0;
-}
 
 /**
  * remap_io_sg - remap an IO mapping to userspace
-- 
2.30.1

[PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Christoph Hellwig]

remap_io_sg claims that that the pgprot is pre-verified using an
io_mapping, but actually does not get passed an io_mapping and just
uses the pgprot in the VMA.  Remove the apply_to_page_range abuse
and just loop over remap_pfn_range for each segment.

Note: this could use io_mapping_map_user by passing an iomap to
remap_io_sg if the maintainers can verify that the pgprot in the
iomap in the only caller is indeed the desired one here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/gpu/drm/i915/i915_mm.c | 73 +++++++++++-----------------------
 1 file changed, 23 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
index 9a777b0ff59b05..4c8cd08c672d2d 100644
--- a/drivers/gpu/drm/i915/i915_mm.c
+++ b/drivers/gpu/drm/i915/i915_mm.c
@@ -28,46 +28,10 @@
 
 #include "i915_drv.h"
 
-struct remap_pfn {
-	struct mm_struct *mm;
-	unsigned long pfn;
-	pgprot_t prot;
-
-	struct sgt_iter sgt;
-	resource_size_t iobase;
-};
+#define EXPECTED_FLAGS (VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP)
 
 #define use_dma(io) ((io) != -1)
 
-static inline unsigned long sgt_pfn(const struct remap_pfn *r)
-{
-	if (use_dma(r->iobase))
-		return (r->sgt.dma + r->sgt.curr + r->iobase) >> PAGE_SHIFT;
-	else
-		return r->sgt.pfn + (r->sgt.curr >> PAGE_SHIFT);
-}
-
-static int remap_sg(pte_t *pte, unsigned long addr, void *data)
-{
-	struct remap_pfn *r = data;
-
-	if (GEM_WARN_ON(!r->sgt.sgp))
-		return -EINVAL;
-
-	/* Special PTE are not associated with any struct page */
-	set_pte_at(r->mm, addr, pte,
-		   pte_mkspecial(pfn_pte(sgt_pfn(r), r->prot)));
-	r->pfn++; /* track insertions in case we need to unwind later */
-
-	r->sgt.curr += PAGE_SIZE;
-	if (r->sgt.curr >= r->sgt.max)
-		r->sgt = __sgt_iter(__sg_next(r->sgt.sgp), use_dma(r->iobase));
-
-	return 0;
-}
-
-#define EXPECTED_FLAGS (VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP)
-
 /**
  * remap_io_sg - remap an IO mapping to userspace
  * @vma: user vma to map to
@@ -82,12 +46,7 @@ int remap_io_sg(struct vm_area_struct *vma,
 		unsigned long addr, unsigned long size,
 		struct scatterlist *sgl, resource_size_t iobase)
 {
-	struct remap_pfn r = {
-		.mm = vma->vm_mm,
-		.prot = vma->vm_page_prot,
-		.sgt = __sgt_iter(sgl, use_dma(iobase)),
-		.iobase = iobase,
-	};
+	unsigned long pfn, len, remapped = 0;
 	int err;
 
 	/* We rely on prevalidation of the io-mapping to skip track_pfn(). */
@@ -96,11 +55,25 @@ int remap_io_sg(struct vm_area_struct *vma,
 	if (!use_dma(iobase))
 		flush_cache_range(vma, addr, size);
 
-	err = apply_to_page_range(r.mm, addr, size, remap_sg, &r);
-	if (unlikely(err)) {
-		zap_vma_ptes(vma, addr, r.pfn << PAGE_SHIFT);
-		return err;
-	}
-
-	return 0;
+	do {
+		if (use_dma(iobase)) {
+			if (!sg_dma_len(sgl))
+				break;
+			pfn = (sg_dma_address(sgl) + iobase) >> PAGE_SHIFT;
+			len = sg_dma_len(sgl);
+		} else {
+			pfn = page_to_pfn(sg_page(sgl));
+			len = sgl->length;
+		}
+
+		err = remap_pfn_range(vma, addr + remapped, pfn, len,
+				      vma->vm_page_prot);
+		if (err)
+			break;
+		remapped += len;
+	} while ((sgl = __sg_next(sgl)));
+
+	if (err)
+		zap_vma_ptes(vma, addr, remapped);
+	return err;
 }
-- 
2.30.1

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[youling257]

This patch cause "x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x064a2000-0x064a2fff], got write-back" problem.
my 2GB ram Bay trail z3735f tablet runing on android-x86, "i915: fix remap_io_sg to verify the pgprot" cause this problem.

05-09 02:59:25.099     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x0640a000-0x0640dfff], got write-back
05-09 02:59:25.106  1440  1440 W hwc-gl-worker: EGL_ANDROID_native_fence_sync extension not supported
05-09 02:59:25.111     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x064a2000-0x064a2fff], got write-back
05-09 02:59:25.118     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06400000-0x06404fff], got write-back
05-09 02:59:25.125     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06405000-0x06408fff], got write-back
05-09 02:59:25.148  1440  1440 W hwc-gl-worker: EGL_ANDROID_native_fence_sync extension not supported
05-09 02:59:25.158     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06542000-0x06542fff], got write-back
05-09 02:59:25.165     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06499000-0x0649dfff], got write-back
05-09 02:59:25.171     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x0649e000-0x064a1fff], got write-back
05-09 02:59:25.177  1440  1440 W hwc-gl-worker: EGL_ANDROID_native_fence_sync extension not supported
05-09 02:59:25.183     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x065fa000-0x065fafff], got write-back
05-09 02:59:25.192     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06539000-0x0653dfff], got write-back
05-09 02:59:25.199     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x0653e000-0x06541fff], got write-back
05-09 02:59:25.204  1440  1440 W hwc-gl-worker: EGL_ANDROID_native_fence_sync extension not supported
05-09 02:59:25.212     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x066a2000-0x066a2fff], got write-back
05-09 02:59:25.218     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x065f1000-0x065f5fff], got write-back
05-09 02:59:25.226     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x065f6000-0x065f9fff], got write-back


05-09 02:59:27.101     0     0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x08a76000-0x08a76fff], got write-back
05-09 02:59:27.225     0     0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x08a77000-0x08a7afff], got write-back
05-09 02:59:27.242     0     0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x08bd0000-0x08bd0fff], got write-back
05-09 02:59:27.254     0     0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x08bd1000-0x08bf0fff], got write-back
05-09 02:59:27.310  1440  1440 E drm-fb  : Failed to get handle from prime fd: 25
05-09 02:59:27.322     0     0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x080d5000-0x080d9fff], got write-back
05-09 02:59:27.322     0     0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x080da000-0x080ddfff], got write-back
05-09 02:59:27.338     0     0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x1b830000-0x1b83ffff], got write-back
05-09 02:59:27.338     0     0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x1b76a000-0x1b76efff], got write-back
05-09 02:59:27.344     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x07e87000-0x07e8bfff], got write-back
05-09 02:59:27.349     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x07e8c000-0x07e90fff], got write-back
05-09 02:59:27.347  1440  1440 E drm-fb  : Failed to get handle from prime fd: 25
05-09 02:59:27.361     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c123000-0x1c126fff], got write-back
05-09 02:59:27.361     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c127000-0x1c12afff], got write-back
05-09 02:59:27.362     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c12f000-0x1c13efff], got write-back
05-09 02:59:27.362     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c12b000-0x1c12efff], got write-back
05-09 02:59:27.364  1440  1440 E drm-fb  : Failed to get handle from prime fd: 25
05-09 02:59:27.377     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c140000-0x1c144fff], got write-back
05-09 02:59:27.377     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c145000-0x1c148fff], got write-back
05-09 02:59:27.378     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c14b000-0x1c14ffff], got write-back
05-09 02:59:27.379     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c151000-0x1c155fff], got write-back
05-09 02:59:27.377  1440  1440 E drm-fb  : Failed to get handle from prime fd: 25
05-09 02:59:27.393     0     0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c157000-0x1c15bfff], got write-back

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Christoph Hellwig]

On Sun, May 09, 2021 at 03:33:29AM +0800, youling257 wrote:
> This patch cause "x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x064a2000-0x064a2fff], got write-back" problem.
> my 2GB ram Bay trail z3735f tablet runing on android-x86, "i915: fix remap_io_sg to verify the pgprot" cause this problem.

So this is the memtype verification added by the patch, meaning that the
old code did in fact not call into track_pfn_remap with the right flags.

Can the i915 maintainers take a look at making sure the page permissions
here make sense?

Re: [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Serge Belyshev]

I have another problem with this patch since it landed in mainline. On
my m3-6Y30 skylake HD Graphics 515 (rev 07), it causes visual artifacts
that look like bunch of one pixel high horizontal streaks, seen most
often in firefox while scrolling or in menu controls.

Reverting this patch on top of current mainline fixes the problem.

Re: [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Christoph Hellwig]

As an ad-hoc experiment:  can you replace the call to remap_pfn_range
with remap_pfn_range_notrack (and export it if you build i915 modular)
in remap_io_sg and see if that makes any difference?

Re: [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Serge Belyshev]

Christoph Hellwig <hch@lst.de> writes:

> As an ad-hoc experiment:  can you replace the call to remap_pfn_range
> with remap_pfn_range_notrack (and export it if you build i915 modular)
> in remap_io_sg and see if that makes any difference?

That worked, thanks -- no artifacts seen.

Re: [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Christoph Hellwig]

On Mon, May 17, 2021 at 04:09:42PM +0300, Serge Belyshev wrote:
> Christoph Hellwig <hch@lst.de> writes:
> 
> > As an ad-hoc experiment:  can you replace the call to remap_pfn_range
> > with remap_pfn_range_notrack (and export it if you build i915 modular)
> > in remap_io_sg and see if that makes any difference?
> 
> That worked, thanks -- no artifacts seen.

Looks like it is caused by the validation failure then.  Which means the
existing code is doing something wrong in its choice of the page
protection bit.  I really need help from the i915 maintainers here..

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Matthew Auld]

On Mon, 17 May 2021 at 14:11, Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, May 17, 2021 at 04:09:42PM +0300, Serge Belyshev wrote:
> > Christoph Hellwig <hch@lst.de> writes:
> >
> > > As an ad-hoc experiment:  can you replace the call to remap_pfn_range
> > > with remap_pfn_range_notrack (and export it if you build i915 modular)
> > > in remap_io_sg and see if that makes any difference?
> >
> > That worked, thanks -- no artifacts seen.
>
> Looks like it is caused by the validation failure then.  Which means the
> existing code is doing something wrong in its choice of the page
> protection bit.  I really need help from the i915 maintainers here..

AFAIK there are two users of remap_io_sg, the first is our shmem
objects(see i915_gem_shmem.c), and for these we support UC, WC, and WB
mmap modes for userspace. The other user is device local-memory
objects(VRAM), and for this one we have an actual io_mapping which is
allocated as WC, and IIRC this should only be mapped as WC for the
mmap mode, but normal userspace can't hit this path yet.

What do we need to do here? It sounds like shmem backed objects are
allocated as WB for the pages underneath, but i915 allows mapping them
as UC/WC which trips up this track_pfn thing?

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Christoph Hellwig]

On Mon, May 17, 2021 at 06:06:44PM +0100, Matthew Auld wrote:
> > Looks like it is caused by the validation failure then.  Which means the
> > existing code is doing something wrong in its choice of the page
> > protection bit.  I really need help from the i915 maintainers here..
> 
> AFAIK there are two users of remap_io_sg, the first is our shmem
> objects(see i915_gem_shmem.c), and for these we support UC, WC, and WB
> mmap modes for userspace. The other user is device local-memory
> objects(VRAM), and for this one we have an actual io_mapping which is
> allocated as WC, and IIRC this should only be mapped as WC for the
> mmap mode, but normal userspace can't hit this path yet.

The only caller in current mainline is vm_fault_cpu in i915_gem_mman.c.
Is that device local?

> What do we need to do here? It sounds like shmem backed objects are
> allocated as WB for the pages underneath, but i915 allows mapping them
> as UC/WC which trips up this track_pfn thing?

To me the warnings looks like system memory is mapped with the wrong
permissions, yes.  If you want to map it as UC/WC the right set_memory_*
needs to be used on the kernel mapping as well to ensure that the
attributes don't conflict.

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Matthew Auld]

On Tue, 18 May 2021 at 14:21, Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, May 17, 2021 at 06:06:44PM +0100, Matthew Auld wrote:
> > > Looks like it is caused by the validation failure then.  Which means the
> > > existing code is doing something wrong in its choice of the page
> > > protection bit.  I really need help from the i915 maintainers here..
> >
> > AFAIK there are two users of remap_io_sg, the first is our shmem
> > objects(see i915_gem_shmem.c), and for these we support UC, WC, and WB
> > mmap modes for userspace. The other user is device local-memory
> > objects(VRAM), and for this one we have an actual io_mapping which is
> > allocated as WC, and IIRC this should only be mapped as WC for the
> > mmap mode, but normal userspace can't hit this path yet.
>
> The only caller in current mainline is vm_fault_cpu in i915_gem_mman.c.
> Is that device local?

The vm_fault_cpu covers both device local and shmem objects.

>
> > What do we need to do here? It sounds like shmem backed objects are
> > allocated as WB for the pages underneath, but i915 allows mapping them
> > as UC/WC which trips up this track_pfn thing?
>
> To me the warnings looks like system memory is mapped with the wrong
> permissions, yes.  If you want to map it as UC/WC the right set_memory_*
> needs to be used on the kernel mapping as well to ensure that the
> attributes don't conflict.

AFAIK mmap_offset also supports multiple active mmap modes for a given
object, so set_memory_* should still work here?

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Thomas Hellström (Intel)]

On 5/18/21 5:00 PM, Matthew Auld wrote:
> On Tue, 18 May 2021 at 14:21, Christoph Hellwig <hch@lst.de> wrote:
>> On Mon, May 17, 2021 at 06:06:44PM +0100, Matthew Auld wrote:
>>>> Looks like it is caused by the validation failure then.  Which means the
>>>> existing code is doing something wrong in its choice of the page
>>>> protection bit.  I really need help from the i915 maintainers here..
>>> AFAIK there are two users of remap_io_sg, the first is our shmem
>>> objects(see i915_gem_shmem.c), and for these we support UC, WC, and WB
>>> mmap modes for userspace. The other user is device local-memory
>>> objects(VRAM), and for this one we have an actual io_mapping which is
>>> allocated as WC, and IIRC this should only be mapped as WC for the
>>> mmap mode, but normal userspace can't hit this path yet.
>> The only caller in current mainline is vm_fault_cpu in i915_gem_mman.c.
>> Is that device local?
> The vm_fault_cpu covers both device local and shmem objects.
>
>>> What do we need to do here? It sounds like shmem backed objects are
>>> allocated as WB for the pages underneath, but i915 allows mapping them
>>> as UC/WC which trips up this track_pfn thing?
>> To me the warnings looks like system memory is mapped with the wrong
>> permissions, yes.  If you want to map it as UC/WC the right set_memory_*
>> needs to be used on the kernel mapping as well to ensure that the
>> attributes don't conflict.
> AFAIK mmap_offset also supports multiple active mmap modes for a given
> object, so set_memory_* should still work here?

No, that won't work because there are active maps with conflicting 
caching attributes. I think the history here is that that was assumed to 
be OK for integrated graphics that ran only on Intel processors that 
promise to never write back unmodified cache lines resulting from 
prefetching, like some AMD processors did way back at least.

These conflicting mappings can obviously not be supported for discrete 
graphics, but for integrated they are part of the uAPI.

/Thomas





> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Thomas Hellström]

On 5/17/21 3:11 PM, Christoph Hellwig wrote:
> On Mon, May 17, 2021 at 04:09:42PM +0300, Serge Belyshev wrote:
>> Christoph Hellwig <hch@lst.de> writes:
>>
>>> As an ad-hoc experiment:  can you replace the call to remap_pfn_range
>>> with remap_pfn_range_notrack (and export it if you build i915 modular)
>>> in remap_io_sg and see if that makes any difference?
>> That worked, thanks -- no artifacts seen.
> Looks like it is caused by the validation failure then.  Which means the
> existing code is doing something wrong in its choice of the page
> protection bit.  I really need help from the i915 maintainers here..

Hmm,

Apart from the caching aliasing Mattew brought up, doesn't the 
remap_pfn_range_xxx() family require the mmap_sem held in write mode 
since it modifies the vma structure? remap_io_sg() is called from the 
fault handler with the mmap_sem held in read mode only.

/Thomas

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Thomas Hellström]

On 5/17/21 11:46 PM, Thomas Hellström wrote:
>
> On 5/17/21 3:11 PM, Christoph Hellwig wrote:
>> On Mon, May 17, 2021 at 04:09:42PM +0300, Serge Belyshev wrote:
>>> Christoph Hellwig <hch@lst.de> writes:
>>>
>>>> As an ad-hoc experiment:  can you replace the call to remap_pfn_range
>>>> with remap_pfn_range_notrack (and export it if you build i915 modular)
>>>> in remap_io_sg and see if that makes any difference?
>>> That worked, thanks -- no artifacts seen.
>> Looks like it is caused by the validation failure then.  Which means the
>> existing code is doing something wrong in its choice of the page
>> protection bit.  I really need help from the i915 maintainers here..
>
> Hmm,
>
> Apart from the caching aliasing Mattew brought up, doesn't the 
> remap_pfn_range_xxx() family require the mmap_sem held in write mode 
> since it modifies the vma structure? remap_io_sg() is called from the 
> fault handler with the mmap_sem held in read mode only.
>
> /Thomas

And worse, if we prefault a user-space buffer object map using 
remap_io_sg() and then zap some ptes using madvise(), the next time 
those ptes are accessed, we'd trigger a new call to remap_io_sg() which 
would now find already populated ptes. While the old code looks to just 
silently overwrite those, it looks like the new code would BUG in 
remap_pte_range()?

/Thomas




>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Christoph Hellwig]

On Tue, May 18, 2021 at 08:46:44AM +0200, Thomas Hellström wrote:
> And worse, if we prefault a user-space buffer object map using 
> remap_io_sg() and then zap some ptes using madvise(), the next time those 
> ptes are accessed, we'd trigger a new call to remap_io_sg() which would now 
> find already populated ptes. While the old code looks to just silently 
> overwrite those, it looks like the new code would BUG in remap_pte_range()?

How can you zap the PTEs using madvise?

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Thomas Hellström]

On 5/18/21 3:24 PM, Christoph Hellwig wrote:
> On Tue, May 18, 2021 at 08:46:44AM +0200, Thomas Hellström wrote:
>> And worse, if we prefault a user-space buffer object map using
>> remap_io_sg() and then zap some ptes using madvise(), the next time those
>> ptes are accessed, we'd trigger a new call to remap_io_sg() which would now
>> find already populated ptes. While the old code looks to just silently
>> overwrite those, it looks like the new code would BUG in remap_pte_range()?
> How can you zap the PTEs using madvise?

Hmm, that's not possible with VM_PFNMAP. My bad. Should be OK then.

/Thomas

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Christoph Hellwig]

On Mon, May 17, 2021 at 11:46:35PM +0200, Thomas Hellström wrote:
> Apart from the caching aliasing Mattew brought up, doesn't the 
> remap_pfn_range_xxx() family require the mmap_sem held in write mode since 
> it modifies the vma structure? remap_io_sg() is called from the fault 
> handler with the mmap_sem held in read mode only.

Only for vma->vm_flags, and remap_sg already asserts all the interesting
flags are set, although it does not assert VM_IO.

We could move the assignment out of remap_pfn_range_notrack and
into remap_pfn_range and just assert that the proper flags are set,
though.

Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot

[Thomas Hellström]

On 5/18/21 3:23 PM, Christoph Hellwig wrote:
> On Mon, May 17, 2021 at 11:46:35PM +0200, Thomas Hellström wrote:
>> Apart from the caching aliasing Mattew brought up, doesn't the
>> remap_pfn_range_xxx() family require the mmap_sem held in write mode since
>> it modifies the vma structure? remap_io_sg() is called from the fault
>> handler with the mmap_sem held in read mode only.
> Only for vma->vm_flags, and remap_sg already asserts all the interesting
> flags are set, although it does not assert VM_IO.
>
> We could move the assignment out of remap_pfn_range_notrack and
> into remap_pfn_range and just assert that the proper flags are set,
> though.

That to me sounds like a way forward. It sound like in general a gpu 
prefaulting helper that in the long run also supports faulting huge ptes 
is desired also by TTM. Although it looks like that BUG_ON() I pointed 
out was hit anyway....

/Thomas

Re: add remap_pfn_range_notrack instead of reinventing it in i915 v2

[Daniel Vetter]

On Fri, Mar 26, 2021 at 06:55:01AM +0100, Christoph Hellwig wrote:
> Hi all,
> 
> i915 has some reason to want to avoid the track_pfn_remap overhead in
> remap_pfn_range.  Add a function to the core VM to do just that rather
> than reinventing the functionality poorly in the driver.
> 
> Note that the remap_io_sg path does get exercises when using Xorg on my
> Thinkpad X1, so this should be considered lightly tested, I've not
> managed to hit the remap_io_mapping path at all.
> 
> Changes since v1:
>  - create a io_mapping_map_user wrapper instead of exporting
>    remap_pfn_range_notrack
>  - switch to plain remap_pfn_range for remap_sg as it does not use
>    a pre-verified pgprot from an iomap

I'm burried under patches and stuff so no in-depth review. But from a
quick scan lgtm. On the series:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I've seen it's landed already in some tree, maybe if you can add the acks
ftr?

Cheers, Daniel

> 
> Diffstat:
>  drivers/gpu/drm/i915/Kconfig             |    1 
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c |    9 +-
>  drivers/gpu/drm/i915/i915_drv.h          |    3 
>  drivers/gpu/drm/i915/i915_mm.c           |  117 ++++++-------------------------
>  include/linux/io-mapping.h               |    3 
>  include/linux/mm.h                       |    2 
>  mm/Kconfig                               |    3 
>  mm/Makefile                              |    1 
>  mm/io-mapping.c                          |   29 +++++++
>  mm/memory.c                              |   51 ++++++++-----
>  10 files changed, 97 insertions(+), 122 deletions(-)

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: add remap_pfn_range_notrack instead of reinventing it in i915 v2

[Christoph Hellwig]

On Thu, Apr 08, 2021 at 12:36:21PM +0200, Daniel Vetter wrote:
> I've seen it's landed already in some tree, maybe if you can add the acks
> ftr?

Andrew has picked it up in -mm.