[BUG] gpu: drm: amdgpu: Possible sleep-in-atomic-context bugs in amdgpu_uvd_ring_begin_use()

[Jia-Ju Bai]

On 2018/9/15 17:23, Koenig, Christian wrote:
> No, the problem is the function pointer analysis.
>
> In other words the KIQ ring is sometimes used in atomic and even 
> interrupt context.
>
> But the UVD ring is never used in atomic context.
>
> But I don't see a way a static analysis could ever figure that out.
>

Okay, thanks for your explanation :)
Besides, I find that amdgpu_virt_kiq_rreg() calls msleep(), so mdelay() 
should be used instead.


Best wishes,
Jia-Ju Bai

>
> Am 15.09.2018 11:18 schrieb Jia-Ju Bai <baijiaju1990 at gmail.com>:
> Sorry, I am still not clear why the call chain I proposed is incorrect...
>
> I find a conditional in amdgpu_mm_wreg():
>
>     if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
>         return amdgpu_virt_kiq_wreg(adev, reg, v);
>
> Is amdgpu_virt_kiq_wreg() never called from WREG32() or RREG32()?
>
>
> Best wishes,
> Jia-Ju Bai
>
>
> On 2018/9/15 17:10, Koenig, Christian wrote:
>> amdgpu_ring_alloc() does call amdgpu_uvd_begin_use(), but never in 
>> the call chain you proposed.
>>
>> Thinking about it I actually don't see a way a statically analysis 
>> could ever figure that out.
>>
>> Christian.
>>
>> Am 15.09.2018 11:05 schrieb Jia-Ju Bai <baijiaju1990 at gmail.com>:
>>
>>     Thanks for your reply.
>>
>>     On 2018/9/15 17:01, Koenig, Christian wrote:
>>
>>         Sorry to say that but your analysis tool is buggy.
>>
>>         The proposed call paths will never trigger.
>>
>>
>>     Could you please explain which piece of the call path is incorrect?
>>     I am not very sure of my function pointer analysis.
>>     Does amdgpu_ring_alloc() never calls amdgpu_uvd_ring_begin_use()?
>>
>>     Thanks in advance.
>>
>>
>>     Best wishes,
>>     Jia-Ju Bai
>>
>>
>>         Regards,
>>         Christian.
>>
>>         Am 15.09.2018 10:59 schrieb Jia-Ju Bai
>>         <baijiaju1990 at gmail.com> <mailto:baijiaju1990 at gmail.com>:
>>         The driver may sleep with holding a spinlock.
>>         The function call paths (from bottom to top) in Linux-4.17 are:
>>
>>         [FUNC] mutex_lock_nested
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
>>              mutex_lock_nested in amdgpu_dpm_enable_uvd
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
>>              amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
>>              [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
>>              amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
>>              amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
>>         drivers/gpu/drm/amd/amdgpu/vi.c, 207:
>>              amdgpu_mm_wreg in vi_gc_cac_rreg
>>         drivers/gpu/drm/amd/amdgpu/vi.c, 206:
>>              _raw_spin_lock_irqsave in vi_gc_cac_rreg
>>
>>         [FUNC] mutex_lock_nested
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
>>              mutex_lock_nested in amdgpu_dpm_enable_uvd
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
>>              amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
>>              [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
>>              amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
>>              amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
>>         drivers/gpu/drm/amd/amdgpu/soc15.c, 106:
>>              amdgpu_mm_wreg in soc15_pcie_rreg
>>         drivers/gpu/drm/amd/amdgpu/soc15.c, 105:
>>              _raw_spin_lock_irqsave in soc15_pcie_rreg
>>
>>         [FUNC] mutex_lock_nested
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
>>              mutex_lock_nested in amdgpu_dpm_enable_uvd
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
>>              amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
>>              [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
>>              amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
>>              amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
>>         drivers/gpu/drm/amd/amdgpu/cik.c, 139:
>>              amdgpu_mm_wreg in cik_uvd_ctx_wreg
>>         drivers/gpu/drm/amd/amdgpu/cik.c, 138:
>>              _raw_spin_lock_irqsave in cik_uvd_ctx_wreg
>>
>>         [FUNC] mutex_lock_nested
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
>>              mutex_lock_nested in amdgpu_dpm_enable_uvd
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
>>              amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
>>              [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
>>              amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
>>         drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
>>              amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
>>         drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 126:
>>              amdgpu_mm_wreg in dce_v6_0_audio_endpt_rreg
>>         drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 125:
>>              _raw_spin_lock_irqsave in dce_v6_0_audio_endpt_rreg
>>
>>         Note that [FUNC_PTR] means a function pointer call is used.
>>
>>         These bugs are found by my static analysis tool DSAC.
>>
>>
>>         Best wishes,
>>         Jia-Ju Bai
>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180915/4a506d53/attachment-0001.html>