intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
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

  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).