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@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@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@gmail.com> <mailto:baijiaju1990@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