* FAILED: patch "[PATCH] drm/i915/gem: Fix Virtual Memory mapping boundaries" failed to apply to 4.19-stable tree
@ 2024-08-12 9:26 gregkh
2024-08-13 14:14 ` [PATCH 4.19.y] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation Andi Shyti
2024-08-13 17:09 ` [PATCH 4.19.y v2] " Andi Shyti
0 siblings, 2 replies; 7+ messages in thread
From: gregkh @ 2024-08-12 9:26 UTC (permalink / raw)
To: andi.shyti, Jonathan.cavitt, chris.p.wilson, jannh,
joonas.lahtinen, matthew.auld, rodrigo.vivi, stable
Cc: stable
The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-4.19.y
git checkout FETCH_HEAD
git cherry-pick -x 8bdd9ef7e9b1b2a73e394712b72b22055e0e26c3
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024081222-process-suspect-d983@gregkh' --subject-prefix 'PATCH 4.19.y' HEAD^..
Possible dependencies:
8bdd9ef7e9b1 ("drm/i915/gem: Fix Virtual Memory mapping boundaries calculation")
8e4ee5e87ce6 ("drm/i915: Wrap all access to i915_vma.node.start|size")
3bb6a44251b4 ("drm/i915: Rename ggtt_view as gtt_view")
d976521a995a ("drm/i915: extend i915_vma_pin_iomap()")
d63ddca7c581 ("drm/i915: Update tiled blits selftest")
a0ed9c95cce6 ("drm/i915/gt: Use XY_FAST_COLOR_BLT to clear obj on graphics ver 12+")
fd5803e5eebe ("drm/i915/gt: use engine instance directly for offset")
892bfb8a604d ("drm/i915/fbdev: fixup setting screen_size")
c674c5b9342e ("drm/i915/xehp: CCS should use RCS setup functions")
30b9d1b3ef37 ("drm/i915: add I915_BO_ALLOC_GPU_ONLY")
3312a4ac8a46 ("drm/i915/ttm: require mappable by default")
8fbf28934acf ("drm/i915/ttm: fixup the mock_bo")
db927686e43f ("Merge drm/drm-next into drm-intel-gt-next")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 8bdd9ef7e9b1b2a73e394712b72b22055e0e26c3 Mon Sep 17 00:00:00 2001
From: Andi Shyti <andi.shyti@linux.intel.com>
Date: Fri, 2 Aug 2024 10:38:50 +0200
Subject: [PATCH] drm/i915/gem: Fix Virtual Memory mapping boundaries
calculation
Calculating the size of the mapped area as the lesser value
between the requested size and the actual size does not consider
the partial mapping offset. This can cause page fault access.
Fix the calculation of the starting and ending addresses, the
total size is now deduced from the difference between the end and
start addresses.
Additionally, the calculations have been rewritten in a clearer
and more understandable form.
Fixes: c58305af1835 ("drm/i915: Use remap_io_mapping() to prefault all PTE in a single pass")
Reported-by: Jann Horn <jannh@google.com>
Co-developed-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: <stable@vger.kernel.org> # v4.9+
Reviewed-by: Jann Horn <jannh@google.com>
Reviewed-by: Jonathan Cavitt <Jonathan.cavitt@intel.com>
[Joonas: Add Requires: tag]
Requires: 60a2066c5005 ("drm/i915/gem: Adjust vma offset for framebuffer mmap offset")
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240802083850.103694-3-andi.shyti@linux.intel.com
(cherry picked from commit 97b6784753da06d9d40232328efc5c5367e53417)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index ce10dd259812..cac6d4184506 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -290,6 +290,41 @@ static vm_fault_t vm_fault_cpu(struct vm_fault *vmf)
return i915_error_to_vmf_fault(err);
}
+static void set_address_limits(struct vm_area_struct *area,
+ struct i915_vma *vma,
+ unsigned long obj_offset,
+ unsigned long *start_vaddr,
+ unsigned long *end_vaddr)
+{
+ unsigned long vm_start, vm_end, vma_size; /* user's memory parameters */
+ long start, end; /* memory boundaries */
+
+ /*
+ * Let's move into the ">> PAGE_SHIFT"
+ * domain to be sure not to lose bits
+ */
+ vm_start = area->vm_start >> PAGE_SHIFT;
+ vm_end = area->vm_end >> PAGE_SHIFT;
+ vma_size = vma->size >> PAGE_SHIFT;
+
+ /*
+ * Calculate the memory boundaries by considering the offset
+ * provided by the user during memory mapping and the offset
+ * provided for the partial mapping.
+ */
+ start = vm_start;
+ start -= obj_offset;
+ start += vma->gtt_view.partial.offset;
+ end = start + vma_size;
+
+ start = max_t(long, start, vm_start);
+ end = min_t(long, end, vm_end);
+
+ /* Let's move back into the "<< PAGE_SHIFT" domain */
+ *start_vaddr = (unsigned long)start << PAGE_SHIFT;
+ *end_vaddr = (unsigned long)end << PAGE_SHIFT;
+}
+
static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
@@ -302,14 +337,18 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
bool write = area->vm_flags & VM_WRITE;
struct i915_gem_ww_ctx ww;
+ unsigned long obj_offset;
+ unsigned long start, end; /* memory boundaries */
intel_wakeref_t wakeref;
struct i915_vma *vma;
pgoff_t page_offset;
+ unsigned long pfn;
int srcu;
int ret;
- /* We don't use vmf->pgoff since that has the fake offset */
+ obj_offset = area->vm_pgoff - drm_vma_node_start(&mmo->vma_node);
page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
+ page_offset += obj_offset;
trace_i915_gem_object_fault(obj, page_offset, true, write);
@@ -402,12 +441,14 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
if (ret)
goto err_unpin;
+ set_address_limits(area, vma, obj_offset, &start, &end);
+
+ pfn = (ggtt->gmadr.start + i915_ggtt_offset(vma)) >> PAGE_SHIFT;
+ pfn += (start - area->vm_start) >> PAGE_SHIFT;
+ pfn += obj_offset - vma->gtt_view.partial.offset;
+
/* Finally, remap it using the new GTT offset */
- ret = remap_io_mapping(area,
- area->vm_start + (vma->gtt_view.partial.offset << PAGE_SHIFT),
- (ggtt->gmadr.start + i915_ggtt_offset(vma)) >> PAGE_SHIFT,
- min_t(u64, vma->size, area->vm_end - area->vm_start),
- &ggtt->iomap);
+ ret = remap_io_mapping(area, start, pfn, end - start, &ggtt->iomap);
if (ret)
goto err_fence;
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4.19.y] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation
2024-08-12 9:26 FAILED: patch "[PATCH] drm/i915/gem: Fix Virtual Memory mapping boundaries" failed to apply to 4.19-stable tree gregkh
@ 2024-08-13 14:14 ` Andi Shyti
2024-08-13 15:28 ` Greg KH
2024-08-13 17:09 ` [PATCH 4.19.y v2] " Andi Shyti
1 sibling, 1 reply; 7+ messages in thread
From: Andi Shyti @ 2024-08-13 14:14 UTC (permalink / raw)
To: stable
Cc: Andi Shyti, Jann Horn, Chris Wilson, Joonas Lahtinen,
Matthew Auld, Rodrigo Vivi, Jonathan Cavitt
Commit 8bdd9ef7e9b1b2a73e394712b72b22055e0e26c3 upstream.
Calculating the size of the mapped area as the lesser value
between the requested size and the actual size does not consider
the partial mapping offset. This can cause page fault access.
Fix the calculation of the starting and ending addresses, the
total size is now deduced from the difference between the end and
start addresses.
Additionally, the calculations have been rewritten in a clearer
and more understandable form.
Fixes: c58305af1835 ("drm/i915: Use remap_io_mapping() to prefault all PTE in a single pass")
Reported-by: Jann Horn <jannh@google.com>
Co-developed-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: <stable@vger.kernel.org> # v4.9+
Reviewed-by: Jann Horn <jannh@google.com>
Reviewed-by: Jonathan Cavitt <Jonathan.cavitt@intel.com>
[Joonas: Add Requires: tag]
Requires: 60a2066c5005 ("drm/i915/gem: Adjust vma offset for framebuffer mmap offset")
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240802083850.103694-3-andi.shyti@linux.intel.com
(cherry picked from commit 97b6784753da06d9d40232328efc5c5367e53417)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 48 +++++++++++++++++++++++++++++----
1 file changed, 43 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5b0d6d8b3ab8..d9cdd351caa4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2009,6 +2009,40 @@ compute_partial_view(struct drm_i915_gem_object *obj,
return view;
}
+static void set_address_limits(struct vm_area_struct *area,
+ struct i915_vma *vma,
+ unsigned long *start_vaddr,
+ unsigned long *end_vaddr)
+{
+ unsigned long vm_start, vm_end, vma_size; /* user's memory parameters */
+ long start, end; /* memory boundaries */
+
+ /*
+ * Let's move into the ">> PAGE_SHIFT"
+ * domain to be sure not to lose bits
+ */
+ vm_start = area->vm_start >> PAGE_SHIFT;
+ vm_end = area->vm_end >> PAGE_SHIFT;
+ vma_size = vma->size >> PAGE_SHIFT;
+
+ /*
+ * Calculate the memory boundaries by considering the offset
+ * provided by the user during memory mapping and the offset
+ * provided for the partial mapping.
+ */
+ start = vm_start;
+ start -= obj_offset;
+ start += vma->ggtt_view.partial.offset;
+ end = start + vma_size;
+
+ start = max_t(long, start, vm_start);
+ end = min_t(long, end, vm_end);
+
+ /* Let's move back into the "<< PAGE_SHIFT" domain */
+ *start_vaddr = (unsigned long)start << PAGE_SHIFT;
+ *end_vaddr = (unsigned long)end << PAGE_SHIFT;
+}
+
/**
* i915_gem_fault - fault a page into the GTT
* @vmf: fault info
@@ -2036,8 +2070,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
struct drm_i915_private *dev_priv = to_i915(dev);
struct i915_ggtt *ggtt = &dev_priv->ggtt;
bool write = !!(vmf->flags & FAULT_FLAG_WRITE);
+ unsigned long start, end; /* memory boundaries */
struct i915_vma *vma;
pgoff_t page_offset;
+ unsigned long pfn;
int ret;
/* Sanity check that we allow writing into this object */
@@ -2119,12 +2155,14 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
if (ret)
goto err_unpin;
+ set_address_limits(area, vma, &start, &end);
+
+ pfn = (ggtt->gmadr.start + i915_ggtt_offset(vma)) >> PAGE_SHIFT;
+ pfn += (start - area->vm_start) >> PAGE_SHIFT;
+ pfn -= vma->ggtt_view.partial.offset;
+
/* 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 = remap_io_mapping(area, start, pfn, end - start, &ggtt->iomap);
if (ret)
goto err_fence;
--
2.45.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 4.19.y] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation
2024-08-13 14:14 ` [PATCH 4.19.y] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation Andi Shyti
@ 2024-08-13 15:28 ` Greg KH
2024-08-13 16:09 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2024-08-13 15:28 UTC (permalink / raw)
To: Andi Shyti
Cc: stable, Jann Horn, Chris Wilson, Joonas Lahtinen, Matthew Auld,
Rodrigo Vivi, Jonathan Cavitt
On Tue, Aug 13, 2024 at 04:14:36PM +0200, Andi Shyti wrote:
> Commit 8bdd9ef7e9b1b2a73e394712b72b22055e0e26c3 upstream.
>
> Calculating the size of the mapped area as the lesser value
> between the requested size and the actual size does not consider
> the partial mapping offset. This can cause page fault access.
>
> Fix the calculation of the starting and ending addresses, the
> total size is now deduced from the difference between the end and
> start addresses.
>
> Additionally, the calculations have been rewritten in a clearer
> and more understandable form.
>
> Fixes: c58305af1835 ("drm/i915: Use remap_io_mapping() to prefault all PTE in a single pass")
> Reported-by: Jann Horn <jannh@google.com>
> Co-developed-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: <stable@vger.kernel.org> # v4.9+
> Reviewed-by: Jann Horn <jannh@google.com>
> Reviewed-by: Jonathan Cavitt <Jonathan.cavitt@intel.com>
> [Joonas: Add Requires: tag]
> Requires: 60a2066c5005 ("drm/i915/gem: Adjust vma offset for framebuffer mmap offset")
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20240802083850.103694-3-andi.shyti@linux.intel.com
> (cherry picked from commit 97b6784753da06d9d40232328efc5c5367e53417)
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 48 +++++++++++++++++++++++++++++----
> 1 file changed, 43 insertions(+), 5 deletions(-)
Both now applied, thanks.
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 4.19.y] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation
2024-08-13 15:28 ` Greg KH
@ 2024-08-13 16:09 ` Greg KH
2024-08-13 16:57 ` Andi Shyti
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2024-08-13 16:09 UTC (permalink / raw)
To: Andi Shyti
Cc: stable, Jann Horn, Chris Wilson, Joonas Lahtinen, Matthew Auld,
Rodrigo Vivi, Jonathan Cavitt
On Tue, Aug 13, 2024 at 05:28:15PM +0200, Greg KH wrote:
> On Tue, Aug 13, 2024 at 04:14:36PM +0200, Andi Shyti wrote:
> > Commit 8bdd9ef7e9b1b2a73e394712b72b22055e0e26c3 upstream.
> >
> > Calculating the size of the mapped area as the lesser value
> > between the requested size and the actual size does not consider
> > the partial mapping offset. This can cause page fault access.
> >
> > Fix the calculation of the starting and ending addresses, the
> > total size is now deduced from the difference between the end and
> > start addresses.
> >
> > Additionally, the calculations have been rewritten in a clearer
> > and more understandable form.
> >
> > Fixes: c58305af1835 ("drm/i915: Use remap_io_mapping() to prefault all PTE in a single pass")
> > Reported-by: Jann Horn <jannh@google.com>
> > Co-developed-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: <stable@vger.kernel.org> # v4.9+
> > Reviewed-by: Jann Horn <jannh@google.com>
> > Reviewed-by: Jonathan Cavitt <Jonathan.cavitt@intel.com>
> > [Joonas: Add Requires: tag]
> > Requires: 60a2066c5005 ("drm/i915/gem: Adjust vma offset for framebuffer mmap offset")
> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Link: https://patchwork.freedesktop.org/patch/msgid/20240802083850.103694-3-andi.shyti@linux.intel.com
> > (cherry picked from commit 97b6784753da06d9d40232328efc5c5367e53417)
> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 48 +++++++++++++++++++++++++++++----
> > 1 file changed, 43 insertions(+), 5 deletions(-)
>
> Both now applied, thanks.
Wait, did you build this? I get the following error:
CC [M] drivers/gpu/drm/i915/i915_gem.o
drivers/gpu/drm/i915/i915_gem.c: In function ‘set_address_limits’:
drivers/gpu/drm/i915/i915_gem.c:2034:18: error: ‘obj_offset’ undeclared (first use in this function); did you mean ‘iova_offset’?
2034 | start -= obj_offset;
| ^~~~~~~~~~
| iova_offset
I'll drop this now.
Can you fix this up and provide a working version?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 4.19.y] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation
2024-08-13 16:09 ` Greg KH
@ 2024-08-13 16:57 ` Andi Shyti
0 siblings, 0 replies; 7+ messages in thread
From: Andi Shyti @ 2024-08-13 16:57 UTC (permalink / raw)
To: Greg KH
Cc: Andi Shyti, stable, Jann Horn, Chris Wilson, Joonas Lahtinen,
Matthew Auld, Rodrigo Vivi, Jonathan Cavitt
On Tue, Aug 13, 2024 at 06:09:04PM +0200, Greg KH wrote:
> On Tue, Aug 13, 2024 at 05:28:15PM +0200, Greg KH wrote:
> > On Tue, Aug 13, 2024 at 04:14:36PM +0200, Andi Shyti wrote:
> > > Commit 8bdd9ef7e9b1b2a73e394712b72b22055e0e26c3 upstream.
> > >
> > > Calculating the size of the mapped area as the lesser value
> > > between the requested size and the actual size does not consider
> > > the partial mapping offset. This can cause page fault access.
> > >
> > > Fix the calculation of the starting and ending addresses, the
> > > total size is now deduced from the difference between the end and
> > > start addresses.
> > >
> > > Additionally, the calculations have been rewritten in a clearer
> > > and more understandable form.
> > >
> > > Fixes: c58305af1835 ("drm/i915: Use remap_io_mapping() to prefault all PTE in a single pass")
> > > Reported-by: Jann Horn <jannh@google.com>
> > > Co-developed-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > > Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: <stable@vger.kernel.org> # v4.9+
> > > Reviewed-by: Jann Horn <jannh@google.com>
> > > Reviewed-by: Jonathan Cavitt <Jonathan.cavitt@intel.com>
> > > [Joonas: Add Requires: tag]
> > > Requires: 60a2066c5005 ("drm/i915/gem: Adjust vma offset for framebuffer mmap offset")
> > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Link: https://patchwork.freedesktop.org/patch/msgid/20240802083850.103694-3-andi.shyti@linux.intel.com
> > > (cherry picked from commit 97b6784753da06d9d40232328efc5c5367e53417)
> > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_gem.c | 48 +++++++++++++++++++++++++++++----
> > > 1 file changed, 43 insertions(+), 5 deletions(-)
> >
> > Both now applied, thanks.
>
> Wait, did you build this? I get the following error:
>
> CC [M] drivers/gpu/drm/i915/i915_gem.o
> drivers/gpu/drm/i915/i915_gem.c: In function ‘set_address_limits’:
> drivers/gpu/drm/i915/i915_gem.c:2034:18: error: ‘obj_offset’ undeclared (first use in this function); did you mean ‘iova_offset’?
> 2034 | start -= obj_offset;
> | ^~~~~~~~~~
> | iova_offset
>
>
> I'll drop this now.
>
> Can you fix this up and provide a working version?
ops! Sorry, I was on the wrong branch.
Will send you immediately the correct patch.
Thank you,
Andi
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 4.19.y v2] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation
2024-08-12 9:26 FAILED: patch "[PATCH] drm/i915/gem: Fix Virtual Memory mapping boundaries" failed to apply to 4.19-stable tree gregkh
2024-08-13 14:14 ` [PATCH 4.19.y] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation Andi Shyti
@ 2024-08-13 17:09 ` Andi Shyti
2024-08-13 17:25 ` Greg KH
1 sibling, 1 reply; 7+ messages in thread
From: Andi Shyti @ 2024-08-13 17:09 UTC (permalink / raw)
To: stable
Cc: Andi Shyti, Jann Horn, Chris Wilson, Joonas Lahtinen,
Matthew Auld, Rodrigo Vivi, Jonathan Cavitt
Commit 8bdd9ef7e9b1b2a73e394712b72b22055e0e26c3 upstream.
Calculating the size of the mapped area as the lesser value
between the requested size and the actual size does not consider
the partial mapping offset. This can cause page fault access.
Fix the calculation of the starting and ending addresses, the
total size is now deduced from the difference between the end and
start addresses.
Additionally, the calculations have been rewritten in a clearer
and more understandable form.
Fixes: c58305af1835 ("drm/i915: Use remap_io_mapping() to prefault all PTE in a single pass")
Reported-by: Jann Horn <jannh@google.com>
Co-developed-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: <stable@vger.kernel.org> # v4.9+
Reviewed-by: Jann Horn <jannh@google.com>
Reviewed-by: Jonathan Cavitt <Jonathan.cavitt@intel.com>
[Joonas: Add Requires: tag]
Requires: 60a2066c5005 ("drm/i915/gem: Adjust vma offset for framebuffer mmap offset")
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240802083850.103694-3-andi.shyti@linux.intel.com
(cherry picked from commit 97b6784753da06d9d40232328efc5c5367e53417)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
Hi,
sorry for sending this v2 after I submitted a patch with a
compilation error. It slipped off a variable (obj_offset) that
was removed for kernel 4.19.
Thanks,
Andi
drivers/gpu/drm/i915/i915_gem.c | 47 +++++++++++++++++++++++++++++----
1 file changed, 42 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5b0d6d8b3ab8..478d989a2369 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2009,6 +2009,39 @@ compute_partial_view(struct drm_i915_gem_object *obj,
return view;
}
+static void set_address_limits(struct vm_area_struct *area,
+ struct i915_vma *vma,
+ unsigned long *start_vaddr,
+ unsigned long *end_vaddr)
+{
+ unsigned long vm_start, vm_end, vma_size; /* user's memory parameters */
+ long start, end; /* memory boundaries */
+
+ /*
+ * Let's move into the ">> PAGE_SHIFT"
+ * domain to be sure not to lose bits
+ */
+ vm_start = area->vm_start >> PAGE_SHIFT;
+ vm_end = area->vm_end >> PAGE_SHIFT;
+ vma_size = vma->size >> PAGE_SHIFT;
+
+ /*
+ * Calculate the memory boundaries by considering the offset
+ * provided by the user during memory mapping and the offset
+ * provided for the partial mapping.
+ */
+ start = vm_start;
+ start += vma->ggtt_view.partial.offset;
+ end = start + vma_size;
+
+ start = max_t(long, start, vm_start);
+ end = min_t(long, end, vm_end);
+
+ /* Let's move back into the "<< PAGE_SHIFT" domain */
+ *start_vaddr = (unsigned long)start << PAGE_SHIFT;
+ *end_vaddr = (unsigned long)end << PAGE_SHIFT;
+}
+
/**
* i915_gem_fault - fault a page into the GTT
* @vmf: fault info
@@ -2036,8 +2069,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
struct drm_i915_private *dev_priv = to_i915(dev);
struct i915_ggtt *ggtt = &dev_priv->ggtt;
bool write = !!(vmf->flags & FAULT_FLAG_WRITE);
+ unsigned long start, end; /* memory boundaries */
struct i915_vma *vma;
pgoff_t page_offset;
+ unsigned long pfn;
int ret;
/* Sanity check that we allow writing into this object */
@@ -2119,12 +2154,14 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
if (ret)
goto err_unpin;
+ set_address_limits(area, vma, &start, &end);
+
+ pfn = (ggtt->gmadr.start + i915_ggtt_offset(vma)) >> PAGE_SHIFT;
+ pfn += (start - area->vm_start) >> PAGE_SHIFT;
+ pfn -= vma->ggtt_view.partial.offset;
+
/* 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 = remap_io_mapping(area, start, pfn, end - start, &ggtt->iomap);
if (ret)
goto err_fence;
--
2.45.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 4.19.y v2] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation
2024-08-13 17:09 ` [PATCH 4.19.y v2] " Andi Shyti
@ 2024-08-13 17:25 ` Greg KH
0 siblings, 0 replies; 7+ messages in thread
From: Greg KH @ 2024-08-13 17:25 UTC (permalink / raw)
To: Andi Shyti
Cc: stable, Jann Horn, Chris Wilson, Joonas Lahtinen, Matthew Auld,
Rodrigo Vivi, Jonathan Cavitt
On Tue, Aug 13, 2024 at 07:09:30PM +0200, Andi Shyti wrote:
> Commit 8bdd9ef7e9b1b2a73e394712b72b22055e0e26c3 upstream.
>
> Calculating the size of the mapped area as the lesser value
> between the requested size and the actual size does not consider
> the partial mapping offset. This can cause page fault access.
>
> Fix the calculation of the starting and ending addresses, the
> total size is now deduced from the difference between the end and
> start addresses.
>
> Additionally, the calculations have been rewritten in a clearer
> and more understandable form.
>
> Fixes: c58305af1835 ("drm/i915: Use remap_io_mapping() to prefault all PTE in a single pass")
> Reported-by: Jann Horn <jannh@google.com>
> Co-developed-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: <stable@vger.kernel.org> # v4.9+
> Reviewed-by: Jann Horn <jannh@google.com>
> Reviewed-by: Jonathan Cavitt <Jonathan.cavitt@intel.com>
> [Joonas: Add Requires: tag]
> Requires: 60a2066c5005 ("drm/i915/gem: Adjust vma offset for framebuffer mmap offset")
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20240802083850.103694-3-andi.shyti@linux.intel.com
> (cherry picked from commit 97b6784753da06d9d40232328efc5c5367e53417)
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> Hi,
>
> sorry for sending this v2 after I submitted a patch with a
> compilation error. It slipped off a variable (obj_offset) that
> was removed for kernel 4.19.
Much better, now queued up, thanks!
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-08-13 17:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-12 9:26 FAILED: patch "[PATCH] drm/i915/gem: Fix Virtual Memory mapping boundaries" failed to apply to 4.19-stable tree gregkh
2024-08-13 14:14 ` [PATCH 4.19.y] drm/i915/gem: Fix Virtual Memory mapping boundaries calculation Andi Shyti
2024-08-13 15:28 ` Greg KH
2024-08-13 16:09 ` Greg KH
2024-08-13 16:57 ` Andi Shyti
2024-08-13 17:09 ` [PATCH 4.19.y v2] " Andi Shyti
2024-08-13 17:25 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox