| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAD=FV=VYkpJb_z7N_JANNZRCTJ6vbSqbLw0sKwrssENgnSqdhQ@mail.gmail.com>
Date: Tue, 26 Apr 2022 11:48:58 -0700
From: Doug Anderson <dianders@...omium.org>
To: Kuogee Hsieh <quic_khsieh@...cinc.com>
Cc: Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
Stephen Boyd <swboyd@...omium.org>,
Vinod Koul <vkoul@...nel.org>, Daniel Vetter <daniel@...ll.ch>,
David Airlie <airlied@...ux.ie>,
Andy Gross <agross@...nel.org>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
"Abhinav Kumar (QUIC)" <quic_abhinavk@...cinc.com>,
"Aravind Venkateswaran (QUIC)" <quic_aravindh@...cinc.com>,
Sankeerth Billakanti <quic_sbillaka@...cinc.com>,
freedreno <freedreno@...ts.freedesktop.org>,
dri-devel <dri-devel@...ts.freedesktop.org>,
linux-arm-msm <linux-arm-msm@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] drm/msm/dp: remove fail safe mode related code
Hi,
On Mon, Apr 25, 2022 at 10:56 PM Kuogee Hsieh <quic_khsieh@...cinc.com> wrote:
>
> Current DP driver implementation has adding safe mode done at
> dp_hpd_plug_handle() which is expected to be executed under event
> thread context.
>
> However there is possible circular locking happen (see blow stack trace)
> after edp driver call dp_hpd_plug_handle() from dp_bridge_enable() which
> is executed under drm_thread context.
>
> After review all possibilities methods and as discussed on
> https://patchwork.freedesktop.org/patch/483155/, supporting EDID
> compliance tests in the driver is quite hacky. As seen with other
> vendor drivers, supporting these will be much easier with IGT. Hence
> removing all the related fail safe code for it so that no possibility
> of circular lock will happen.
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 5.15.35-lockdep #6 Tainted: G W
> ------------------------------------------------------
> frecon/429 is trying to acquire lock:
> ffffff808dc3c4e8 (&dev->mode_config.mutex){+.+.}-{3:3}, at:
> dp_panel_add_fail_safe_mode+0x4c/0xa0
>
> but task is already holding lock:
> ffffff808dc441e0 (&kms->commit_lock[i]){+.+.}-{3:3}, at: lock_crtcs+0xb4/0x124
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #3 (&kms->commit_lock[i]){+.+.}-{3:3}:
> __mutex_lock_common+0x174/0x1a64
> mutex_lock_nested+0x98/0xac
> lock_crtcs+0xb4/0x124
> msm_atomic_commit_tail+0x330/0x748
> commit_tail+0x19c/0x278
> drm_atomic_helper_commit+0x1dc/0x1f0
> drm_atomic_commit+0xc0/0xd8
> drm_atomic_helper_set_config+0xb4/0x134
> drm_mode_setcrtc+0x688/0x1248
> drm_ioctl_kernel+0x1e4/0x338
> drm_ioctl+0x3a4/0x684
> __arm64_sys_ioctl+0x118/0x154
> invoke_syscall+0x78/0x224
> el0_svc_common+0x178/0x200
> do_el0_svc+0x94/0x13c
> el0_svc+0x5c/0xec
> el0t_64_sync_handler+0x78/0x108
> el0t_64_sync+0x1a4/0x1a8
>
> -> #2 (crtc_ww_class_mutex){+.+.}-{3:3}:
> __mutex_lock_common+0x174/0x1a64
> ww_mutex_lock+0xb8/0x278
> modeset_lock+0x304/0x4ac
> drm_modeset_lock+0x4c/0x7c
> drmm_mode_config_init+0x4a8/0xc50
> msm_drm_init+0x274/0xac0
> msm_drm_bind+0x20/0x2c
> try_to_bring_up_master+0x3dc/0x470
> __component_add+0x18c/0x3c0
> component_add+0x1c/0x28
> dp_display_probe+0x954/0xa98
> platform_probe+0x124/0x15c
> really_probe+0x1b0/0x5f8
> __driver_probe_device+0x174/0x20c
> driver_probe_device+0x70/0x134
> __device_attach_driver+0x130/0x1d0
> bus_for_each_drv+0xfc/0x14c
> __device_attach+0x1bc/0x2bc
> device_initial_probe+0x1c/0x28
> bus_probe_device+0x94/0x178
> deferred_probe_work_func+0x1a4/0x1f0
> process_one_work+0x5d4/0x9dc
> worker_thread+0x898/0xccc
> kthread+0x2d4/0x3d4
> ret_from_fork+0x10/0x20
>
> -> #1 (crtc_ww_class_acquire){+.+.}-{0:0}:
> ww_acquire_init+0x1c4/0x2c8
> drm_modeset_acquire_init+0x44/0xc8
> drm_helper_probe_single_connector_modes+0xb0/0x12dc
> drm_mode_getconnector+0x5dc/0xfe8
> drm_ioctl_kernel+0x1e4/0x338
> drm_ioctl+0x3a4/0x684
> __arm64_sys_ioctl+0x118/0x154
> invoke_syscall+0x78/0x224
> el0_svc_common+0x178/0x200
> do_el0_svc+0x94/0x13c
> el0_svc+0x5c/0xec
> el0t_64_sync_handler+0x78/0x108
> el0t_64_sync+0x1a4/0x1a8
>
> -> #0 (&dev->mode_config.mutex){+.+.}-{3:3}:
> __lock_acquire+0x2650/0x672c
> lock_acquire+0x1b4/0x4ac
> __mutex_lock_common+0x174/0x1a64
> mutex_lock_nested+0x98/0xac
> dp_panel_add_fail_safe_mode+0x4c/0xa0
> dp_hpd_plug_handle+0x1f0/0x280
> dp_bridge_enable+0x94/0x2b8
> drm_atomic_bridge_chain_enable+0x11c/0x168
> drm_atomic_helper_commit_modeset_enables+0x500/0x740
> msm_atomic_commit_tail+0x3e4/0x748
> commit_tail+0x19c/0x278
> drm_atomic_helper_commit+0x1dc/0x1f0
> drm_atomic_commit+0xc0/0xd8
> drm_atomic_helper_set_config+0xb4/0x134
> drm_mode_setcrtc+0x688/0x1248
> drm_ioctl_kernel+0x1e4/0x338
> drm_ioctl+0x3a4/0x684
> __arm64_sys_ioctl+0x118/0x154
> invoke_syscall+0x78/0x224
> el0_svc_common+0x178/0x200
> do_el0_svc+0x94/0x13c
> el0_svc+0x5c/0xec
> el0t_64_sync_handler+0x78/0x108
> el0t_64_sync+0x1a4/0x1a8
>
> Changes in v2:
> -- re text commit title
> -- remove all fail safe mode
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
Reported-by: Douglas Anderson <dianders@...omium.org>
Fixes: f1b47e6a8df8 ("drm/msm/dp: remove fail safe mode related code")
> @@ -151,15 +151,6 @@ static int dp_panel_update_modes(struct drm_connector *connector,
> return rc;
> }
>
> -void dp_panel_add_fail_safe_mode(struct drm_connector *connector)
> -{
> - /* fail safe edid */
> - mutex_lock(&connector->dev->mode_config.mutex);
> - if (drm_add_modes_noedid(connector, 640, 480))
> - drm_set_preferred_mode(connector, 640, 480);
> - mutex_unlock(&connector->dev->mode_config.mutex);
> -}
You removed this function but you forgot to remove it from the header
in "drivers/gpu/drm/msm/dp/dp_panel.h"
-Doug
Powered by blists - more mailing lists