Daniel Vetter <daniel at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch