[Intel-gfx] [PATCH 6/8] drm/i915: Sanitize stolen memory size calculation

Joonas Lahtinen joonas.lahtinen at linux.intel.com
Thu Apr 27 09:34:18 UTC 2017


On ke, 2017-04-26 at 18:27 +0300, Ville Syrjälä wrote:
> On Wed, Apr 26, 2017 at 04:40:11PM +0300, Imre Deak wrote:
> > 
> > On GEN8+ (not counting CHV) the calculation can in theory result in an
> > incorrect sign extension with all upper bits set. In practice this is
> > unlikely to happen since it would require 4GB of stolen memory set
> > aside. For consistency still prevent the sign extension explicitly
> > everywhere.
> > 
> > Signed-off-by: Imre Deak <imre.deak at intel.com>

<SNIP>

> > @@ -2577,14 +2577,14 @@ static size_t gen6_get_stolen_size(u16 snb_gmch_ctl)
> >  {
> > > >  	snb_gmch_ctl >>= SNB_GMCH_GMS_SHIFT;
> > > >  	snb_gmch_ctl &= SNB_GMCH_GMS_MASK;
> > > > -	return snb_gmch_ctl << 25; /* 32 MB units */
> > > > +	return (size_t)snb_gmch_ctl << 25; /* 32 MB units */
> 
> So the u16 gets promoted to int, which gets converted to size_t,
> which may be larger than int, and thus things get sign extended.
> 
> Can't happen in the gen6 case actually due to SNB_GMCH_GMS_MASK being
> small enough. But the gen8 case at least looks theoretically possible.
> But having the case everywhere seems like the best way to avoid
> someone copy-pasting the wrong thing when the next variant gets added.

I was about to comment that early-quirks needs to be fixed too, but it
was already fixed when I synchronized the code last time.

That reminded me that I still have the GIT branch to eliminate the code
duplication, which is probably why I didn't revamp the i915 variants.

The trouble is that in early-quirks, pci subsystem functions are
unavailable and the functions are __init, so we'll have to choose
between:

a) code duplication, as we have now
b) move the functions to i915_drm.h as inline with hooks, code size is not
   shrunk, but code duplication is eliminated
c) always have the functions as non __init, even if i915 is disabled
   reduces duplication and kernel size, (well, increases x86 tiny kernel
   size)
c) <insert your suggestion here>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation


More information about the Intel-gfx mailing list