Re: [PATCH 1/7] drm/i915: Fix the offset issue for the stolen GEM objects

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jan 09, 2014 at 09:45:21AM +0000, Chris Wilson wrote:
> On Thu, Jan 09, 2014 at 11:00:17AM +0530, akash.goel@intel.com wrote:
> > From: Akash Goel <akash.goel@intel.com>
> > 
> > The 'offset' field of the 'scatterlist' structure was wrongly
> > programmed with the offset value from the base of stolen area,
> > whereas this field indicates the offset from where the interested
> > data starts within the PAGE pointed to by the 'page-link' field.
> > As a result when a new GEM object allocated from the stolen
> > area is mapped to GTT, it could lead to an overwrite of GTT entries
> > as the page count calculation will go wrong, refer the function
> > 'sg_page_count'.
> 
> This statement is incorrect since my use of sg here predates
> sg_page_iter.
> 
> The stolen sg has no page_link, the meaning of offset/length here are
> relative to the base of the stolen area.
> 
> However, if you wish to rephrase the above...

Actually we add offset both to sg->offset and adjust the dma_bus_addr
since this has been introduced in

commit 0104fdbb84d7adb0e377ed05bf75eba97b007544
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Nov 15 11:32:26 2012 +0000

    drm/i915: Introduce i915_gem_object_create_stolen()

But only Imre's conversion to the sg_page_iter started to pay any
attention to sg->offset in

commit 6e995e231a90ce7c5ce2a9eae23c8e22f4388db1
Author: Imre Deak <imre.deak@intel.com>
Date:   Mon Feb 18 19:28:04 2013 +0200

    drm/i915: use for_each_sg_page for setting up the gtt ptes

So with a bit of commit message rewording and these references and cc:
stable this looks good.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch