| Message ID | 20221019071252.180682-1-andrzej.hajda@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/selftests: add prefetch padding to store_dw batchbuffer | expand |
On 19/10/2022 08:12, Andrzej Hajda wrote: > Instruction prefetch mechanism requires that 512 bytes after the last > command should be readable by EU. Otherwise DMAR errors and engine > hangs can happen. > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5278 > Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> Is there a Bspec ref for this? I would have assumed that EU was more about kernels/shaders, than simple MI commands? Also should we be hitting dmar errors for ppGTT if this were some kind of overfetch? AFAICT we always point entries back to scratch, unlike with say the GGTT where we might have stale entries, and unbinding should flush the tlb? > --- > drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c > index 3c55e77b0f1b00..fe999a02f8e10a 100644 > --- a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c > +++ b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c > @@ -50,7 +50,7 @@ igt_emit_store_dw(struct i915_vma *vma, > u32 *cmd; > int err; > > - size = (4 * count + 1) * sizeof(u32); > + size = (4 * count + 1) * sizeof(u32) + 512; > size = round_up(size, PAGE_SIZE); > obj = i915_gem_object_create_internal(vma->vm->i915, size); > if (IS_ERR(obj))
On 19/10/2022 10:12, Matthew Auld wrote: > On 19/10/2022 08:12, Andrzej Hajda wrote: >> Instruction prefetch mechanism requires that 512 bytes after the last >> command should be readable by EU. Otherwise DMAR errors and engine >> hangs can happen. >> >> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5278 >> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> > > Is there a Bspec ref for this? I would have assumed that EU was more > about kernels/shaders, than simple MI commands? Also should we be > hitting dmar errors for ppGTT if this were some kind of overfetch? > AFAICT we always point entries back to scratch, unlike with say the GGTT > where we might have stale entries, and unbinding should flush the tlb? s/unbinding/put_pages/ > >> --- >> drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c >> b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c >> index 3c55e77b0f1b00..fe999a02f8e10a 100644 >> --- a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c >> +++ b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c >> @@ -50,7 +50,7 @@ igt_emit_store_dw(struct i915_vma *vma, >> u32 *cmd; >> int err; >> - size = (4 * count + 1) * sizeof(u32); >> + size = (4 * count + 1) * sizeof(u32) + 512; >> size = round_up(size, PAGE_SIZE); >> obj = i915_gem_object_create_internal(vma->vm->i915, size); >> if (IS_ERR(obj))
On 19.10.2022 11:14, Matthew Auld wrote: > On 19/10/2022 10:12, Matthew Auld wrote: >> On 19/10/2022 08:12, Andrzej Hajda wrote: >>> Instruction prefetch mechanism requires that 512 bytes after the last >>> command should be readable by EU. Otherwise DMAR errors and engine >>> hangs can happen. >>> >>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5278 >>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> >> >> Is there a Bspec ref for this? I would have assumed that EU was more >> about kernels/shaders, than simple MI commands? Also should we be >> hitting dmar errors for ppGTT if this were some kind of overfetch? >> AFAICT we always point entries back to scratch, unlike with say the >> GGTT where we might have stale entries, and unbinding should flush >> the tlb? > > s/unbinding/put_pages/ Bspec is here [1], but when you made distinction between simple MI commands and kernel/shaders I am not so sure if it applies to this case, so I will present my finding leading to this conclusion: My findings (on RaptorLake): 1. dmar errors always print physical address of recently removed bb created by igt_emit_store_dw, at least in my tests. 2. intel_iommu enqueues tlb flush during put_pages of this bb, but actual flush happens later, triggered by timer. 3. Together with dmar errors GuC reports CAT error on context/engine executing this batch (with IPEHR=MI_BATCH_BUFFER_END). 4. Errors happens only on vcs/vecs (???). 5. Errors happens only in case tested huge page has size SZ_2M - SZ_64K, or SZ_2M - SZ_4K. In both cases calculated size of bb (8kb) is just few dwords after the last cmd, in other cases there is much more padding. 6. Enlarging bb works (as in this patch). 7. Flushing iommu tlb for the phys address of bb just before calling dma_unmap_sg (in i915_gem_gtt_finish_pages) helps as well :) 8. There is already some workaround present in i915_gem_gtt_finish_pages: > > /* XXXThis does not prevent more requests being submitted! */ > > if(unlikely(ggtt->do_idle_maps)) > > /* Wait a bit, in the hope it avoids the hang */ > > usleep_range(100, 250); > but it is only implemented for Gen5 and is slow, but also works (probably because tlb is flushed meantime). [1]: https://gfxspecs.intel.com/Predator/Home/Index/47286 Regards Andrzej > >> >>> --- >>> drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c >>> b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c >>> index 3c55e77b0f1b00..fe999a02f8e10a 100644 >>> --- a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c >>> +++ b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c >>> @@ -50,7 +50,7 @@ igt_emit_store_dw(struct i915_vma *vma, >>> u32 *cmd; >>> int err; >>> - size = (4 * count + 1) * sizeof(u32); >>> + size = (4 * count + 1) * sizeof(u32) + 512; >>> size = round_up(size, PAGE_SIZE); >>> obj = i915_gem_object_create_internal(vma->vm->i915, size); >>> if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c index 3c55e77b0f1b00..fe999a02f8e10a 100644 --- a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c +++ b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c @@ -50,7 +50,7 @@ igt_emit_store_dw(struct i915_vma *vma, u32 *cmd; int err; - size = (4 * count + 1) * sizeof(u32); + size = (4 * count + 1) * sizeof(u32) + 512; size = round_up(size, PAGE_SIZE); obj = i915_gem_object_create_internal(vma->vm->i915, size); if (IS_ERR(obj))
Instruction prefetch mechanism requires that 512 bytes after the last command should be readable by EU. Otherwise DMAR errors and engine hangs can happen. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5278 Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> --- drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)