[Intel-gfx] [PATCH] drm/i915: Reorder phys backing storage release

[Chris Wilson]

On Wed, Dec 07, 2016 at 02:10:34PM +0200, Joonas Lahtinen wrote:
> On ke, 2016-12-07 at 10:07 +0000, Chris Wilson wrote:
> > In commit a4f5ea64f0a8 ("drm/i915: Refactor object page API"), I
> > reordered the object->pages teardown to be more friendly wrt to a
> > separate obj->mm.lock. However, I overlooked the phys object and left it
> > with a dangling use-after-free of its phys_handle. Move the allocation
> > of the phys handle to get_pages and it release to put_pages to prevent
> > the invalid access and to improve symmetry.
> > 
> > Testcase: igt/drv_selftest/objects
> > Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Fixes: a4f5ea64f0a8 ("drm/i915: Refactor object page API")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: intel-gfx at lists.freedesktop.org
> 
> <SNIP>
> 
> >  i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
> >  {
> >  	struct address_space *mapping = obj->base.filp->f_mapping;
> > -	char *vaddr = obj->phys_handle->vaddr;
> > +	drm_dma_handle_t *phys;
> >  	struct sg_table *st;
> >  	struct scatterlist *sg;
> > +	char *vaddr;
> >  	int i;
> >  
> >  	if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
> >  		return ERR_PTR(-EINVAL);
> >  
> > +	phys = drm_pci_alloc(obj->base.dev, obj->base.size, obj->base.size);
> 
> Aligning to object size sounds bit rough without any comments.

Not that rough really, since the phys allocation will be power-of-two
aligned both in size and address. The alignment can only be power-of-two
up to the size of the object (rounded up).

The choice is adding an extra parameter for a rarely used feature or
just picking an alignment that must work for all callers. Since the
objects are either 4k or 16k and either 4k or 16k aligned (though only
830 cursors are 16k aligned), the alignment falls out of the actual
buddy allocation for the contiguous pages.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre