From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3 1/8] drm/i915: Make sure fb gtt offsets stay within 32bits
Date: Wed, 26 Sep 2018 12:27:40 +0300 [thread overview]
Message-ID: <20180926092740.GO9144@intel.com> (raw)
In-Reply-To: <153790738445.21139.17414049229386732949@skylake-alporthouse-com>
On Tue, Sep 25, 2018 at 09:29:44PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-09-25 20:37:07)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Let's try to make sure the fb offset computations never hit
> > an integer overflow by making sure the entire fb stays
> > below 32bits. framebuffer_check() in the core already does
> > the same check, but as it doesn't know about tiling some things
> > can slip through. Repeat the check in the driver with tiling
> > taken into account.
> >
> > v2: Use add_overflows() after massaging it to work for me (Chris)
>
> Oh, it had fallen out of use.
>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_utils.h | 8 ++++----
> > drivers/gpu/drm/i915/intel_display.c | 18 +++++++++++++++++-
> > 2 files changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> > index 395dd2511568..c43ec993fa90 100644
> > --- a/drivers/gpu/drm/i915/i915_utils.h
> > +++ b/drivers/gpu/drm/i915/i915_utils.h
> > @@ -44,13 +44,13 @@
> > __stringify(x), (long)(x))
> >
> > #if defined(GCC_VERSION) && GCC_VERSION >= 70000
> > -#define add_overflows(A, B) \
> > - __builtin_add_overflow_p((A), (B), (typeof((A) + (B)))0)
> > +#define add_overflows(A, B, C) \
> > + __builtin_add_overflow_p((A), (B), (C))
> > #else
> > -#define add_overflows(A, B) ({ \
> > +#define add_overflows(A, B, C) ({ \
> > typeof(A) a = (A); \
> > typeof(B) b = (B); \
> > - a + b < a; \
> > + (typeof(C))(a + b) < a; \
> > })
> > #endif
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 4c5c2b39e65c..a3ae24e03316 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2400,10 +2400,26 @@ static int intel_fb_offset_to_xy(int *x, int *y,
> > int color_plane)
> > {
> > struct drm_i915_private *dev_priv = to_i915(fb->dev);
> > + unsigned int height;
> >
> > if (fb->modifier != DRM_FORMAT_MOD_LINEAR &&
> > - fb->offsets[color_plane] % intel_tile_size(dev_priv))
> > + fb->offsets[color_plane] % intel_tile_size(dev_priv)) {
> > + DRM_DEBUG_KMS("Misaligned offset 0x%08x for color plane %d\n",
> > + fb->offsets[color_plane], color_plane);
> > return -EINVAL;
> > + }
> > +
> > + height = drm_framebuffer_plane_height(fb->height, fb, color_plane);
> > + height = ALIGN(height, intel_tile_height(fb, color_plane));
> > +
> > + /* Catch potential overflows early */
> > + if (add_overflows(mul_u32_u32(height, fb->pitches[color_plane]),
> > + fb->offsets[color_plane], (u32)0)) {
>
> Should we just pass type? Atm we aren't using the value for anything.
> Then it would be add_overflows_t(a, b, T) with the obvious wrapping for
> add_overflows(a, b). Although to be consistent with min_t, perhaps
> add_overflows_t(T, a, b).
Indeed, that does seem a bit more consistent with existing stuff.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-09-26 9:29 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-25 19:37 [PATCH v3 0/8] drm/i915: GTT remapping for display Ville Syrjala
2018-09-25 19:37 ` [PATCH v3 1/8] drm/i915: Make sure fb gtt offsets stay within 32bits Ville Syrjala
2018-09-25 20:29 ` Chris Wilson
2018-09-26 9:27 ` Ville Syrjälä [this message]
2018-09-26 20:09 ` Chris Wilson
2018-09-27 12:24 ` Ville Syrjälä
2018-10-23 16:02 ` [PATCH v4 " Ville Syrjala
2018-10-23 18:49 ` Chris Wilson
2018-10-23 19:16 ` Ville Syrjälä
2018-09-25 19:37 ` [PATCH v3 2/8] drm/i915: Decouple SKL stride units from intel_fb_stride_alignment() Ville Syrjala
2018-10-23 18:50 ` Chris Wilson
2018-09-25 19:37 ` [PATCH v3 3/8] drm/i915: Add a new "remapped" gtt_view Ville Syrjala
2018-09-26 7:50 ` Tvrtko Ursulin
2018-10-01 15:03 ` Ville Syrjälä
2018-10-01 15:12 ` Chris Wilson
2018-10-01 15:27 ` Ville Syrjälä
2018-10-01 15:37 ` Chris Wilson
2018-10-01 15:48 ` Tvrtko Ursulin
2018-10-05 18:42 ` Ville Syrjälä
2018-10-09 8:24 ` Tvrtko Ursulin
2018-10-09 8:41 ` Chris Wilson
2018-10-09 11:54 ` Ville Syrjälä
2018-10-10 7:04 ` Tvrtko Ursulin
2018-10-01 15:38 ` Tvrtko Ursulin
2018-10-01 15:35 ` Tvrtko Ursulin
2018-10-23 16:02 ` [PATCH v4 " Ville Syrjala
2018-10-23 18:56 ` Chris Wilson
2018-10-23 19:10 ` Ville Syrjälä
2018-10-26 9:19 ` Tvrtko Ursulin
2018-10-26 12:43 ` Ville Syrjälä
2018-10-26 12:48 ` Tvrtko Ursulin
2018-09-25 19:37 ` [PATCH v3 4/8] drm/i915/selftests: Add mock selftest for remapped vmas Ville Syrjala
2018-09-25 20:22 ` Chris Wilson
2018-09-26 9:28 ` Ville Syrjälä
2018-10-23 16:03 ` [PATCH v4 " Ville Syrjala
2018-10-23 19:02 ` Chris Wilson
2018-10-23 19:14 ` Ville Syrjälä
2018-09-25 19:37 ` [PATCH v3 5/8] drm/i915/selftests: Add live vma selftest Ville Syrjala
2018-09-25 20:19 ` Chris Wilson
2018-09-25 20:40 ` Chris Wilson
2018-09-26 9:33 ` Ville Syrjälä
2018-10-23 16:03 ` [PATCH v4 " Ville Syrjala
2018-10-23 19:05 ` Chris Wilson
2018-09-25 19:37 ` [PATCH v3 6/8] drm/i915: Overcome display engine stride limits via GTT remapping Ville Syrjala
2018-10-23 19:16 ` Chris Wilson
2018-10-25 13:45 ` Ville Syrjälä
2018-09-25 19:37 ` [PATCH v3 7/8] drm/i915: Bump gen4+ fb stride limit to 256KiB Ville Syrjala
2018-09-25 20:13 ` Chris Wilson
2018-09-28 19:19 ` Ville Syrjälä
2018-09-25 19:37 ` [PATCH v3 8/8] drm/i915: Bump gen7+ fb size limits to 16kx16k Ville Syrjala
2018-09-25 19:59 ` Chris Wilson
2018-09-26 9:25 ` Ville Syrjälä
2018-09-25 20:05 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: GTT remapping for display Patchwork
2018-09-25 20:08 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-09-25 20:30 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-25 21:21 ` ✓ Fi.CI.IGT: " Patchwork
2018-10-23 16:21 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: GTT remapping for display (rev5) Patchwork
2018-10-23 16:24 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-10-23 16:43 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-23 18:13 ` ✓ Fi.CI.IGT: " Patchwork
2019-01-09 9:45 ` [PATCH v3 0/8] drm/i915: GTT remapping for display Timo Aaltonen
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=20180926092740.GO9144@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.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).