[Intel-gfx] [PATCH] drm/i915: make i915_stolen_to_physical() return phys_addr_t

[Paulo Zanoni]

Em Sex, 2017-01-27 às 15:59 +0200, Ville Syrjälä escreveu:
> On Thu, Jan 26, 2017 at 06:19:07PM -0200, Paulo Zanoni wrote:
> > 
> > The i915_stolen_to_physical() function has 'unsigned long' as its
> > return type but it returns the 'base' variable, which is of type
> > 'u32'. The only place where this function is called assigns the
> > returned value to dev_priv->mm.stolen_base, which is of type
> > 'phys_addr_t'. The return value is actually a physical address and
> > everything else in the stolen memory code seems to be using
> > phys_addr_t, so fix i915_stolen_to_physical() to use phys_addr_t.
> 
> Size of phys_addr_t depends on PAE no? So what if someone were to
> boot
> a !PAE kernel on a machine where stolen lives somewhere >4GiB?

Notice that this should not be possible today since we read the stolen
memory address from a 32 bit register. Of course, things may change in
the future, so having future-proof code is obviously good.


> 
> > 
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_stolen.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index 127d698..0816ebd 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -79,12 +79,12 @@ void i915_gem_stolen_remove_node(struct
> > drm_i915_private *dev_priv,
> >  	mutex_unlock(&dev_priv->mm.stolen_lock);
> >  }
> >  
> > -static unsigned long i915_stolen_to_physical(struct
> > drm_i915_private *dev_priv)
> > +static phys_addr_t i915_stolen_to_physical(struct drm_i915_private
> > *dev_priv)
> >  {
> >  	struct pci_dev *pdev = dev_priv->drm.pdev;
> >  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> >  	struct resource *r;
> > -	u32 base;
> > +	phys_addr_t base;
> >  
> >  	/* Almost universally we can find the Graphics Base of
> > Stolen Memory
> >  	 * at register BSM (0x5c) in the igfx configuration space.
> > On a few
> > @@ -196,7 +196,7 @@ static unsigned long
> > i915_stolen_to_physical(struct drm_i915_private *dev_priv)
> >  	if (INTEL_GEN(dev_priv) <= 4 &&
> >  	    !IS_G33(dev_priv) && !IS_PINEVIEW(dev_priv) &&
> > !IS_G4X(dev_priv)) {
> >  		struct {
> > -			u32 start, end;
> > +			phys_addr_t start, end;
> >  		} stolen[2] = {
> >  			{ .start = base, .end = base + ggtt-
> > >stolen_size, },
> >  			{ .start = base, .end = base + ggtt-
> > >stolen_size, },
> > @@ -228,11 +228,12 @@ static unsigned long
> > i915_stolen_to_physical(struct drm_i915_private *dev_priv)
> >  
> >  		if (stolen[0].start != stolen[1].start ||
> >  		    stolen[0].end != stolen[1].end) {
> > +			phys_addr_t end = base + ggtt->stolen_size 
> > - 1;
> >  			DRM_DEBUG_KMS("GTT within stolen memory at
> > 0x%llx-0x%llx\n",
> >  				      (unsigned long
> > long)ggtt_start,
> >  				      (unsigned long long)ggtt_end
> > - 1);
> > -			DRM_DEBUG_KMS("Stolen memory adjusted to
> > 0x%x-0x%x\n",
> > -				      base, base + (u32)ggtt-
> > >stolen_size - 1);
> > +			DRM_DEBUG_KMS("Stolen memory adjusted to
> > %pa - %pa\n",
> > +				      &base, &end);
> >  		}
> >  	}
> >  
> > @@ -261,8 +262,9 @@ static unsigned long
> > i915_stolen_to_physical(struct drm_i915_private *dev_priv)
> >  		 * range. Apparently this works.
> >  		 */
> >  		if (r == NULL && !IS_GEN3(dev_priv)) {
> > -			DRM_ERROR("conflict detected with stolen
> > region: [0x%08x - 0x%08x]\n",
> > -				  base, base + (uint32_t)ggtt-
> > >stolen_size);
> > +			phys_addr_t end = base + ggtt-
> > >stolen_size;
> > +			DRM_ERROR("conflict detected with stolen
> > region: [%pa - %pa]\n",
> > +				  &base, &end);
> >  			base = 0;
> >  		}
> >  	}
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>