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/amd-gfx/attachments/20180915/4a506d53/attachment.html>
