Re: drm/i915: Initialize all contexts

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Mon, Mar 23, 2015 at 06:54:52PM +0300, Dan Carpenter wrote:
>> Hello Ben Widawsky,
>> 
>> This is a semi-automatic email about new static checker warnings.
>
> Adding Michel and Mika.
> -Daniel
>
>> 
>> The patch 6702cf16e0ba: "drm/i915: Initialize all contexts" from Mar 
>> 16, 2015, leads to the following Smatch complaint:
>> 
>> drivers/gpu/drm/i915/i915_gem_context.c:730 do_switch()
>> 	 error: we previously assumed 'to->ppgtt' could be null (see line 713)
>> 
>> drivers/gpu/drm/i915/i915_gem_context.c
>>    712			 * occur when this occurs. */
>>    713		} else if (to->ppgtt &&
>>                            ^^^^^^^^^
>> Patch introduces a new NULL check.
>> 
>>    714				test_and_clear_bit(ring->id, &to->ppgtt->pd_dirty_rings))
>>    715			hw_flags |= MI_FORCE_RESTORE;
>>    716	
>>    717		/* We should never emit switch_mm more than once */
>>    718		WARN_ON(needs_pd_load_pre(ring, to) &&
>>    719				needs_pd_load_post(ring, to, hw_flags));
>>    720	
>>    721		ret = mi_set_context(ring, to, hw_flags);
>>    722		if (ret)
>>    723			goto unpin_out;
>>    724	
>>    725		/* GEN8 does *not* require an explicit reload if the PDPs have been
>>    726		 * setup, and we do not wish to move them.
>>    727		 */
>>    728		if (needs_pd_load_post(ring, to, hw_flags)) {
>>    729			trace_switch_mm(ring, to);
>>    730			ret = to->ppgtt->switch_mm(to->ppgtt, ring);
>>                               ^^^^^^^^^^^
>> Existing unchecked dereference.

That branch can't be taken if to->ppgtt == NULL. (see
needs_pd_load_post()) So we are safe.
Or is there something I miss out with the context?

Thanks,
--Mika


>> 
>>    731			/* The hardware context switch is emitted, but we haven't
>>    732			 * actually changed the state - so it's probably safe to bail
>> 
>> regards,
>> dan carpenter
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx