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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
