Re: [Intel-gfx] [PATCH] drm/i915/selftests: add prefetch padding to store_dw batchbuffer

[Andrzej Hajda <andrzej.hajda@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3108 bytes --]



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))

[-- Attachment #2: Type: text/html, Size: 5608 bytes --]