<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<body>
<div dir="auto">
<div dir="auto"><span style="font-size: 12pt;">On May 7, 2021 03:35:33 Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:</span></div><div id="aqm-original" style="color: black;">
<div><br></div>
<blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #808080; padding-left: 0.75ex;">
<div dir="auto">From: Tvrtko Ursulin <tvrtko.ursulin@intel.com></div>
<div dir="auto"><br></div>
<div dir="auto">This is an alternative proposed fix for the below references bug report</div>
<div dir="auto">where dma fence error propagation is causing undesirable change in</div>
<div dir="auto">behaviour post GPU hang/reset.</div>
<div dir="auto"><br></div>
<div dir="auto">Approach in this patch is to simply stop propagating all dma fence errors</div>
<div dir="auto">by default since that seems to be the upstream ask.</div>
<div dir="auto"><br></div>
<div dir="auto">To handle the case where i915 needs error propagation for security, I add</div>
<div dir="auto">a new dma fence flag DMA_FENCE_FLAG_PROPAGATE_ERROR and make use of it in</div>
<div dir="auto">the command parsing chain only.</div></blockquote></div><div dir="auto"><br></div><div dir="auto">This sounds plausible. I can't review in full without doing a thorough audit of the surrounding code, though. I'll try to get to that next week if Daniel doesn't beat me to it. Thanks for working on this!</div><div dir="auto"><br></div><div dir="auto">--Jason</div><div dir="auto"><br></div><div dir="auto"><br></div><div id="aqm-original" style="color: black;" dir="auto"><blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #808080; padding-left: 0.75ex;"><div dir="auto"></div>
<div dir="auto">It sounds a plausible argument that fence propagation could be useful in</div>
<div dir="auto">which case a core flag to enable opt-in should be universally useful.</div>
<div dir="auto"><br></div>
<div dir="auto">Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com></div>
<div dir="auto">Reported-by: Marcin Slusarz <marcin.slusarz@intel.com></div>
<div dir="auto">Reported-by: Miroslav Bendik</div>
<div dir="auto">References: 9e31c1fe45d5 ("drm/i915: Propagate errors on awaiting already signaled fences")</div>
<div dir="auto">References: https://gitlab.freedesktop.org/drm/intel/-/issues/3080</div>
<div dir="auto">Cc: Jason Ekstrand <jason.ekstrand@intel.com></div>
<div dir="auto">Cc: Daniel Vetter <daniel.vetter@ffwll.ch></div>
<div dir="auto">---</div>
<div dir="auto"> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 ++</div>
<div dir="auto"> drivers/gpu/drm/i915/i915_sw_fence.c | 8 ++++----</div>
<div dir="auto"> drivers/gpu/drm/i915/i915_sw_fence.h | 8 ++++++++</div>
<div dir="auto"> include/linux/dma-fence.h | 1 +</div>
<div dir="auto"> 4 files changed, 15 insertions(+), 4 deletions(-)</div>
<div dir="auto"><br></div>
<div dir="auto">diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c</div>
<div dir="auto">index 297143511f99..6a516d1261d0 100644</div>
<div dir="auto">--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c</div>
<div dir="auto">+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c</div>
<div dir="auto">@@ -2522,6 +2522,8 @@ static int eb_parse_pipeline(struct i915_execbuffer *eb,</div>
<div dir="auto"> }</div>
<div dir="auto"> </div>
<div dir="auto"> dma_fence_work_init(&pw->base, &eb_parse_ops);</div>
<div dir="auto">+ /* Propagate errors for security. */</div>
<div dir="auto">+ __set_bit(DMA_FENCE_FLAG_PROPAGATE_ERROR, &pw->base.dma.flags);</div>
<div dir="auto"> </div>
<div dir="auto"> pw->engine = eb->engine;</div>
<div dir="auto"> pw->batch = eb->batch->vma;</div>
<div dir="auto">diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c</div>
<div dir="auto">index 2744558f3050..2ee917932ccf 100644</div>
<div dir="auto">--- a/drivers/gpu/drm/i915/i915_sw_fence.c</div>
<div dir="auto">+++ b/drivers/gpu/drm/i915/i915_sw_fence.c</div>
<div dir="auto">@@ -449,7 +449,7 @@ static void dma_i915_sw_fence_wake_timer(struct dma_fence *dma,</div>
<div dir="auto"> </div>
<div dir="auto"> fence = xchg(&cb->base.fence, NULL);</div>
<div dir="auto"> if (fence) {</div>
<div dir="auto">- i915_sw_fence_set_error_once(fence, dma->error);</div>
<div dir="auto">+ i915_sw_fence_propagate_dma_fence_error(fence, dma);</div>
<div dir="auto"> i915_sw_fence_complete(fence);</div>
<div dir="auto"> }</div>
<div dir="auto"> </div>
<div dir="auto">@@ -480,7 +480,7 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,</div>
<div dir="auto"> might_sleep_if(gfpflags_allow_blocking(gfp));</div>
<div dir="auto"> </div>
<div dir="auto"> if (dma_fence_is_signaled(dma)) {</div>
<div dir="auto">- i915_sw_fence_set_error_once(fence, dma->error);</div>
<div dir="auto">+ i915_sw_fence_propagate_dma_fence_error(fence, dma);</div>
<div dir="auto"> return 0;</div>
<div dir="auto"> }</div>
<div dir="auto"> </div>
<div dir="auto">@@ -496,7 +496,7 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,</div>
<div dir="auto"> if (ret)</div>
<div dir="auto"> return ret;</div>
<div dir="auto"> </div>
<div dir="auto">- i915_sw_fence_set_error_once(fence, dma->error);</div>
<div dir="auto">+ i915_sw_fence_propagate_dma_fence_error(fence, dma);</div>
<div dir="auto"> return 0;</div>
<div dir="auto"> }</div>
<div dir="auto"> </div>
<div dir="auto">@@ -548,7 +548,7 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,</div>
<div dir="auto"> debug_fence_assert(fence);</div>
<div dir="auto"> </div>
<div dir="auto"> if (dma_fence_is_signaled(dma)) {</div>
<div dir="auto">- i915_sw_fence_set_error_once(fence, dma->error);</div>
<div dir="auto">+ i915_sw_fence_propagate_dma_fence_error(fence, dma);</div>
<div dir="auto"> return 0;</div>
<div dir="auto"> }</div>
<div dir="auto"> </div>
<div dir="auto">diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h</div>
<div dir="auto">index 30a863353ee6..872ef80ebd10 100644</div>
<div dir="auto">--- a/drivers/gpu/drm/i915/i915_sw_fence.h</div>
<div dir="auto">+++ b/drivers/gpu/drm/i915/i915_sw_fence.h</div>
<div dir="auto">@@ -116,4 +116,12 @@ i915_sw_fence_set_error_once(struct i915_sw_fence *fence, int error)</div>
<div dir="auto"> cmpxchg(&fence->error, 0, error);</div>
<div dir="auto"> }</div>
<div dir="auto"> </div>
<div dir="auto">+static inline void</div>
<div dir="auto">+i915_sw_fence_propagate_dma_fence_error(struct i915_sw_fence *fence,</div>
<div dir="auto">+ struct dma_fence *dma)</div>
<div dir="auto">+{</div>
<div dir="auto">+ if (unlikely(test_bit(DMA_FENCE_FLAG_PROPAGATE_ERROR, &dma->flags)))</div>
<div dir="auto">+ i915_sw_fence_set_error_once(fence, dma->error);</div>
<div dir="auto">+}</div>
<div dir="auto">+</div>
<div dir="auto"> #endif /* _I915_SW_FENCE_H_ */</div>
<div dir="auto">diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h</div>
<div dir="auto">index 6ffb4b2c6371..8dabe1650f11 100644</div>
<div dir="auto">--- a/include/linux/dma-fence.h</div>
<div dir="auto">+++ b/include/linux/dma-fence.h</div>
<div dir="auto">@@ -99,6 +99,7 @@ enum dma_fence_flag_bits {</div>
<div dir="auto"> DMA_FENCE_FLAG_SIGNALED_BIT,</div>
<div dir="auto"> DMA_FENCE_FLAG_TIMESTAMP_BIT,</div>
<div dir="auto"> DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,</div>
<div dir="auto">+ DMA_FENCE_FLAG_PROPAGATE_ERROR,</div>
<div dir="auto"> DMA_FENCE_FLAG_USER_BITS, /* must always be last member */</div>
<div dir="auto"> };</div>
<div dir="auto"> </div>
<div dir="auto">-- </div>
<div dir="auto">2.30.2</div>
<div dir="auto"><br></div>
<div dir="auto">_______________________________________________</div>
<div dir="auto">Intel-gfx mailing list</div>
<div dir="auto">Intel-gfx@lists.freedesktop.org</div>
<div dir="auto">https://lists.freedesktop.org/mailman/listinfo/intel-gfx</div>
</blockquote>
</div><div dir="auto"><br></div>
</div></body>
</html>