* Re: drm/i915: Initialize all contexts
@ 2015-03-23 15:54 Dan Carpenter
2015-03-24 9:12 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2015-03-23 15:54 UTC (permalink / raw)
To: benjamin.widawsky; +Cc: intel-gfx
Hello Ben Widawsky,
This is a semi-automatic email about new static checker warnings.
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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: drm/i915: Initialize all contexts
2015-03-23 15:54 drm/i915: Initialize all contexts Dan Carpenter
@ 2015-03-24 9:12 ` Daniel Vetter
2015-03-24 10:09 ` Mika Kuoppala
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2015-03-24 9:12 UTC (permalink / raw)
To: Dan Carpenter; +Cc: intel-gfx, Mika Kuoppala, benjamin.widawsky
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.
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: drm/i915: Initialize all contexts
2015-03-24 9:12 ` Daniel Vetter
@ 2015-03-24 10:09 ` Mika Kuoppala
2015-03-24 10:57 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Mika Kuoppala @ 2015-03-24 10:09 UTC (permalink / raw)
To: Daniel Vetter, Dan Carpenter; +Cc: intel-gfx, benjamin.widawsky
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-24 10:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-23 15:54 drm/i915: Initialize all contexts Dan Carpenter
2015-03-24 9:12 ` Daniel Vetter
2015-03-24 10:09 ` Mika Kuoppala
2015-03-24 10:57 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox